e-paper UX improvements - #672
KenVanHoeylandt wants to merge 1 commit into
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 📝 WalkthroughWalkthroughWhen mono themes are enabled, the display module extends the active theme to remove the default button border. AppGrid detects 1-bit monochrome displays and adjusts tile outlines, icon colors, focus handlers, and label sizing based on the display type. Priority: ⬇️ Low Merge Risk: 🔵 Low · up to On builds that enable mono-theme support, color displays may lose default button borders. This is a small visual regression, and the fix is a small localized change. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change affects screen appearance rather than privileges or data access. Remaining uncertainty concerns how display settings behave when multiple screens are initialized or replaced. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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:
e3a6c1f0-b8cc-4d18-96db-28fe04710a3a
📒 Files selected for processing (2)
Modules/lvgl-module/source/devices/display.cppTactility/Source/app/AppGrid.cpp
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.
| } | ||
|
|
||
| #if LV_USE_THEME_MONO | ||
| lvgl_display_extend_mono_theme(disp); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
sed -n 1,70p Modules/lvgl-module/source/devices/display.cpp; sed -n 490,520p Modules/lvgl-module/source/devices/display.cpp; rg -n 'LV_USE_THEME_MONO|LV_USE_THEME_DEFAULT' --glob '*.h' --glob '*.txt' --glob '*.cmake' . | head -30Repository: TactilityProject/Tactility
Length of output: 4045
🏁 Script executed:
printf '%s\n' '--- tracked LVGL/theme files ---'
git ls-files | rg '(^|/)(lvgl|lv_conf|theme|display)\.(c|h|cpp)$|lvgl/.+(display|theme)|theme_.+\.c$' | head -100
printf '%s\n' '--- theme init and default theme references ---'
rg -n 'lv_theme_(default|mono)_init|lv_display_set_theme|lv_display_get_theme|lv_display_create|LV_USE_THEME_MONO|LV_USE_THEME_DEFAULT|LV_COLOR_DEPTH' --glob '*.[ch]' --glob '*.cpp' --glob '*.cmake' --glob '*.txt' Modules lv_conf.h | head -160Repository: TactilityProject/Tactility
Length of output: 1197
🏁 Script executed:
printf '%s\n' '--- lvgl module source ---'
sed -n '1,220p' Modules/lvgl-module/source/lvgl.c
printf '%s\n' '--- relevant tracked build/dependency files ---'
git ls-files | rg '(^|/)(CMakeLists\\.txt|idf_component\\.yml|component\\.yml|Kconfig|manifest\\.yml|west\\.yml|.*lock.*|.*lvgl.*)$' | head -100
printf '%s\n' '--- LVGL setup/dependency references ---'
rg -n -i 'lvgl|theme_default_init|theme_mono_init|lv_init\\(' CMakeLists.txt Modules components idf_component.yml 2>/dev/null | head -180Repository: TactilityProject/Tactility
Length of output: 4958
🏁 Script executed:
printf '%s\n' '--- dependency and guidance ---'
git submodule status -- Libraries/lvgl
sed -n '1,180p' Modules/lvgl-module/CMakeLists.txt
sed -n '1,220p' .claude/rules/lvgl.md
printf '%s\n' '--- LVGL tracked theme/display files ---'
git -C Libraries/lvgl ls-files | rg '(^|/)(lv_display|lv_theme|theme|display).*\.(c|h)$' | head -120
printf '%s\n' '--- LVGL version/config references ---'
git -C Libraries/lvgl describe --tags --always --dirty 2>/dev/null || true
rg -n 'lv_display_create|lv_display_set_color_format|lv_theme_default_init|lv_theme_mono_init|LV_USE_THEME_MONO|LV_USE_THEME_DEFAULT|lv_display_get_theme|lv_display_set_theme' Libraries/lvgl/src Libraries/lvgl 2>/dev/null | head -220Repository: TactilityProject/Tactility
Length of output: 7434
🏁 Script executed:
printf '%s\n' '--- submodule metadata ---'
git ls-files .gitmodules
cat .gitmodules 2>/dev/null
git ls-tree HEAD Libraries/lvgl
git ls-tree 2b6b4b5167ff0795f720f724f6f378fcb8c7e2b7 Libraries/lvgl
printf '%s\n' '--- available submodule metadata files ---'
find Libraries/lvgl -maxdepth 2 -type f -print 2>/dev/null | head -40
printf '%s\n' '--- LVGL commit object availability ---'
git cat-file -t e9db313f8ccc07e27f6afd0f34fe3cf6f20596f0 2>/dev/null || trueRepository: TactilityProject/Tactility
Length of output: 1083
🌐 Web query:
LVGL e9db313f8ccc07e27f6afd0f34fe3cf6f20596f0 lv_display_create theme color format
💡 Result:
At commit `e9db313f8ccc07e27f6afd0f34fe3cf6f20596f0`:
- **Color format:** `lv_display_create()` initializes the display’s format to `LV_COLOR_FORMAT_NATIVE` (not `LV_COLOR_FORMAT_DEFAULT`). You can change it afterward with `lv_display_set_color_format(disp, format)`. ([github.com](https://github.com/lvgl/lvgl/blob/e9db313f8ccc07e27f6afd0f34fe3cf6f20596f0/src/display/lv_display.c))
- **Theme:** If `LV_USE_THEME_DEFAULT` is enabled and no default theme exists yet, creation initializes it with **blue primary**, **red secondary**, `LV_THEME_DEFAULT_DARK`, and `LV_FONT_DEFAULT`. Otherwise it reuses the existing default theme. Simple and mono themes have their own conditional initialization paths. ([github.com](https://github.com/lvgl/lvgl/blob/e9db313f8ccc07e27f6afd0f34fe3cf6f20596f0/src/display/lv_display.c))
So, the theme colors are initialization defaults; the display color format is set independently.
Citations:
- 1: https://github.com/lvgl/lvgl/blob/e9db313f8ccc07e27f6afd0f34fe3cf6f20596f0/src/display/lv_display.c
- 2: https://github.com/lvgl/lvgl/blob/e9db313f8ccc07e27f6afd0f34fe3cf6f20596f0/src/display/lv_display.c
🌐 Web query:
site:github.com/lvgl/lvgl/blob/e9db313f8ccc07e27f6afd0f34fe3cf6f20596f0/src/display/lv_display.c "lv_theme_mono_init" "lv_theme_default_init" "lv_display_create"
💡 Result:
At the matching path and commit, `lv_display_create()` initializes **one** theme, selected at compile time: default, simple, or mono. It calls the corresponding `*_init()` only if that theme is not already initialized; otherwise it uses `*_get()`. So `lv_theme_mono_init()` and `lv_theme_default_init()` are alternatives, not both called by the same creation path. ([github.com](https://github.com/lvgl/lvgl/blob/master/src/display/lv_display.c?utm_source=openai))
The search result is GitHub’s **`master`** view, not a confirmed rendering of commit `e9db313f8ccc07e27f6afd0f34fe3cf6f20596f0`; the exact commit’s contents remain unverified. ([github.com](https://github.com/lvgl/lvgl/blob/master/src/display/lv_display.c?utm_source=openai))
Citations:
- 1: https://github.com/lvgl/lvgl/blob/master/src/display/lv_display.c?utm_source=openai
- 2: https://github.com/lvgl/lvgl/blob/master/src/display/lv_display.c?utm_source=openai
🌐 Web query:
raw.githubusercontent.com/lvgl/lvgl/e9db313f8ccc07e27f6afd0f34fe3cf6f20596f0/src/display/lv_display.c lv_theme_mono_init lv_theme_default_init
💡 Result:
At commit `e9db313f8ccc07e27f6afd0f34fe3cf6f20596f0`, `lv_display.c` calls one theme initializer depending on compile-time configuration:
- If `LV_USE_THEME_DEFAULT` is enabled, it initializes the default theme with blue/red palette colors, `LV_THEME_DEFAULT_DARK`, and `LV_FONT_DEFAULT`; otherwise it reuses the existing default theme.
- If `LV_USE_THEME_MONO` is enabled instead, it initializes the mono theme with `dark_bg = false` and `LV_FONT_DEFAULT`; otherwise it reuses the existing mono theme. ([raw.githubusercontent.com](https://raw.githubusercontent.com/lvgl/lvgl/e9db313f8ccc07e27f6afd0f34fe3cf6f20596f0/src/display/lv_display.c))
The initializers are implemented in separate files, not `lv_display.c`: [`lv_theme_mono_init`](https://raw.githubusercontent.com/lvgl/lvgl/e9db313f8ccc07e27f6afd0f34fe3cf6f20596f0/src/themes/mono/lv_theme_mono.c) sets up the mono theme and its styles; [`lv_theme_default_init`](https://raw.githubusercontent.com/lvgl/lvgl/e9db313f8ccc07e27f6afd0f34fe3cf6f20596f0/src/themes/default/lv_theme_default.c) configures the default theme’s display, colors, fonts, and styles. ([raw.githubusercontent.com](https://raw.githubusercontent.com/lvgl/lvgl/e9db313f8ccc07e27f6afd0f34fe3cf6f20596f0/src/themes/mono/lv_theme_mono.c))
Citations:
- 1: https://raw.githubusercontent.com/lvgl/lvgl/e9db313f8ccc07e27f6afd0f34fe3cf6f20596f0/src/display/lv_display.c
- 2: https://raw.githubusercontent.com/lvgl/lvgl/e9db313f8ccc07e27f6afd0f34fe3cf6f20596f0/src/themes/mono/lv_theme_mono.c
🏁 Script executed:
printf '%s\n' '--- display API and implementation ---'
rg -n 'lvgl_display_add|lv_display_set_color_format|lv_color_format_t|LV_COLOR_FORMAT_' Modules/lvgl-module/include/lvgl/devices/display.h Modules/lvgl-module/source/devices/display.cpp | head -140
sed -n '1,180p' Modules/lvgl-module/include/lvgl/devices/display.h
sed -n '430,525p' Modules/lvgl-module/source/devices/display.cpp
printf '%s\n' '--- display-add call sites ---'
rg -n 'lvgl_display_add\\(' --glob '*.[ch]' --glob '*.cpp' . | head -100Repository: TactilityProject/Tactility
Length of output: 10815
Apply the border override only to monochrome displays.
LV_USE_THEME_MONO does not guarantee that lv_display_create() selects the mono theme. LVGL can select the default theme instead. The current call wraps whichever theme is active and sets button borders to zero. Because the call occurs before lv_display_set_color_format(), it can change buttons on color displays.
Set the color format first, then apply the extension only for LV_COLOR_FORMAT_I1.
Suggested fix
-#if LV_USE_THEME_MONO
- lvgl_display_extend_mono_theme(disp);
-#endif
-
ctx->render_mode = render_mode;
lv_display_set_color_format(disp, lv_color_format);
+#if LV_USE_THEME_MONO
+ if (lv_color_format == LV_COLOR_FORMAT_I1) {
+ lvgl_display_extend_mono_theme(disp);
+ }
+#endif1 participant
Summary by CodeRabbit