Split the v1 implementation into functional translation units

.github/scripts/check_inl_sources.py

@@ -1,36 +1,35 @@
 #!/usr/bin/env python3
-"""Consistency checks for a library built as a single translation unit.
-
-The implementation lives in *.inl files that a single hub *.cpp includes,
-directly or through other *.inl files. This script checks that the hub and
-the *.inl files agree:
-
-  1. every *.inl include reachable from the hub names an existing file under
-     the source root, no file includes the same *.inl twice, and *.inl files
-     are included with the quoted form only
-  2. every *.inl under the source root is reachable from the hub
-  3. every *.inl starts with  #ifndef <MACRO> / #error / #endif  so it cannot
-     be included by user code on its own
-  4. no *.c / *.cpp other than the hub exists under the source root (a stray
-     one would compile as a separate translation unit and defeat the purpose)
-  5. (--compile) every *.inl the hub includes directly compiles on its own with
-     only the guard macro defined, so no file silently depends on what an
-     earlier include brought in (files included by other *.inl files are
-     fragments of those and are not compiled separately)
+"""Consistency checks for functional hubs and standalone translation units.
+
+Each hub *.cpp includes implementation *.inl files, directly or through other
+*.inl files. Large implementations can instead be standalone *.cpp files.
+This script checks that the source tree and its declared translation units agree:
+
+  1. every reachable *.inl include names an existing file under the source root,
+     no file includes the same *.inl twice, and includes use the quoted form
+  2. every *.inl under the source root is reachable from a hub
+  3. every *.inl starts with #ifndef <MACRO> / #error / #endif
+  4. no *.c / *.cpp other than the declared hubs and standalone sources exists
+     under the source root
+  5. (--compile) every *.inl directly included by a hub compiles on its own with
+     only the guard macro defined; nested fragments are not compiled separately
+  6. no *.inl is reachable from more than one hub
 
 The checks are lexical: comments, string literals and line splices are
-handled like the preprocessor does, but #if conditions are not evaluated, so
-an include inside a disabled #if block still counts as reachable, a file
-included from several places (the platform directories each include the
-same bit-bang helpers, under mutually exclusive conditions) is not reported
-as a duplicate, and a platform file that compiles to nothing on the host
-still passes the compile check. The script guards against mistakes, not
-against code written to evade it.
+handled like the preprocessor does, but #if conditions are not evaluated.
+An include inside a disabled #if still counts as reachable. Multiple paths
+within one hub (such as mutually exclusive platform bit-bang helpers) are
+allowed; paths from different hubs are rejected. A platform file that
+compiles to nothing on the host still passes the compile check. The script
+guards against mistakes, not against code written to evade it.
 
 Usage:
-  check_inl_sources.py --hub src/lgfx/v1/lgfx_v1.cpp --root src/lgfx/v1 \
-      --macro LGFX_V1_IMPLEMENTATION [--compile CXX -std=c++17 -DLGFX_SDL -Isrc ...]
+  check_inl_sources.py --hub src/lgfx/v1/lgfx_v1.cpp \
+      --hub src/lgfx/v1/lgfx_v1_panel.cpp --source src/lgfx/v1/LGFXBase.cpp \
+      --root src/lgfx/v1 --macro LGFX_V1_IMPLEMENTATION \
+      [--compile CXX -std=c++17 -DLGFX_SDL -Isrc ...]
 
