Repository navigation
Dynamic font loading - #674
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 📝 WalkthroughWalkthroughThe font pipeline adds cmap-based codepoint extraction, Adwaita font generation and caching, configurable font sizes, and appearance settings for regular and monospace fonts. LVGL, the boot screen, and the terminal use registered or loaded BinFont fonts. The graphics module’s fixed bitmap fonts and renderer are removed. The file browser gains keyboard actions for deleting and renaming entries. The LED-strip brightness default and Apps package-list stack depth also change. Priority: ➖ Normal Merge Risk: 🔵 Low · up to Font changes can appear to apply but differ after a restart, particularly if settings change while fonts are generated or an existing cache is unusable. These bounded cases warrant fixes or owner acceptance before merge. Security Architecture ReviewSecurity architecture risk: 🟠 High · up to Selected font files now reach a native parser that is not designed for untrusted input and affect shared rendering and startup. The apply flow can also save settings different from those it prepared. Confirmed exposure is device-local; remote control has not been established. Retained concerns
Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 13.89% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 144 functions across 44 files. (2 skipped: 2 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
5f338c8f-5c14-407d-b388-6613f3b41820
⛔ Files ignored due to path filters (16)
Data/system/fonts/AdwaitaMono.ttfis excluded by!**/*.ttfData/system/fonts/AdwaitaSans.ttfis excluded by!**/*.ttfDocumentation/pics/screenshot-AppList.pngis excluded by!**/*.pngDocumentation/pics/screenshot-Launcher.pngis excluded by!**/*.pngDocumentation/pics/screenshot-Settings.pngis excluded by!**/*.pngTactility/Fonts/AdwaitaMono-Regular.ttfis excluded by!**/*.ttfTactility/Fonts/AdwaitaSans-Regular.ttfis excluded by!**/*.ttfpartitions-16mb-no-sd-dev.csvis excluded by!**/*.csvpartitions-16mb-no-sd.csvis excluded by!**/*.csvpartitions-16mb-with-sd.csvis excluded by!**/*.csvpartitions-32mb-no-sd-dev.csvis excluded by!**/*.csvpartitions-32mb-no-sd.csvis excluded by!**/*.csvpartitions-4mb-with-sd.csvis excluded by!**/*.csvpartitions-8mb-no-sd-dev.csvis excluded by!**/*.csvpartitions-8mb-no-sd.csvis excluded by!**/*.csvpartitions-8mb-with-sd.csvis excluded by!**/*.csv
📒 Files selected for processing (55)
Modules/binfont-module/include/binfont/generator.hModules/binfont-module/source/generator.cppModules/binfont-module/tests/CMakeLists.txtModules/binfont-module/tests/source/binfont_test.cppModules/graphics-module/KconfigModules/graphics-module/include/font/font.hModules/graphics-module/include/font/fonts.hModules/graphics-module/include/font/ibmplexmono12.hModules/graphics-module/include/font/ibmplexmono14.hModules/graphics-module/include/font/ibmplexmono16.hModules/graphics-module/include/font/ibmplexmono18.hModules/graphics-module/include/font/ibmplexmono24.hModules/graphics-module/include/font/ibmplexmono28.hModules/graphics-module/include/font/render.hModules/graphics-module/scripts/.gitignoreModules/graphics-module/scripts/generate-all.pyModules/graphics-module/scripts/generate.pyModules/graphics-module/source/ibmplexmono12.cModules/graphics-module/source/ibmplexmono14.cModules/graphics-module/source/ibmplexmono16.cModules/graphics-module/source/ibmplexmono18.cModules/graphics-module/source/ibmplexmono24.cModules/graphics-module/source/ibmplexmono28.cModules/graphics-module/source/module.cModules/graphics-module/source/render.cppModules/lvgl-module/CMakeLists.txtModules/lvgl-module/README.mdModules/lvgl-module/include/lvgl/fonts.hModules/lvgl-module/source/fonts.cTHIRD-PARTY-NOTICES.mdTactility/CMakeLists.txtTactility/Fonts/generate.pyTactility/Include/Tactility/settings/LedStripSettings.hTactility/KconfigTactility/Private/Tactility/app/boot/BootScreen.hTactility/Private/Tactility/app/terminal/TerminalRenderer.hTactility/Private/Tactility/lvgl/FontCache.hTactility/Private/Tactility/lvgl/FontSizes.hTactility/Private/Tactility/lvgl/FontVersions.hTactility/Private/Tactility/lvgl/TextFonts.hTactility/Source/app/apppackagelist/AppPackageList.cppTactility/Source/app/boot/BootInit.cppTactility/Source/app/boot/BootScreen.cppTactility/Source/app/terminal/Terminal.cppTactility/Source/app/terminal/TerminalRenderer.cppTactility/Source/lvgl/FontCache.cppTactility/Source/lvgl/FontSizes.cppTactility/Source/lvgl/IconFonts.cppTactility/Source/lvgl/Lvgl.cppTactility/Source/lvgl/TextFonts.cppTactility/Source/service/displayidle/MatrixRainScreensaver.cppTactility/Tests/CMakeLists.txtTactility/Tests/Source/FontCacheTest.cppdevice.pylv_conf.h
💤 Files with no reviewable changes (22)
- Modules/graphics-module/scripts/.gitignore
- Modules/graphics-module/include/font/fonts.h
- Modules/graphics-module/include/font/ibmplexmono24.h
- Modules/graphics-module/include/font/ibmplexmono16.h
- Modules/graphics-module/include/font/ibmplexmono14.h
- Modules/graphics-module/include/font/ibmplexmono12.h
- Modules/graphics-module/include/font/font.h
- Modules/graphics-module/include/font/render.h
- Modules/graphics-module/include/font/ibmplexmono18.h
- Modules/graphics-module/scripts/generate.py
- Modules/graphics-module/source/render.cpp
- Modules/graphics-module/source/ibmplexmono18.c
- Modules/graphics-module/source/ibmplexmono16.c
- Modules/graphics-module/source/ibmplexmono24.c
- Modules/graphics-module/source/ibmplexmono14.c
- Modules/graphics-module/source/ibmplexmono28.c
- Modules/graphics-module/source/ibmplexmono12.c
- Modules/graphics-module/include/font/ibmplexmono28.h
- Modules/graphics-module/scripts/generate-all.py
- Modules/graphics-module/Kconfig
- Modules/graphics-module/source/module.c
- Modules/lvgl-module/CMakeLists.txt
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
381eff62-5681-4be7-95a7-89c7a8a733ea
📒 Files selected for processing (24)
Modules/binfont-module/include/binfont/generator.hModules/binfont-module/private/stb_truetype.hModules/binfont-module/source/generator.cppModules/binfont-module/source/module.cModules/binfont-module/tests/source/binfont_test.cppTactility/Include/Tactility/settings/AppearanceSettings.hTactility/Private/Tactility/app/files/View.hTactility/Private/Tactility/lvgl/FontCache.hTactility/Private/Tactility/lvgl/FontSizes.hTactility/Private/Tactility/lvgl/Fonts.hTactility/Private/Tactility/lvgl/IconFonts.hTactility/Source/InitApps.cppTactility/Source/app/appearancesettings/AppearanceSettings.cppTactility/Source/app/boot/BootInit.cppTactility/Source/app/boot/BootScreen.cppTactility/Source/app/files/View.cppTactility/Source/app/terminal/Terminal.cppTactility/Source/lvgl/FontCache.cppTactility/Source/lvgl/FontSizes.cppTactility/Source/lvgl/Fonts.cppTactility/Source/lvgl/IconFonts.cppTactility/Source/settings/AppearanceSettings.cppTactility/Tests/CMakeLists.txtTactility/Tests/Source/FontsTest.cpp
💤 Files with no reviewable changes (2)
- Tactility/Source/lvgl/IconFonts.cpp
- Tactility/Private/Tactility/lvgl/IconFonts.h
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟡 Minor · Validate cached fonts before approving the appearance change. · FontCache.cpp:144-160
Tactility/Source/lvgl/FontCache.cpp:144-160
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winValidate cached fonts before approving the appearance change.
When
binfont_open_filerejects an existing custom-font cache and regeneration from the selected TTF also fails,ensureCachedFontstill returnstrue. The appearance app then saves the custom path beforeloadFontsfalls back to the system font. Validate the cache here so a failed regeneration prevents the save.Suggested fix
if (!cache_path.empty() && file::isFile(cache_path)) { - return true; + BinFont* cached_font = nullptr; + if (binfont_open_file(cache_path.c_str(), &cached_font) == ERROR_NONE) { + binfont_close(cached_font); + return true; + } }
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
44b000bd-49d5-4933-a7d1-a130cf07160a
⛔ Files ignored due to path filters (1)
Data/system/fonts/MaterialSymbolsRounded.ttfis excluded by!**/*.ttf
📒 Files selected for processing (10)
Data/system/fonts/MaterialSymbolsRounded.codepointsModules/lvgl-module/generate-icons.pyModules/lvgl-module/include/lvgl/icons/names.hModules/lvgl-module/include/lvgl/icons/shared.hModules/lvgl-module/source/icon_names.cTactility/KconfigTactility/Source/app/AppGrid.cppTactility/Source/app/appearancesettings/AppearanceSettings.cppTactility/Source/app/settings/Settings.cppTactility/Source/lvgl/Fonts.cpp
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.
Fonts
Dynamically load text and icon fonts when the device boots.
Font bitdepth depends on whether the device is an ESP32 one and whether it has external RAM available.
When no external RAM is available on ESP32, only small and medium font variants are generated; large font variants refer to the medium size ones.
Implemented Appearance settings app for changing font and font size.
Other changes