+Repeat --hub and --source for every translation unit under --root.
 Everything after --compile is the compiler command line; the script appends
 -fsyntax-only -x c++ -D<MACRO> and the file.
 """
@@ -221,9 +220,6 @@ def visit(src, chain):
             visit(path, chain + (real,))
 
     visit(hub, (os.path.realpath(hub),))
-    for inl in walk(root, {'.inl'}):
-        if inl not in reachable:
-            problems.append(f'{rel(inl, root)}: not reachable from the hub')
     return direct, reachable
 
 
@@ -239,10 +235,11 @@ def check_guards(root, macro, problems):
             problems.append(f'{rel(inl, root)}: does not start with "#ifndef {macro}" / "#error" / "#endif"')
 
 
-def check_stray_sources(root, hub, problems):
+def check_stray_sources(root, sources, problems):
+    allowed = {os.path.realpath(src) for src in sources}
     for src in walk(root, {'.c', '.cpp'}):
-        if os.path.abspath(src) != os.path.abspath(hub):
-            problems.append(f'{rel(src, root)}: source file outside the hub (add it as *.inl to the hub instead)')
+        if os.path.realpath(src) not in allowed:
+            problems.append(f'{rel(src, root)}: undeclared source file (add it to a hub as *.inl or declare it with --source)')
 
 
 def compile_one(cmd, macro, path):
@@ -272,22 +269,49 @@ def main():
         if not compile_cmd:
             sys.exit('--compile needs a compiler command line')
     ap = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter)
-    ap.add_argument('--hub', required=True, help='the *.cpp that includes the *.inl files')
-    ap.add_argument('--root', required=True, help='directory whose *.inl files must all be reachable from the hub')
-    ap.add_argument('--macro', required=True, help='guard macro the hub defines around its includes')
+    ap.add_argument('--hub', required=True, action='append', help='hub *.cpp (repeat for each hub)')
+    ap.add_argument('--source', action='append', default=[], help='standalone *.c / *.cpp (repeat for each source)')
+    ap.add_argument('--root', required=True, help='directory whose *.inl files must all be reachable from the hubs')
+    ap.add_argument('--macro', required=True, help='guard macro each hub defines around its includes')
     args = ap.parse_args(argv)
 
-    hub = os.path.normpath(os.path.abspath(args.hub))
-    root = os.path.normpath(os.path.abspath(args.root))
+    hubs = [os.path.abspath(p) for p in args.hub]
+    sources = [os.path.abspath(p) for p in args.source]
+    root = os.path.abspath(args.root)
     problems = []
+    seen = set()
+    for src in hubs + sources:
+        real = os.path.realpath(src)
+        if real in seen:
+            ap.error(f'duplicate translation unit: {src}')
+        seen.add(real)
+        if not os.path.isfile(src) or not is_under(src, root):
+            ap.error(f'translation unit must be an existing file under --root: {src}')
+        if os.path.splitext(src)[1] not in {'.c', '.cpp'}:
+            ap.error(f'translation unit must be a *.c / *.cpp file: {src}')
 
-    direct, reachable = check_reachability(hub, root, problems)
+    direct = []
+    owners = {}
+    for hub in hubs:
+        hub_direct, reachable = check_reachability(hub, root, problems)
+        direct.extend(hub_direct)
+        for inl in sorted(reachable):
+            real = os.path.realpath(inl)
+            previous = owners.get(real)
+            if previous is not None and previous != hub:
+                problems.append(f'{rel(inl, root)}: reachable from multiple hubs: '
+                                f'{rel(previous, root)}, {rel(hub, root)}')
+            owners[real] = hub
+        print(f'{rel(hub, root)}: {len(hub_direct)} files included directly, {len(reachable)} reachable')
+    for inl in walk(root, {'.inl'}):
+        if os.path.realpath(inl) not in owners:
+            problems.append(f'{rel(inl, root)}: not reachable from any hub')
     check_guards(root, args.macro, problems)
-    check_stray_sources(root, hub, problems)
+    check_stray_sources(root, hubs + sources, problems)
     if compile_cmd:
-        check_compile(direct, root, args.macro, compile_cmd, problems)
+        check_compile(list(dict.fromkeys(direct)), root, args.macro, compile_cmd, problems)
 
-    print(f'{rel(hub, root)}: {len(direct)} files included directly, {len(reachable)} reachable, '
+    print(f'{len(hubs)} hubs, {len(sources)} standalone sources, '
           f'{sum(1 for _ in walk(root, {".inl"}))} *.inl under {rel(root, os.getcwd())}'
           + (f', compiled with: {" ".join(compile_cmd)}' if compile_cmd else ''))
     if problems:

.github/workflows/SourceCheck.yml

@@ -15,7 +15,7 @@ jobs:
 
   inl-sources:
 
-    name: single translation unit consistency
+    name: translation unit consistency
 
     runs-on: ubuntu-latest
 
@@ -28,14 +28,28 @@ jobs:
           sudo apt-get update
           sudo apt-get install -y libsdl2-dev
 
-      - name: Check the hub and the *.inl files (SDL back end)
+      - name: Check the hubs and the *.inl files (SDL back end)
         run: |
           python3 .github/scripts/check_inl_sources.py \
-            --hub src/lgfx/v1/lgfx_v1.cpp --root src/lgfx/v1 --macro LGFX_V1_IMPLEMENTATION \
+            --hub src/lgfx/v1/lgfx_v1.cpp \
+            --hub src/lgfx/v1/lgfx_v1_panel.cpp \
+            --hub src/lgfx/v1/lgfx_v1_touch.cpp \
+            --hub src/lgfx/v1/lgfx_v1_platforms.cpp \
+            --source src/lgfx/v1/LGFXBase.cpp \
+            --source src/lgfx/v1/lgfx_fonts.cpp \
+            --source src/lgfx/v1/panel/Panel_M5HDMI.cpp \
+            --root src/lgfx/v1 --macro LGFX_V1_IMPLEMENTATION \
             --compile g++ -std=c++17 -DLGFX_SDL -Isrc $(sdl2-config --cflags)
 
       - name: Check the *.inl files (Linux framebuffer back end)
         run: |
           python3 .github/scripts/check_inl_sources.py \
-            --hub src/lgfx/v1/lgfx_v1.cpp --root src/lgfx/v1 --macro LGFX_V1_IMPLEMENTATION \
+            --hub src/lgfx/v1/lgfx_v1.cpp \
+            --hub src/lgfx/v1/lgfx_v1_panel.cpp \
+            --hub src/lgfx/v1/lgfx_v1_touch.cpp \
+            --hub src/lgfx/v1/lgfx_v1_platforms.cpp \
+            --source src/lgfx/v1/LGFXBase.cpp \
+            --source src/lgfx/v1/lgfx_fonts.cpp \
+            --source src/lgfx/v1/panel/Panel_M5HDMI.cpp \
+            --root src/lgfx/v1 --macro LGFX_V1_IMPLEMENTATION \
             --compile g++ -std=c++17 -DLGFX_LINUX_FB -Isrc

CMakeLists.txt

@@ -1,11 +1,17 @@
 set(COMPONENT_ADD_INCLUDEDIRS
     src
     )
-# The build set is fixed: src/lgfx/v1/lgfx_v1.cpp includes every implementation file (*.inl),
-# the font tables and the C decoders are separate translation units on purpose.
+# Functional hubs share header parsing; large implementations and data tables stay
+# in separate translation units so unused groups can be discarded by the linker.
 set(SRCS
      src/M5GFX.cpp
      src/lgfx/v1/lgfx_v1.cpp
+     src/lgfx/v1/lgfx_v1_panel.cpp
+     src/lgfx/v1/lgfx_v1_touch.cpp
+     src/lgfx/v1/lgfx_v1_platforms.cpp
+     src/lgfx/v1/LGFXBase.cpp
+     src/lgfx/v1/lgfx_fonts.cpp
+     src/lgfx/v1/panel/Panel_M5HDMI.cpp
      src/lgfx/Fonts/efont/lgfx_efont_cn.c
      src/lgfx/Fonts/efont/lgfx_efont_ja.c
      src/lgfx/Fonts/efont/lgfx_efont_kr.c

component.mk

@@ -3,5 +3,5 @@
 #
 # (Uses default behaviour of compiling all source files in directory, adding 'include' to include path.)
 
-COMPONENT_SRCDIRS := src src/lgfx/v1 src/lgfx/utility src/lgfx/Fonts/efont src/lgfx/Fonts/IPA
+COMPONENT_SRCDIRS := src src/lgfx/v1 src/lgfx/v1/panel src/lgfx/utility src/lgfx/Fonts/efont src/lgfx/Fonts/IPA
 COMPONENT_ADD_INCLUDEDIRS := src

src/lgfx/v1/LGFXBase.inl → src/lgfx/v1/LGFXBase.cpp

@@ -16,9 +16,6 @@ Original Source:
  [tobozo](https://github.com/tobozo)
 /----------------------------------------------------------------------------*/
 
-#ifndef LGFX_V1_IMPLEMENTATION
-#error "LGFXBase.inl is part of lgfx_v1.cpp and is not meant to be included on its own"
-#endif
 #include "LGFXBase.hpp"
 
 #include "../internal/limits.h"

src/lgfx/v1/lgfx_fonts.inl → src/lgfx/v1/lgfx_fonts.cpp

@@ -1,6 +1,3 @@
-#ifndef LGFX_V1_IMPLEMENTATION
-#error "lgfx_fonts.inl is part of lgfx_v1.cpp and is not meant to be included on its own"
-#endif
 #include "lgfx_fonts.hpp"
 
 #include "platforms/common.hpp"

src/lgfx/v1/lgfx_v1.cpp

@@ -1,87 +1,37 @@
-// LovyanGFX v1 is built as a single translation unit: this file includes every
-// implementation file (*.inl) under src/lgfx/v1, and those are not compiled on their own.
-//
-// Why: every implementation file pulls in the same framework headers, and parsing them
-// once instead of once per file cuts the library build time several-fold (the Arduino
-// build also pre-scans each source file for includes, which doubled that cost).
+// LovyanGFX v1 uses functional hubs for *.inl implementations and standalone *.cpp
+// files for LGFXBase, fonts, and implementations with large data tables.
+// Older RISC-V toolchains (assembler and linker) can retain unused code throughout
+// a translation unit when its last function is used. Keeping functional groups and large tables in separate
+// translation units limits this effect while sharing header parsing within each hub.
 //
 // Maintenance notes:
-// - Adding an implementation file: create it as *.inl and add an #include below. Do not
-//   add *.cpp files under src/lgfx/v1. LovyanGFX and M5GFX share this source tree but keep
-//   their own copy of this file, because their panel sets differ; update both.
-// - File-local names are visible to every file that follows: static functions and
-//   constants, macros, and anonymous namespaces do not isolate files here. Use a name that
-//   identifies the file (or a class member), and #undef helper macros at the end of the
-//   file that defines them.
-// - Platform directories are self-guarded, so only the active platform contributes code;
-//   the desktop back ends (sdl / opencv / framebuffer) follow the selection made in
-//   platforms/common.hpp (LGFX_PLATFORM_*).
-// - Two files provide helpers for the ones that follow and are marked "must precede".
-// - The C decoders in src/lgfx/utility and the font tables in src/lgfx/Fonts stay separate
-//   translation units on purpose (third-party file-local names collide; the font tables
-//   would make a single 100+ MB translation unit).
-// - .github/scripts/check_inl_sources.py checks these rules (every *.inl is reachable from
-//   this file, directly or through another *.inl, starts with the guard below, and the ones
-//   included here compile on their own); CI runs it, and it can be run locally with the
-//   command line in .github/workflows/SourceCheck.yml.
-
+// - Add a small implementation as *.inl to the matching hub: lgfx_v1.cpp for sprites,
+//   buttons and misc, lgfx_v1_panel.cpp for panels, lgfx_v1_touch.cpp for touch, or
+//   lgfx_v1_platforms.cpp for platform back ends. Keep implementations with large
+//   tables in standalone *.cpp files, and update the CMake source lists and SourceCheck.
+// - LovyanGFX and M5GFX share this source tree but keep their own hub include lists
+//   because their supported panels and platforms differ; update both.
+// - File-local names are visible to later includes within the same hub. Use names
+//   identifying the file (or class members), and #undef helper macros at the end.
+//   Dependencies across translation units need declarations in shared headers.
+// - Platform directories are self-guarded. Desktop back ends follow the selection
+//   in platforms/common.hpp (LGFX_PLATFORM_*). Keep the platform hub's "must precede"
+//   includes in order: FrameBufferBase provides cache helpers and esp32/common
+//   provides reg() / writereg().
+// - C decoders in src/lgfx/utility and font tables in src/lgfx/Fonts stay separate
+//   translation units (third-party file-local names collide; the font tables would
+//   make a single 100+ MB translation unit).
+// - .github/scripts/check_inl_sources.py checks reachability, guards, allowed sources,
+//   and ownership by exactly one hub. It also compiles each direct *.inl include on
+//   its own; run the commands in .github/workflows/SourceCheck.yml locally.
 
 #define LGFX_V1_IMPLEMENTATION
 
-#include "LGFXBase.inl"
 #include "LGFX_Sprite.inl"
 #include "LGFX_Button.inl"
-#include "lgfx_fonts.inl"
-
 #include "misc/DividedFrameBuffer.inl"
 #include "misc/SpriteBuffer.inl"
 #include "misc/common_function.inl"
 #include "misc/pixelcopy.inl"
 
-#include "panel/Panel_AMOLED.inl"
-#include "panel/Panel_Device.inl"
-#include "panel/Panel_ED2208.inl"
-#include "panel/Panel_EPDiy.inl"
-#include "panel/Panel_FrameBufferBase.inl"   // must precede platforms/esp32/Panel_EPD.inl (provides cacheWriteBack / Cache_WriteBack_Addr)
-#include "panel/Panel_GDEW0154D67.inl"
-#include "panel/Panel_GDEW0154M09.inl"
-#include "panel/Panel_HasBuffer.inl"
-#include "panel/Panel_IT8951.inl"
-#include "panel/Panel_LCD.inl"
-#include "panel/Panel_M5HDMI.inl"
-#include "panel/Panel_M5UnitGLASS.inl"
-#include "panel/Panel_M5UnitLCD.inl"
-#include "panel/Panel_SSD1306.inl"
-#include "panel/Panel_SSD1677.inl"
-#include "panel/Panel_TM1680.inl"
-
-#include "touch/Touch_CHSC6540.inl"
-#include "touch/Touch_CSTxxx.inl"
-#include "touch/Touch_FT5x06.inl"
-#include "touch/Touch_GT911.inl"
-
-#include "platforms/esp32/common.inl"   // must precede the other esp32* files (provides reg() / writereg())
-#include "platforms/esp32/Bus_EPD.inl"
-#include "platforms/esp32/Bus_I2C.inl"
-#include "platforms/esp32/Bus_Parallel8.inl"
-#include "platforms/esp32/Bus_SPI.inl"
-#include "platforms/esp32/Light_PWM.inl"
-#include "platforms/esp32/Panel_CVBS.inl"
-#include "platforms/esp32/Panel_EPD.inl"
-
-#include "platforms/esp32c3/Bus_Parallel8.inl"
-#include "platforms/esp32p4/Bus_DSI.inl"
-#include "platforms/esp32p4/Panel_DSI.inl"
-#include "platforms/esp32p4/Panel_LT8912B.inl"
-#include "platforms/esp32p4/Touch_ST7123.inl"
-#include "platforms/esp32s2/Bus_Parallel16.inl"
-#include "platforms/esp32s2/Bus_Parallel8.inl"
-#include "platforms/esp32s3/Bus_Parallel16.inl"
-#include "platforms/esp32s3/Bus_Parallel8.inl"
-
-#include "platforms/framebuffer/Panel_fb.inl"
-#include "platforms/framebuffer/common.inl"
-#include "platforms/sdl/Panel_sdl.inl"
-#include "platforms/sdl/common.inl"
-
 #undef LGFX_V1_IMPLEMENTATION

src/lgfx/v1/lgfx_v1_panel.cpp

@@ -0,0 +1,20 @@
+// See lgfx_v1.cpp for the translation unit layout and maintenance notes.
+
+#define LGFX_V1_IMPLEMENTATION
+
+#include "panel/Panel_AMOLED.inl"
+#include "panel/Panel_Device.inl"
+#include "panel/Panel_ED2208.inl"
+#include "panel/Panel_EPDiy.inl"
+#include "panel/Panel_GDEW0154D67.inl"
+#include "panel/Panel_GDEW0154M09.inl"
+#include "panel/Panel_HasBuffer.inl"
+#include "panel/Panel_IT8951.inl"
+#include "panel/Panel_LCD.inl"
+#include "panel/Panel_M5UnitGLASS.inl"
+#include "panel/Panel_M5UnitLCD.inl"
+#include "panel/Panel_SSD1306.inl"
+#include "panel/Panel_SSD1677.inl"
+#include "panel/Panel_TM1680.inl"
+
+#undef LGFX_V1_IMPLEMENTATION

src/lgfx/v1/lgfx_v1_platforms.cpp

@@ -0,0 +1,28 @@
+// See lgfx_v1.cpp for the translation unit layout and maintenance notes.
+
+#define LGFX_V1_IMPLEMENTATION
+
+#include "panel/Panel_FrameBufferBase.inl"   // must precede platforms/esp32/Panel_EPD.inl (provides cacheWriteBack / Cache_WriteBack_Addr)
+#include "platforms/esp32/common.inl"   // must precede the other esp32* files (provides reg() / writereg())
+#include "platforms/esp32/Bus_EPD.inl"
+#include "platforms/esp32/Bus_I2C.inl"
+#include "platforms/esp32/Bus_Parallel8.inl"
+#include "platforms/esp32/Bus_SPI.inl"
+#include "platforms/esp32/Light_PWM.inl"
+#include "platforms/esp32/Panel_CVBS.inl"
+#include "platforms/esp32/Panel_EPD.inl"
+#include "platforms/esp32c3/Bus_Parallel8.inl"
+#include "platforms/esp32p4/Bus_DSI.inl"
+#include "platforms/esp32p4/Panel_DSI.inl"
+#include "platforms/esp32p4/Panel_LT8912B.inl"
+#include "platforms/esp32p4/Touch_ST7123.inl"
+#include "platforms/esp32s2/Bus_Parallel16.inl"
+#include "platforms/esp32s2/Bus_Parallel8.inl"
+#include "platforms/esp32s3/Bus_Parallel16.inl"
+#include "platforms/esp32s3/Bus_Parallel8.inl"
+#include "platforms/framebuffer/Panel_fb.inl"
+#include "platforms/framebuffer/common.inl"
+#include "platforms/sdl/Panel_sdl.inl"
+#include "platforms/sdl/common.inl"
+
+#undef LGFX_V1_IMPLEMENTATION