[PATCH 00/27] Expo debugging and textedit improvements (part E)
From: Simon Glass <simon.glass@canonical.com> This series adds debugging infrastructure for expo development and fixes several issues with textedit objects. Debugging improvements: - Add 'sb grid' command to overlay a grid on the sandbox display - Add file log driver for sandbox to capture log output to a file - Add scene_chklog() to filter log output by object name - Add per-line measurement logging for text objects - Add expo_dump_file() to dump expo structures to a file - Add line-measurement info to expo dump output Textedit fixes: - Extract common text-input code into scene_txtin.c - Fix textedit height calculation - Fix word-wrap measurement order to use correct bbox - Support highlighting textedit objects Trace fixes: - Fix proftool funcgraph depth for exit records - Fix trace test to handle more traced functions Also includes minor fixes for RTC test flakiness and pinctrl-single boot phase. Simon Glass (27): hook: Add hooks for kaka test/py: Allow --help without a build directory proftool: Fix funcgraph depth for exit records test: trace: Remove stale trace file before saving test: trace: Fix funcgraph check for matching exit indent pickman: Handle qconfig resync commits specially sandbox: test: Add bootph-all to pinctrl-single for serial test: dm: rtc: Fix flaky dm_test_rtc_dual test video: Log each measured line with log_content() sandbox: Add grid-drawing option to sandbox_sdl_sync() sandbox: Add 'sb grid' command to enable grid overlay sandbox: Allow grid size to be specified in 'sb grid' command sandbox: log: Add file log driver expo: Use a configurable output function in expo_dump() expo: Dump the line-measurement info expo: Add debug logging to scene_txtin_arrange() sandbox: expo: Add expo_dump_file() expo: Add scene_chklog() for filtered logging expo: Fix textedit word-wrap measurement order expo: Add per-line measurement logging expo: Add a txtin function to render dependencies expo: Combine textline and textedit switch cases expo: Add scene_txtin_open() for common text-input open code expo: Add scene_txtin_close() for common text-input close code expo: Mark textedits as highlightable expo: Fix textedit height calculation expo: Render textedit dependencies correctly arch/sandbox/cpu/sdl.c | 23 ++++++- arch/sandbox/dts/test.dts | 2 + arch/sandbox/include/asm/sdl.h | 8 ++- arch/sandbox/include/asm/sdl_sync.h | 23 +++++++ arch/sandbox/include/asm/state.h | 2 + boot/Kconfig | 10 +++ boot/expo_dump.c | 88 ++++++++++++++++++++++-- boot/scene.c | 68 ++++++++++++++---- boot/scene_internal.h | 62 +++++++++++------ boot/scene_textedit.c | 49 ++++++++++--- boot/scene_textline.c | 77 +-------------------- boot/scene_txtin.c | 88 ++++++++++++++++++++++++ cmd/sb.c | 17 +++++ common/Kconfig | 8 +++ common/Makefile | 1 + common/log_file.c | 103 ++++++++++++++++++++++++++++ configs/sandbox_defconfig | 2 + doc/develop/expo.rst | 31 +++++++++ doc/develop/logging.rst | 5 ++ doc/usage/cmd/sb.rst | 9 +++ drivers/video/console_truetype.c | 10 ++- drivers/video/sandbox_sdl.c | 7 +- include/expo.h | 11 ++- include/log.h | 11 +++ lib/getopt.c | 4 +- test/boot/expo.c | 47 ++++++++++++- test/cmd/sb.c | 34 +++++++++ test/dm/rtc.c | 19 +++-- test/hooks/bin/kaka | 1 + test/log/Makefile | 1 + test/log/log_file_test.c | 43 ++++++++++++ test/log/log_filter.c | 9 +-- test/py/conftest.py | 4 ++ test/py/tests/test_trace.py | 6 +- tools/pickman/agent.py | 44 +++++++++++- tools/proftool.c | 23 ++++--- 36 files changed, 789 insertions(+), 161 deletions(-) create mode 100644 arch/sandbox/include/asm/sdl_sync.h create mode 100644 common/log_file.c create mode 120000 test/hooks/bin/kaka create mode 100644 test/log/log_file_test.c -- 2.43.0 base-commit: 58e407261a63cd833a8395c2f6dc620814b56300 branch: expe
From: Simon Glass <simon.glass@canonical.com> Provide a symlink so that tests can run on kaka (a desktop machine). Signed-off-by: Simon Glass <simon.glass@canonical.com> --- test/hooks/bin/kaka | 1 + 1 file changed, 1 insertion(+) create mode 120000 test/hooks/bin/kaka diff --git a/test/hooks/bin/kaka b/test/hooks/bin/kaka new file mode 120000 index 00000000000..784d574a1e1 --- /dev/null +++ b/test/hooks/bin/kaka @@ -0,0 +1 @@ +ellesmere \ No newline at end of file -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Currently running './test/py/test.py --help' fails with an error about .config not existing. This is because pytest_configure() tries to parse the build configuration even when just showing help. Add an early return when config.option.help is set, so users can view the help text without needing a build directory. Co-developed-by: Claude <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- test/py/conftest.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/py/conftest.py b/test/py/conftest.py index bd0c226276a..8efbe0d01c6 100644 --- a/test/py/conftest.py +++ b/test/py/conftest.py @@ -239,6 +239,10 @@ def pytest_configure(config): Returns: Nothing. """ + # Skip full configuration when just showing help + if config.option.help: + return + def parse_config(conf_file): """Parse a config file, loading it into the ubconfig container -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> The funcgraph format requires entry and exit records for the same function to have matching depth values. Currently, exit records have their depth written before decrementing, causing trace-cmd to display incorrect or missing function exits. Move the depth decrement and stack handling to before writing the exit record. This ensures entry at depth N writes depth=N, and exit for the same function also writes depth=N. Fixes: b54d8cf0b5b8 ("trace: Support output of funcgraph records") Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/proftool.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/tools/proftool.c b/tools/proftool.c index c7b427f3078..1f7df6d1973 100644 --- a/tools/proftool.c +++ b/tools/proftool.c @@ -1435,6 +1435,21 @@ static int write_pages(struct twriter *tw, enum out_format_t out_format, tw->ptr += tputq(fout, text_offset + caller_func->offset); } else { + ulong func_duration = 0; + + /* + * For funcgraph, entry and exit of a function must + * have the same depth. Decrement before writing the + * exit record. + */ + if (!entry) { + depth--; + if (stack_ptr && stack_ptr <= MAX_STACK_DEPTH) { + ulong start = func_stack[--stack_ptr]; + + func_duration = timestamp - start; + } + } tw->ptr += tputl(fout, rec_words | delta << 5); tw->ptr += tputh(fout, entry ? TRACE_GRAPH_ENT : TRACE_GRAPH_RET); @@ -1449,14 +1464,6 @@ static int write_pages(struct twriter *tw, enum out_format_t out_format, func_stack[stack_ptr] = timestamp; stack_ptr++; } else { - ulong func_duration = 0; - - depth--; - if (stack_ptr && stack_ptr <= MAX_STACK_DEPTH) { - ulong start = func_stack[--stack_ptr]; - - func_duration = timestamp - start; - } tw->ptr += tputl(fout, 0); /* overrun */ tw->ptr += tputq(fout, 0); /* calltime */ /* rettime (nanoseconds) */ -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> The sandbox filesystem's host save command does not truncate existing files. If a previous test run created a larger trace file, the extra data remains when a smaller trace is written, causing proftool to fail with "Cannot read trace file" when it tries to read past the actual data. Fix this by removing any existing trace file before saving. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- test/py/tests/test_trace.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/py/tests/test_trace.py b/test/py/tests/test_trace.py index fcdcbe2c6db..0bddf408833 100644 --- a/test/py/tests/test_trace.py +++ b/test/py/tests/test_trace.py @@ -65,6 +65,8 @@ def collect_trace(ubman): out = ubman.run_command(f'trace calls {addr:x} {size:x}') print(out) fname = os.path.join(TMPDIR, 'trace') + if os.path.exists(fname): + os.unlink(fname) out = ubman.run_command( 'host save hostfs - %x %s ${profoffset}' % (addr, fname)) return fname, int(dm_f_time[0]) -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> The check_funcgraph() function looks for initf_bootstage() entry and its matching exit brace. Fix two issues: 1. The expected_indent should match the entry indent, not child level (indent + 2 spaces). Entry and exit braces are at the same level. 2. Increase head limit from 70 to 100 lines. With the log_file driver enabled, there are more traced function calls and the exit brace may appear beyond line 70. Fixes: 6acf4a242f27 ("proftool: Fix funcgraph depth for exit records") Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- test/py/tests/test_trace.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/py/tests/test_trace.py b/test/py/tests/test_trace.py index 0bddf408833..498949372aa 100644 --- a/test/py/tests/test_trace.py +++ b/test/py/tests/test_trace.py @@ -189,7 +189,7 @@ def check_funcgraph(ubman, fname, proftool, map_fname, trace_dat): 'dump-ftrace', '-f', 'funcgraph']) # Check that the trace has what we expect - cmd = f'trace-cmd report -l {trace_dat} |head -n 70' + cmd = f'trace-cmd report -l {trace_dat} |head -n 100' out = utils.run_and_log(ubman, ['sh', '-c', cmd]) # First look for this: @@ -221,7 +221,7 @@ def check_funcgraph(ubman, fname, proftool, map_fname, trace_dat): break elif func == 'initf_bootstage() ': found_start = True - expected_indent = indent + ' ' + expected_indent = indent elif found_start and indent == expected_indent and brace == '}': found_end = True -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Commits with subject "configs: Resync with savedefconfig" run savedefconfig on all boards. These cannot be cherry-picked directly because the defconfig state depends on the target branch's Kconfig options, not the source branch. Add detection for qconfig resync commits and instruct the agent to run ./tools/qconfig.py -s -Cy instead of cherry-picking. This regenerates the savedefconfig changes based on the current Kconfig state in the target branch. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/pickman/agent.py | 44 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 42 insertions(+), 2 deletions(-) diff --git a/tools/pickman/agent.py b/tools/pickman/agent.py index b081c36b504..589d61b727a 100644 --- a/tools/pickman/agent.py +++ b/tools/pickman/agent.py @@ -24,6 +24,12 @@ SIGNAL_SUCCESS = 'success' SIGNAL_APPLIED = 'already_applied' SIGNAL_CONFLICT = 'conflict' +# Commits that need special handling (regenerate instead of cherry-pick) +# These run savedefconfig on all boards and depend on target branch Kconfig state +QCONFIG_SUBJECTS = [ + 'configs: Resync with savedefconfig', +] + # Check if claude_agent_sdk is available try: from claude_agent_sdk import query, ClaudeAgentOptions @@ -45,6 +51,18 @@ def check_available(): return True +def is_qconfig_commit(subject): + """Check if a commit subject indicates a qconfig resync commit + + Args: + subject (str): Commit subject line + + Returns: + bool: True if this is a qconfig resync commit + """ + return any(subject.startswith(pat) for pat in QCONFIG_SUBJECTS) + + async def run(commits, source, branch_name, repo_path=None): # pylint: disable=too-many-locals """Run the Claude agent to cherry-pick commits @@ -70,13 +88,18 @@ async def run(commits, source, branch_name, repo_path=None): # pylint: disable= os.remove(signal_path) # Build commit list for the prompt, marking potentially applied commits + # and qconfig resync commits commit_entries = [] has_applied = False + has_qconfig = False for commit in commits: entry = f' - {commit.chash}: {commit.subject}' if commit.applied_as: entry += f' (maybe already applied as {commit.applied_as})' has_applied = True + if is_qconfig_commit(commit.subject): + entry += ' [QCONFIG RESYNC]' + has_qconfig = True commit_entries.append(entry) commit_list = '\n'.join(commit_entries) @@ -86,7 +109,7 @@ async def run(commits, source, branch_name, repo_path=None): # pylint: disable= if has_applied: applied_note = ''' -IMPORTANT: Some commits may already be applied. Before cherry-picking commits +IMPORTANT: Some commits may already be applied. Before cherry-picking commits marked as "maybe already applied as <hash>", verify they are truly the same commit: 1. Compare the actual changes between the original and found commits: - Use: git show --no-ext-diff <original-hash> > /tmp/orig.patch @@ -99,12 +122,29 @@ marked as "maybe already applied as <hash>", verify they are truly the same comm proceed with the cherry-pick as they are different commits. ''' + # Add note about qconfig resync commits + qconfig_note = '' + if has_qconfig: + qconfig_note = ''' + +IMPORTANT: Commits marked [QCONFIG RESYNC] need special handling. These commits +run savedefconfig on all boards and cannot be cherry-picked directly because the +defconfig state depends on the target branch's Kconfig options. + +For [QCONFIG RESYNC] commits, instead of cherry-picking: +1. Skip the cherry-pick for this commit +2. Run: ./tools/qconfig.py -s -Cy + This regenerates the savedefconfig changes based on the current Kconfig state +3. The qconfig.py tool will create a commit automatically with the updated defconfigs +4. Continue with the remaining commits after qconfig.py completes +''' + # Get full hash of last commit for signal file last_commit_hash = commits[-1].hash prompt = f"""Cherry-pick the following commits from {source} branch: -{commit_list}{applied_note} +{commit_list}{applied_note}{qconfig_note} Steps to follow: 1. First run 'git status' to check the repository state is clean -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> When running sandbox with -T (test mode), a warning appears: pinctrl_select_state_full() sandbox_serial serial: pinctrl_select_state_full: uclass_get_device_by_phandle_id: err=-19 This occurs because the serial device has bootph-all and is probed early, but the pinctrl-single node and its pinconfig child are not bound yet. Add bootph-all to the pinctrl-single-pins node and its pinmux_uart0_pins child so they are available when the serial device is probed. Co-developed-by: Claude <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- arch/sandbox/dts/test.dts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index 7a67565e4a9..7716140f2bf 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -1874,6 +1874,7 @@ #pinctrl-cells = <1>; pinctrl-single,register-width = <32>; pinctrl-single,function-mask = <0x7f>; + bootph-all; pinmux_pwm_pins: pinmux_pwm_pins { pinctrl-single,pins = < 0x48 0x06 >; @@ -1893,6 +1894,7 @@ 0x70 0x30 0x74 0x00 >; + bootph-all; }; }; -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> The test compares times from two RTC devices to verify they can operate independently with different offsets. However, it is flaky because now1 is read with use_system_time=true (using live os_localtime()) while cmp is read with use_system_time=false (using the fixed base_time captured at bind). If exactly 1 second elapses between emulator bind and test execution, the times accidentally match and the test fails. Fix this by: - Reading now1 after switching emul1 to manual mode, so both reads use the fixed base_time - Using a 10-second offset difference instead of 1 second, to account for the fact that each emulator has its own base_time which could differ slightly Also remove the unused now2 and offset variables. Co-developed-by: Claude <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- test/dm/rtc.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/test/dm/rtc.c b/test/dm/rtc.c index 88f080b64a8..b2687bde433 100644 --- a/test/dm/rtc.c +++ b/test/dm/rtc.c @@ -281,23 +281,30 @@ DM_TEST(dm_test_rtc_reset, UTF_SCAN_PDATA | UTF_SCAN_FDT); /* Check that two RTC devices can be used independently */ static int dm_test_rtc_dual(struct unit_test_state *uts) { - struct rtc_time now1, now2, cmp; + struct rtc_time now1, cmp; struct udevice *dev1, *dev2; struct udevice *emul1, *emul2; - long offset; ut_assertok(uclass_get_device(UCLASS_RTC, 0, &dev1)); - ut_assertok(dm_rtc_get(dev1, &now1)); ut_assertok(uclass_get_device(UCLASS_RTC, 1, &dev2)); - ut_assertok(dm_rtc_get(dev2, &now2)); ut_assertok(i2c_emul_find(dev1, &emul1)); ut_assertnonnull(emul1); ut_assertok(i2c_emul_find(dev2, &emul2)); ut_assertnonnull(emul2); - offset = sandbox_i2c_rtc_set_offset(emul1, false, -1); - sandbox_i2c_rtc_set_offset(emul2, false, offset + 1); + /* + * Put both emulators in manual mode before reading, to avoid a race + * with system time. With use_system_time=true, times are based on + * os_localtime() which advances with real time; with it false, they + * use the fixed base_time captured at bind. Use a large offset + * difference (10s) to account for the fact that each emulator has + * its own base_time, which could differ slightly. + */ + sandbox_i2c_rtc_set_offset(emul1, false, 0); + ut_assertok(dm_rtc_get(dev1, &now1)); + + sandbox_i2c_rtc_set_offset(emul2, false, 10); memset(&cmp, '\0', sizeof(cmp)); ut_assertok(dm_rtc_get(dev2, &cmp)); ut_asserteq(-EINVAL, cmp_times(&now1, &cmp, false)); -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Change log_debug() to log_content() for line measurement logging and add logging for the final line. This allows detailed line-by-line measurement output to be enabled separately from debug logging. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- drivers/video/console_truetype.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/video/console_truetype.c b/drivers/video/console_truetype.c index f73fb3e6595..98356de2fd2 100644 --- a/drivers/video/console_truetype.c +++ b/drivers/video/console_truetype.c @@ -1075,9 +1075,10 @@ static int truetype_measure(struct udevice *dev, const char *name, uint size, mline.len = (s - text) - start; if (lines && !alist_add(lines, mline)) return log_msg_ret("ttm", -ENOMEM); - log_debug("line x1 %d y0 %d y1 %d start %d len %d text '%.*s'\n", - mline.bbox.x1, mline.bbox.y0, mline.bbox.y1, - mline.start, mline.len, mline.len, text + mline.start); + log_content("line x1 %d y0 %d y1 %d start %d len %d text '%.*s'\n", + mline.bbox.x1, mline.bbox.y0, mline.bbox.y1, + mline.start, mline.len, mline.len, + text + mline.start); start = s - text; start++; @@ -1098,6 +1099,9 @@ static int truetype_measure(struct udevice *dev, const char *name, uint size, mline.len = (s - text) - start; if (lines && !alist_add(lines, mline)) return log_msg_ret("ttM", -ENOMEM); + log_content("line x1 %d y0 %d y1 %d start %d len %d text '%.*s'\n", + mline.bbox.x1, mline.bbox.y0, mline.bbox.y1, + mline.start, mline.len, mline.len, text + mline.start); bbox->valid = true; bbox->x0 = 0; -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Add an optional struct parameter to sandbox_sdl_sync() to control sync behaviour. Initially this supports drawing a 10-pixel grid overlay on the display, useful for debugging UI layout. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- arch/sandbox/cpu/sdl.c | 20 +++++++++++++++++++- arch/sandbox/include/asm/sdl.h | 8 ++++++-- arch/sandbox/include/asm/sdl_sync.h | 21 +++++++++++++++++++++ drivers/video/sandbox_sdl.c | 2 +- 4 files changed, 47 insertions(+), 4 deletions(-) create mode 100644 arch/sandbox/include/asm/sdl_sync.h diff --git a/arch/sandbox/cpu/sdl.c b/arch/sandbox/cpu/sdl.c index dd2589c64e2..4a98d920c9f 100644 --- a/arch/sandbox/cpu/sdl.c +++ b/arch/sandbox/cpu/sdl.c @@ -11,6 +11,7 @@ #include <sysreset.h> #include <linux/input.h> #include <SDL2/SDL.h> +#include <asm/sdl_sync.h> #include <asm/state.h> #include <video_defs.h> @@ -310,7 +311,21 @@ static int copy_to_texture(void *lcd_base, const struct vid_bbox *damage) return 0; } -int sandbox_sdl_sync(void *lcd_base, const struct vid_bbox *damage) +static void draw_grid(void) +{ + int x, y; + + SDL_SetRenderDrawColor(sdl.renderer, 192, 192, 192, SDL_ALPHA_OPAQUE); + + for (x = 0; x < sdl.vis_width; x += 10) + SDL_RenderDrawLine(sdl.renderer, x, 0, x, sdl.vis_height - 1); + + for (y = 0; y < sdl.vis_height; y += 10) + SDL_RenderDrawLine(sdl.renderer, 0, y, sdl.vis_width - 1, y); +} + +int sandbox_sdl_sync(void *lcd_base, const struct vid_bbox *damage, + const struct sandbox_sdl_sync_opts *opts) { struct SDL_Rect rect; int ret; @@ -329,6 +344,9 @@ int sandbox_sdl_sync(void *lcd_base, const struct vid_bbox *damage) return -EIO; } + if (opts && opts->draw_grid) + draw_grid(); + /* * On some machines this does not appear. Draw an empty rectangle which * seems to fix that. diff --git a/arch/sandbox/include/asm/sdl.h b/arch/sandbox/include/asm/sdl.h index a80db51ad19..b2965a0a700 100644 --- a/arch/sandbox/include/asm/sdl.h +++ b/arch/sandbox/include/asm/sdl.h @@ -8,6 +8,7 @@ #include <errno.h> #include <video.h> +#include <asm/sdl_sync.h> struct mouse_event; struct vid_bbox; @@ -44,9 +45,11 @@ int sandbox_sdl_remove_display(void); * * @lcd_base: Base of frame buffer * @damage: Optional damage rectangle to limit the update region (may be NULL) + * @opts: Optional sync options (may be NULL) * Return: 0 if screen was updated, -ENODEV is there is no screen. */ -int sandbox_sdl_sync(void *lcd_base, const struct vid_bbox *damage); +int sandbox_sdl_sync(void *lcd_base, const struct vid_bbox *damage, + const struct sandbox_sdl_sync_opts *opts); /** * sandbox_sdl_scan_keys() - scan for pressed keys @@ -139,7 +142,8 @@ static inline int sandbox_sdl_remove_display(void) } static inline int sandbox_sdl_sync(void *lcd_base, - const struct vid_bbox *damage) + const struct vid_bbox *damage, + const struct sandbox_sdl_sync_opts *opts) { return -ENODEV; } diff --git a/arch/sandbox/include/asm/sdl_sync.h b/arch/sandbox/include/asm/sdl_sync.h new file mode 100644 index 00000000000..78f4233e056 --- /dev/null +++ b/arch/sandbox/include/asm/sdl_sync.h @@ -0,0 +1,21 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * Copyright 2026 Canonical Ltd + * Written by Simon Glass <simon.glass@canonical.com> + */ + +#ifndef __SANDBOX_SDL_SYNC_H +#define __SANDBOX_SDL_SYNC_H + +#include <stdbool.h> + +/** + * struct sandbox_sdl_sync_opts - Options for sandbox_sdl_sync() + * + * @draw_grid: Draw a grid overlay on the display + */ +struct sandbox_sdl_sync_opts { + bool draw_grid; +}; + +#endif diff --git a/drivers/video/sandbox_sdl.c b/drivers/video/sandbox_sdl.c index 7954e266d98..2e2ebe14f84 100644 --- a/drivers/video/sandbox_sdl.c +++ b/drivers/video/sandbox_sdl.c @@ -146,7 +146,7 @@ static int sandbox_sdl_video_sync(struct udevice *vid, uint flags) memset(&plat->last_sync_damage, '\0', sizeof(plat->last_sync_damage)); - return sandbox_sdl_sync(uc_priv->fb, damage); + return sandbox_sdl_sync(uc_priv->fb, damage, NULL); } static const struct video_ops sandbox_sdl_ops = { -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Add a new 'sb grid <0|1>' command to enable or disable a grid overlay on the sandbox video display. This is useful for debugging UI layout and alignment. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- arch/sandbox/include/asm/state.h | 1 + cmd/sb.c | 15 +++++++++++++++ doc/usage/cmd/sb.rst | 8 ++++++++ drivers/video/sandbox_sdl.c | 6 +++++- test/cmd/sb.c | 23 +++++++++++++++++++++++ 5 files changed, 52 insertions(+), 1 deletion(-) diff --git a/arch/sandbox/include/asm/state.h b/arch/sandbox/include/asm/state.h index 1b6eef4a9c3..31766a6e7ea 100644 --- a/arch/sandbox/include/asm/state.h +++ b/arch/sandbox/include/asm/state.h @@ -179,6 +179,7 @@ struct sandbox_state { bool no_term_present; /* Assume no terminal present */ bool quiet_vidconsole; /* Don't use vidconsole for stdout */ bool disable_mcheck; /* Disable mcheck heap protection */ + bool show_grid; /* Show grid overlay on video */ int video_test; /* ms to wait before next assert */ const char *video_frames_dir; /* Directory to write video frames */ int video_frame_count; /* Number of frames written */ diff --git a/cmd/sb.c b/cmd/sb.c index 7de43e7edc3..55a4b5b447f 100644 --- a/cmd/sb.c +++ b/cmd/sb.c @@ -97,6 +97,19 @@ static int do_sb_devon(struct cmd_tbl *cmdtp, int flag, int argc, return CMD_RET_SUCCESS; } +static int do_sb_grid(struct cmd_tbl *cmdtp, int flag, int argc, + char *const argv[]) +{ + struct sandbox_state *state = state_get_current(); + + if (argc < 2) + return CMD_RET_USAGE; + + state->show_grid = hextoul(argv[1], NULL); + + return 0; +} + static int do_sb_devoff(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { @@ -147,6 +160,7 @@ static int do_sb_devoff(struct cmd_tbl *cmdtp, int flag, int argc, U_BOOT_LONGHELP(sb, "devoff <node> - Disable device from device tree node\n" "sb devon <node> - Enable device from device tree node\n" + "sb grid <0|1> - Enable/disable grid overlay on video\n" "sb handoff - Show handoff data received from SPL\n" "sb map - Show mapped memory\n" "sb state - Show sandbox state"); @@ -154,6 +168,7 @@ U_BOOT_LONGHELP(sb, U_BOOT_CMD_WITH_SUBCMDS(sb, "Sandbox status commands", sb_help_text, U_BOOT_SUBCMD_MKENT(devoff, 2, 1, do_sb_devoff), U_BOOT_SUBCMD_MKENT(devon, 2, 1, do_sb_devon), + U_BOOT_SUBCMD_MKENT(grid, 2, 1, do_sb_grid), U_BOOT_SUBCMD_MKENT(handoff, 1, 1, do_sb_handoff), U_BOOT_SUBCMD_MKENT(map, 1, 1, do_sb_map), U_BOOT_SUBCMD_MKENT(state, 1, 1, do_sb_state)); diff --git a/doc/usage/cmd/sb.rst b/doc/usage/cmd/sb.rst index b908631dc08..ee72ecd0db9 100644 --- a/doc/usage/cmd/sb.rst +++ b/doc/usage/cmd/sb.rst @@ -13,6 +13,7 @@ Synopsis sb devoff <node> sb devon <node> + sb grid <0|1> sb handoff sb map sb state @@ -40,6 +41,13 @@ devices that are not automatically bound at startup, i.e. those marked as status = "disabled" in the device tree. The parameter is the name of a root devicetree node. +sb grid +~~~~~~~ + +This enables or disables a grid overlay on the video display. When enabled, +a 10-pixel grid is drawn over the display, which is useful for debugging UI +layout and alignment. Use ``sb grid 1`` to enable and ``sb grid 0`` to disable. + sb handoff ~~~~~~~~~~ diff --git a/drivers/video/sandbox_sdl.c b/drivers/video/sandbox_sdl.c index 2e2ebe14f84..4785185dd70 100644 --- a/drivers/video/sandbox_sdl.c +++ b/drivers/video/sandbox_sdl.c @@ -131,6 +131,8 @@ static int sandbox_sdl_video_sync(struct udevice *vid, uint flags) { struct sandbox_sdl_plat *plat = dev_get_plat(vid); struct video_priv *uc_priv = dev_get_uclass_priv(vid); + struct sandbox_state *state = state_get_current(); + struct sandbox_sdl_sync_opts opts; const struct vid_bbox *damage = NULL; if (!(flags & VIDSYNC_FLUSH)) @@ -146,7 +148,9 @@ static int sandbox_sdl_video_sync(struct udevice *vid, uint flags) memset(&plat->last_sync_damage, '\0', sizeof(plat->last_sync_damage)); - return sandbox_sdl_sync(uc_priv->fb, damage, NULL); + opts.draw_grid = state->show_grid; + + return sandbox_sdl_sync(uc_priv->fb, damage, &opts); } static const struct video_ops sandbox_sdl_ops = { diff --git a/test/cmd/sb.c b/test/cmd/sb.c index cb871dffd57..4ce8a8c4215 100644 --- a/test/cmd/sb.c +++ b/test/cmd/sb.c @@ -6,6 +6,7 @@ */ #include <dm.h> +#include <asm/state.h> #include <dm/test.h> #include <test/test.h> #include <test/ut.h> @@ -121,3 +122,25 @@ static int dm_test_sb_devoff_not_bound(struct unit_test_state *uts) return 0; } DM_TEST(dm_test_sb_devoff_not_bound, UTF_SCAN_FDT | UTF_CONSOLE); + +/* Test 'sb grid' command */ +static int dm_test_sb_grid(struct unit_test_state *uts) +{ + struct sandbox_state *state = state_get_current(); + + /* Ensure grid is initially off */ + state->show_grid = false; + + /* Enable grid */ + ut_assertok(run_command("sb grid 1", 0)); + ut_assert_console_end(); + ut_asserteq(true, state->show_grid); + + /* Disable grid */ + ut_assertok(run_command("sb grid 0", 0)); + ut_assert_console_end(); + ut_asserteq(false, state->show_grid); + + return 0; +} +DM_TEST(dm_test_sb_grid, UTF_CONSOLE); -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Add an optional second parameter to 'sb grid' to specify the grid size in pixels. For example, 'sb grid 1 14' enables a 20-pixel (0x14) grid. The default remains 10 pixels when no size is specified. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- arch/sandbox/cpu/sdl.c | 11 +++++++---- arch/sandbox/include/asm/sdl_sync.h | 2 ++ arch/sandbox/include/asm/state.h | 1 + cmd/sb.c | 6 ++++-- doc/develop/expo.rst | 4 ++++ doc/usage/cmd/sb.rst | 7 ++++--- drivers/video/sandbox_sdl.c | 1 + test/cmd/sb.c | 11 +++++++++++ 8 files changed, 34 insertions(+), 9 deletions(-) diff --git a/arch/sandbox/cpu/sdl.c b/arch/sandbox/cpu/sdl.c index 4a98d920c9f..f2cc4bbd041 100644 --- a/arch/sandbox/cpu/sdl.c +++ b/arch/sandbox/cpu/sdl.c @@ -311,16 +311,19 @@ static int copy_to_texture(void *lcd_base, const struct vid_bbox *damage) return 0; } -static void draw_grid(void) +static void draw_grid(int size) { int x, y; + if (!size) + size = 0x20; + SDL_SetRenderDrawColor(sdl.renderer, 192, 192, 192, SDL_ALPHA_OPAQUE); - for (x = 0; x < sdl.vis_width; x += 10) + for (x = 0; x < sdl.vis_width; x += size) SDL_RenderDrawLine(sdl.renderer, x, 0, x, sdl.vis_height - 1); - for (y = 0; y < sdl.vis_height; y += 10) + for (y = 0; y < sdl.vis_height; y += size) SDL_RenderDrawLine(sdl.renderer, 0, y, sdl.vis_width - 1, y); } @@ -345,7 +348,7 @@ int sandbox_sdl_sync(void *lcd_base, const struct vid_bbox *damage, } if (opts && opts->draw_grid) - draw_grid(); + draw_grid(opts->grid_size); /* * On some machines this does not appear. Draw an empty rectangle which diff --git a/arch/sandbox/include/asm/sdl_sync.h b/arch/sandbox/include/asm/sdl_sync.h index 78f4233e056..81b28ff61e5 100644 --- a/arch/sandbox/include/asm/sdl_sync.h +++ b/arch/sandbox/include/asm/sdl_sync.h @@ -13,9 +13,11 @@ * struct sandbox_sdl_sync_opts - Options for sandbox_sdl_sync() * * @draw_grid: Draw a grid overlay on the display + * @grid_size: Grid size in pixels (0 for default of 0x20) */ struct sandbox_sdl_sync_opts { bool draw_grid; + int grid_size; }; #endif diff --git a/arch/sandbox/include/asm/state.h b/arch/sandbox/include/asm/state.h index 31766a6e7ea..001d780aec8 100644 --- a/arch/sandbox/include/asm/state.h +++ b/arch/sandbox/include/asm/state.h @@ -180,6 +180,7 @@ struct sandbox_state { bool quiet_vidconsole; /* Don't use vidconsole for stdout */ bool disable_mcheck; /* Disable mcheck heap protection */ bool show_grid; /* Show grid overlay on video */ + int grid_size; /* Grid size in pixels (0 for default) */ int video_test; /* ms to wait before next assert */ const char *video_frames_dir; /* Directory to write video frames */ int video_frame_count; /* Number of frames written */ diff --git a/cmd/sb.c b/cmd/sb.c index 55a4b5b447f..dcf7ec9d0b4 100644 --- a/cmd/sb.c +++ b/cmd/sb.c @@ -106,6 +106,8 @@ static int do_sb_grid(struct cmd_tbl *cmdtp, int flag, int argc, return CMD_RET_USAGE; state->show_grid = hextoul(argv[1], NULL); + if (argc >= 3) + state->grid_size = hextoul(argv[2], NULL); return 0; } @@ -160,7 +162,7 @@ static int do_sb_devoff(struct cmd_tbl *cmdtp, int flag, int argc, U_BOOT_LONGHELP(sb, "devoff <node> - Disable device from device tree node\n" "sb devon <node> - Enable device from device tree node\n" - "sb grid <0|1> - Enable/disable grid overlay on video\n" + "sb grid <0|1> [<size>] - Enable/disable grid overlay on video\n" "sb handoff - Show handoff data received from SPL\n" "sb map - Show mapped memory\n" "sb state - Show sandbox state"); @@ -168,7 +170,7 @@ U_BOOT_LONGHELP(sb, U_BOOT_CMD_WITH_SUBCMDS(sb, "Sandbox status commands", sb_help_text, U_BOOT_SUBCMD_MKENT(devoff, 2, 1, do_sb_devoff), U_BOOT_SUBCMD_MKENT(devon, 2, 1, do_sb_devon), - U_BOOT_SUBCMD_MKENT(grid, 2, 1, do_sb_grid), + U_BOOT_SUBCMD_MKENT(grid, 3, 1, do_sb_grid), U_BOOT_SUBCMD_MKENT(handoff, 1, 1, do_sb_handoff), U_BOOT_SUBCMD_MKENT(map, 1, 1, do_sb_map), U_BOOT_SUBCMD_MKENT(state, 1, 1, do_sb_state)); diff --git a/doc/develop/expo.rst b/doc/develop/expo.rst index ca83b403621..fc642ed3696 100644 --- a/doc/develop/expo.rst +++ b/doc/develop/expo.rst @@ -797,6 +797,10 @@ For example, to watch an expo test render with a visible display:: ./u-boot -T -l -V 500 --video_frames /tmp/good -c "ut bootstd expo_render_image" +The :doc:`../usage/cmd/sb` ``grid`` subcommand can be used to overlay a grid on +the display, to help with checking alignment of objects. The grid size defaults +to 0x20 pixels but can be specified as a parameter. + This will write each asserted expo frame to ``/tmp/good/frame0.bmp``, ``/tmp/good/frame1.bmp``, etc. diff --git a/doc/usage/cmd/sb.rst b/doc/usage/cmd/sb.rst index ee72ecd0db9..08f8f87f88f 100644 --- a/doc/usage/cmd/sb.rst +++ b/doc/usage/cmd/sb.rst @@ -13,7 +13,7 @@ Synopsis sb devoff <node> sb devon <node> - sb grid <0|1> + sb grid <0|1> [<size>] sb handoff sb map sb state @@ -45,8 +45,9 @@ sb grid ~~~~~~~ This enables or disables a grid overlay on the video display. When enabled, -a 10-pixel grid is drawn over the display, which is useful for debugging UI -layout and alignment. Use ``sb grid 1`` to enable and ``sb grid 0`` to disable. +a grid is drawn over the display, which is useful for debugging UI layout and +alignment. Use ``sb grid 1`` to enable and ``sb grid 0`` to disable. An +optional second parameter specifies the grid size in pixels (default 0x20). sb handoff ~~~~~~~~~~ diff --git a/drivers/video/sandbox_sdl.c b/drivers/video/sandbox_sdl.c index 4785185dd70..6fc71bc9cb9 100644 --- a/drivers/video/sandbox_sdl.c +++ b/drivers/video/sandbox_sdl.c @@ -149,6 +149,7 @@ static int sandbox_sdl_video_sync(struct udevice *vid, uint flags) sizeof(plat->last_sync_damage)); opts.draw_grid = state->show_grid; + opts.grid_size = state->grid_size; return sandbox_sdl_sync(uc_priv->fb, damage, &opts); } diff --git a/test/cmd/sb.c b/test/cmd/sb.c index 4ce8a8c4215..b1fbddac449 100644 --- a/test/cmd/sb.c +++ b/test/cmd/sb.c @@ -130,6 +130,7 @@ static int dm_test_sb_grid(struct unit_test_state *uts) /* Ensure grid is initially off */ state->show_grid = false; + state->grid_size = 0; /* Enable grid */ ut_assertok(run_command("sb grid 1", 0)); @@ -141,6 +142,16 @@ static int dm_test_sb_grid(struct unit_test_state *uts) ut_assert_console_end(); ut_asserteq(false, state->show_grid); + /* Enable grid with custom size (0x14 = 20 decimal) */ + ut_assertok(run_command("sb grid 1 14", 0)); + ut_assert_console_end(); + ut_asserteq(true, state->show_grid); + ut_asserteq(0x14, state->grid_size); + + /* Clean up */ + state->show_grid = false; + state->grid_size = 0; + return 0; } DM_TEST(dm_test_sb_grid, UTF_CONSOLE); -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Add a new log driver that writes log records to a file. This is useful for capturing log output during testing or debugging in sandbox. The filename can be set via the 'log_file' environment variable, or programmatically using log_file_set_fname(). Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- common/Kconfig | 8 +++ common/Makefile | 1 + common/log_file.c | 103 ++++++++++++++++++++++++++++++++++++++ configs/sandbox_defconfig | 2 + doc/develop/logging.rst | 5 ++ include/log.h | 11 ++++ lib/getopt.c | 4 +- test/log/Makefile | 1 + test/log/log_file_test.c | 43 ++++++++++++++++ test/log/log_filter.c | 9 ++-- 10 files changed, 181 insertions(+), 6 deletions(-) create mode 100644 common/log_file.c create mode 100644 test/log/log_file_test.c diff --git a/common/Kconfig b/common/Kconfig index 597bea70b9b..8d3bda6f588 100644 --- a/common/Kconfig +++ b/common/Kconfig @@ -453,6 +453,14 @@ config LOG_SYSLOG Enables a log driver which broadcasts log records via UDP port 514 to syslog servers. +config LOG_FILE + bool "Log output to a file (sandbox only)" + depends on SANDBOX + help + Enables a log driver which writes log records to a file. Set the + 'log_file' environment variable to the filename to use, or call + log_file_set_fname() to set it programmatically. + config SPL_LOG bool "Enable logging support in SPL" depends on LOG && SPL diff --git a/common/Makefile b/common/Makefile index fdf4cff94f4..125f768ef53 100644 --- a/common/Makefile +++ b/common/Makefile @@ -97,6 +97,7 @@ obj-$(CONFIG_DFU_OVER_USB) += dfu.o obj-y += command.o obj-$(CONFIG_$(PHASE_)LOG) += log.o obj-$(CONFIG_$(PHASE_)LOG_CONSOLE) += log_console.o +obj-$(CONFIG_LOG_FILE) += log_file.o obj-$(CONFIG_$(PHASE_)LOG_SYSLOG) += log_syslog.o obj-y += s_record.o obj-$(CONFIG_CMD_LOADB) += xyzModem.o diff --git a/common/log_file.c b/common/log_file.c new file mode 100644 index 00000000000..6f445a9d561 --- /dev/null +++ b/common/log_file.c @@ -0,0 +1,103 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Log driver to write to a file (sandbox only) + * + * Copyright 2026 Canonical Ltd + * Written by Simon Glass <simon.glass@canonical.com> + */ + +#include <env.h> +#include <log.h> +#include <os.h> +#include <asm/global_data.h> + +DECLARE_GLOBAL_DATA_PTR; + +static int log_fd = -1; + +static void append(char **buf, char *buf_end, const char *fmt, ...) +{ + va_list args; + size_t size = buf_end - *buf; + + va_start(args, fmt); + vsnprintf(*buf, size, fmt, args); + va_end(args); + *buf += strlen(*buf); +} + +int log_file_set_fname(const char *fname) +{ + if (log_fd != -1) { + os_close(log_fd); + log_fd = -1; + } + + if (!fname) + return 0; + + log_fd = os_open(fname, OS_O_WRONLY | OS_O_CREAT | OS_O_TRUNC); + if (log_fd < 0) + return log_fd; + + return 0; +} + +static int log_file_emit(struct log_device *ldev, struct log_rec *rec) +{ + int fmt = gd->log_fmt; + char buf[512]; + char *buf_end = buf + sizeof(buf); + char *ptr = buf; + const char *fname; + int len; + + /* If no file open, try to open one from the environment */ + if (log_fd == -1) { + fname = env_get("log_file"); + if (!fname) + return 0; + + log_fd = os_open(fname, OS_O_WRONLY | OS_O_CREAT | OS_O_TRUNC); + if (log_fd < 0) + return 0; + } + + /* + * The output format is designed to give someone a fighting chance of + * figuring out which field is which: + * - level is in CAPS + * - cat is lower case and ends with comma + * - file normally has a .c extension and ends with a colon + * - line is integer and ends with a - + * - function is an identifier and ends with () + * - message has a space before it unless it is on its own + */ + if (!(rec->flags & LOGRECF_CONT) && fmt != BIT(LOGF_MSG)) { + if (fmt & BIT(LOGF_LEVEL)) + append(&ptr, buf_end, "%s.", + log_get_level_name(rec->level)); + if (fmt & BIT(LOGF_CAT)) + append(&ptr, buf_end, "%s,", + log_get_cat_name(rec->cat)); + if (fmt & BIT(LOGF_FILE)) + append(&ptr, buf_end, "%s:", rec->file); + if (fmt & BIT(LOGF_LINE)) + append(&ptr, buf_end, "%d-", rec->line); + if (fmt & BIT(LOGF_FUNC)) + append(&ptr, buf_end, "%s() ", rec->func ?: "?"); + } + if (fmt & BIT(LOGF_MSG)) + append(&ptr, buf_end, "%s", rec->msg); + + len = ptr - buf; + os_write(log_fd, buf, len); + + return 0; +} + +LOG_DRIVER(file) = { + .name = "file", + .emit = log_file_emit, + .flags = LOGDF_ENABLE, +}; diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index 4dbb77abcff..93ef88d597e 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -57,6 +57,7 @@ CONFIG_LOG=y CONFIG_LOG_MAX_LEVEL=9 CONFIG_LOG_DEFAULT_LEVEL=6 CONFIG_LOGF_FUNC=y +CONFIG_LOG_FILE=y CONFIG_DISPLAY_BOARDINFO_LATE=y CONFIG_STACKPROTECTOR=y CONFIG_ANDROID_AB=y @@ -149,6 +150,7 @@ CONFIG_CMD_EROFS=y CONFIG_CMD_EXT4_WRITE=y CONFIG_CMD_SQUASHFS=y CONFIG_CMD_MTDPARTS=y +CONFIG_CMD_LOG=y CONFIG_CMD_STACKPROTECTOR_TEST=y CONFIG_MAC_PARTITION=y CONFIG_OF_CONTROL=y diff --git a/doc/develop/logging.rst b/doc/develop/logging.rst index d7a40c94bf0..6314cae380c 100644 --- a/doc/develop/logging.rst +++ b/doc/develop/logging.rst @@ -172,10 +172,15 @@ enabled or disabled independently: * console - goes to stdout * syslog - broadcast RFC 3164 messages to syslog servers on UDP port 514 +* file - writes to a file (sandbox only) The syslog driver sends the value of environmental variable 'log_hostname' as HOSTNAME if available. +The file driver sends log records to a file and is only available in sandbox. +Set the 'log_file' environment variable to specify the filename, or call +log_file_set_fname() to set it programmatically. + Filters ------- diff --git a/include/log.h b/include/log.h index 8e933071cf1..c79042f1a5c 100644 --- a/include/log.h +++ b/include/log.h @@ -675,6 +675,17 @@ int log_remove_filter(const char *drv_name, int filter_num); */ int log_device_set_enable(struct log_driver *drv, bool enable); +/** + * log_file_set_fname() - Set the filename for the file log driver + * + * This sets or changes the file used by the file log driver. If a file is + * already open it is closed first. + * + * @fname: Filename to use, or NULL to close any existing file + * Return: 0 if OK, -ve on error + */ +int log_file_set_fname(const char *fname); + #if CONFIG_IS_ENABLED(LOG) /** * log_init() - Set up the log system ready for use diff --git a/lib/getopt.c b/lib/getopt.c index e9175e2fff4..c7bb6d3671a 100644 --- a/lib/getopt.c +++ b/lib/getopt.c @@ -25,8 +25,8 @@ int __getopt(struct getopt_state *gs, int argc, char *const argv[], const char *curoptp; /* pointer to the current option in optstring */ while (1) { - log_debug("arg_index: %d index: %d\n", gs->arg_index, - gs->index); + log_content("arg_index: %d index: %d\n", gs->arg_index, + gs->index); /* `--` indicates the end of options */ if (gs->arg_index == 1 && argv[gs->index] && diff --git a/test/log/Makefile b/test/log/Makefile index 24b7c46786d..6fc45d59a16 100644 --- a/test/log/Makefile +++ b/test/log/Makefile @@ -10,6 +10,7 @@ ifdef CONFIG_UT_LOG ifdef CONFIG_SANDBOX obj-$(CONFIG_LOG_SYSLOG) += syslog_test.o obj-$(CONFIG_LOG_SYSLOG) += syslog_test_ndebug.o +obj-$(CONFIG_LOG_FILE) += log_file_test.o endif ifdef CONFIG_LOG diff --git a/test/log/log_file_test.c b/test/log/log_file_test.c new file mode 100644 index 00000000000..e1d6b7dedf1 --- /dev/null +++ b/test/log/log_file_test.c @@ -0,0 +1,43 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright 2026 Canonical Ltd + * Written by Simon Glass <simon.glass@canonical.com> + * + * Test for log file driver + */ + +#include <command.h> +#include <log.h> +#include <os.h> +#include <test/log.h> +#include <test/ut.h> + +/* Test that the log_file driver can write to a file */ +static int log_test_file_driver(struct unit_test_state *uts) +{ + const char *fname = "/tmp/log_test.log"; + void *buf; + int size; + + ut_assertok(log_file_set_fname(fname)); + + /* Generate some log messages using log rec command */ + run_command("log format Lfm", 0); + run_command("log rec none warning test.c 123 my_func 'Test message'", 0); + run_command("log rec none err error.c 456 err_func 'Error occurred'", 0); + + /* Close the file so we can read it */ + ut_assertok(log_file_set_fname(NULL)); + + /* Read the file contents */ + ut_assertok(os_read_file(fname, &buf, &size)); + + /* Check the contents */ + ut_asserteq_strn("123-my_func() Test message\n", buf); + ut_assertnonnull(strstr(buf, "456-err_func() Error occurred\n")); + + os_free(buf); + + return 0; +} +LOG_TEST(log_test_file_driver); diff --git a/test/log/log_filter.c b/test/log/log_filter.c index 8622dcf2913..05ff4e94bd2 100644 --- a/test/log/log_filter.c +++ b/test/log/log_filter.c @@ -39,11 +39,12 @@ static int log_test_filter(struct unit_test_state *uts) #define create_filter(args, filter_num) do {\ ut_assertok(run_command("log filter-add -p " args, 0)); \ + console_record_readline(uts->actual_str, sizeof(uts->actual_str)); \ ut_assertok(strict_strtoul(uts->actual_str, 10, &(filter_num))); \ ut_assert_console_end(); \ } while (0) - create_filter("", filt1); + create_filter("-l info", filt1); create_filter("-DL warning -cmmc -cspi -ffile", filt2); ldev = log_device_find_by_name("console"); @@ -52,7 +53,7 @@ static int log_test_filter(struct unit_test_state *uts) if (filt->filter_num == filt1) { filt1_found = true; ut_asserteq(0, filt->flags); - ut_asserteq(LOGL_MAX, filt->level); + ut_asserteq(LOGL_INFO, filt->level); ut_assertnull(filt->file_list); } else if (filt->filter_num == filt2) { filt2_found = true; @@ -89,8 +90,8 @@ static int log_test_filter(struct unit_test_state *uts) ut_asserteq(false, filt1_found); ut_asserteq(false, filt2_found); - create_filter("", filt1); - create_filter("", filt2); + create_filter("-l info", filt1); + create_filter("-l info", filt2); ut_assertok(run_command("log filter-remove -a", 0)); ut_assert_console_end(); -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Refactor the dump context to use a configurable output function instead of directly writing to a membuf. This allows different output targets to be used. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/expo_dump.c | 44 +++++++++++++++++++++++++++++++++++++------- 1 file changed, 37 insertions(+), 7 deletions(-) diff --git a/boot/expo_dump.c b/boot/expo_dump.c index b2ea60db50c..d357aa6a5be 100644 --- a/boot/expo_dump.c +++ b/boot/expo_dump.c @@ -15,36 +15,64 @@ #include <video.h> #include "scene_internal.h" +/** + * typedef dump_out_func - Function type for dump output + * + * @priv: Private data for the output function + * @buf: Buffer containing data to output + * @len: Length of data in buffer + */ +typedef void (*dump_out_func)(void *priv, const char *buf, int len); + /** * struct dump_ctx - Context for dumping expo structures * - * @mb: Membuf to write output to + * @out: Output function to call for each piece of output + * @priv: Private data to pass to output function * @scn: Current scene being dumped (or NULL if not in a scene) * @indent: Current indentation level (number of spaces) */ struct dump_ctx { - struct membuf *mb; + dump_out_func out; + void *priv; struct scene *scn; int indent; }; +/** + * dump_out_membuf() - Output function that writes to a membuf + * + * @priv: Pointer to struct membuf + * @buf: Buffer containing data to output + * @len: Length of data in buffer + */ +static void dump_out_membuf(void *priv, const char *buf, int len) +{ + struct membuf *mb = priv; + + membuf_put(mb, buf, len); +} + /** * outf() - Output a formatted string with indentation * - * @ctx: Dump context containing membuf, scene, and indent level + * @ctx: Dump context containing output function, scene, and indent level * @fmt: Format string * @...: Arguments for format string */ static void outf(struct dump_ctx *ctx, const char *fmt, ...) { char buf[256]; + char indent_buf[64]; va_list args; int len; + len = snprintf(indent_buf, sizeof(indent_buf), "%*s", ctx->indent, ""); + ctx->out(ctx->priv, indent_buf, len); + va_start(args, fmt); - membuf_printf(ctx->mb, "%*s", ctx->indent, ""); len = vsnprintf(buf, sizeof(buf), fmt, args); - membuf_put(ctx->mb, buf, len); + ctx->out(ctx->priv, buf, len); va_end(args); } @@ -196,7 +224,8 @@ void scene_dump(struct membuf *mb, struct scene *scn, int indent) { struct dump_ctx ctx; - ctx.mb = mb; + ctx.out = dump_out_membuf; + ctx.priv = mb; ctx.scn = scn; ctx.indent = indent; @@ -262,7 +291,8 @@ void expo_dump(struct expo *exp, struct membuf *mb) { struct dump_ctx ctx; - ctx.mb = mb; + ctx.out = dump_out_membuf; + ctx.priv = mb; ctx.scn = NULL; ctx.indent = 0; -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Add mline (measurement line) information to the text object dump, showing the start offset, length, and bounding box for each line. This helps debug text layout and word-wrapping issues. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/expo_dump.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/boot/expo_dump.c b/boot/expo_dump.c index d357aa6a5be..cb0a1e443e3 100644 --- a/boot/expo_dump.c +++ b/boot/expo_dump.c @@ -13,6 +13,7 @@ #include <mapmem.h> #include <membuf.h> #include <video.h> +#include <video_console.h> #include "scene_internal.h" /** @@ -111,6 +112,7 @@ static void dump_menu(struct dump_ctx *ctx, struct scene_obj_menu *menu) static void dump_text(struct dump_ctx *ctx, struct scene_obj_txt *txt) { const char *str = expo_get_str(ctx->scn->expo, txt->gen.str_id); + const struct vidconsole_mline *mline; outf(ctx, "Text: str_id %x font_name '%s' font_size %x\n", txt->gen.str_id, @@ -118,6 +120,12 @@ static void dump_text(struct dump_ctx *ctx, struct scene_obj_txt *txt) txt->gen.font_size); ctx->indent += 2; outf(ctx, "str '%s'\n", str ? str : "(null)"); + alist_for_each(mline, &txt->gen.lines) { + outf(ctx, "mline: start %x len %x bbox (%x,%x)-(%x,%x)\n", + mline->start, mline->len, + mline->bbox.x0, mline->bbox.y0, + mline->bbox.x1, mline->bbox.y1); + } ctx->indent -= 2; } -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Add logging to show arr->label_width and margin values used when arranging text-input objects. This helps debug alignment issues. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/scene_txtin.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/boot/scene_txtin.c b/boot/scene_txtin.c index dbd2555f71d..99fad16c11f 100644 --- a/boot/scene_txtin.c +++ b/boot/scene_txtin.c @@ -42,6 +42,10 @@ int scene_txtin_arrange(struct scene *scn, struct expo_arrange_info *arr, if (ret < 0) return log_msg_ret("lab", ret); + if (scene_chklog(obj->name)) + log_debug("arr->label_width %d margin %d\n", + arr->label_width, + theme->textline_label_margin_x); x += arr->label_width + theme->textline_label_margin_x; } -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Add a function to dump expo structures to a file, which is useful for debugging. This uses the configurable output function added in the previous patch. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/expo_dump.c | 36 ++++++++++++++++++++++++++++++++++++ include/expo.h | 9 +++++++++ 2 files changed, 45 insertions(+) diff --git a/boot/expo_dump.c b/boot/expo_dump.c index cb0a1e443e3..940a505b313 100644 --- a/boot/expo_dump.c +++ b/boot/expo_dump.c @@ -12,6 +12,7 @@ #include <expo.h> #include <mapmem.h> #include <membuf.h> +#include <os.h> #include <video.h> #include <video_console.h> #include "scene_internal.h" @@ -306,3 +307,38 @@ void expo_dump(struct expo *exp, struct membuf *mb) expo_dump_(&ctx, exp); } + +/** + * dump_out_file() - Output function that writes to a file + * + * @priv: Pointer to file descriptor (int *) + * @buf: Buffer containing data to output + * @len: Length of data in buffer + */ +static void dump_out_file(void *priv, const char *buf, int len) +{ + int *fdp = priv; + + os_write(*fdp, buf, len); +} + +int expo_dump_file(struct expo *exp, const char *fname) +{ + struct dump_ctx ctx; + int fd; + + fd = os_open(fname, OS_O_WRONLY | OS_O_CREAT | OS_O_TRUNC); + if (fd < 0) + return fd; + + ctx.out = dump_out_file; + ctx.priv = &fd; + ctx.scn = NULL; + ctx.indent = 0; + + expo_dump_(&ctx, exp); + + os_close(fd); + + return 0; +} diff --git a/include/expo.h b/include/expo.h index e3451d8dd23..c854d78255b 100644 --- a/include/expo.h +++ b/include/expo.h @@ -1279,6 +1279,15 @@ void expo_damage_add(struct expo *exp, const struct vid_bbox *bbox); */ void expo_dump(struct expo *exp, struct membuf *mb); +/** + * expo_dump_file() - Dump expo structure to a file (sandbox only) + * + * @exp: Expo to dump + * @fname: Filename to write to + * Return: 0 if OK, -ve on error + */ +int expo_dump_file(struct expo *exp, const char *fname); + /** * scene_dump() - Dump scene structure to a membuf * -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Add a helper function that checks the 'expo_log_filter' environment variable. If not set, all objects are logged. If set, it contains a comma-separated list of filters; only objects whose name contains one of the filter strings are logged. The feature is controlled by CONFIG_EXPO_LOG_FILTER, which is enabled by default for sandbox. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/Kconfig | 10 ++++++++++ boot/scene.c | 32 ++++++++++++++++++++++++++++++++ boot/scene_internal.h | 12 ++++++++++++ doc/develop/expo.rst | 27 +++++++++++++++++++++++++++ test/boot/expo.c | 37 +++++++++++++++++++++++++++++++++++++ 5 files changed, 118 insertions(+) diff --git a/boot/Kconfig b/boot/Kconfig index 7ff0dedb748..14b7ed573c9 100644 --- a/boot/Kconfig +++ b/boot/Kconfig @@ -1002,6 +1002,16 @@ config EXPO_TEST variable is set to 1. This is useful for debugging and performance analysis. +config EXPO_LOG_FILTER + bool "Enable expo log filter" + depends on EXPO + default y if SANDBOX + help + Enable the expo log filter. When enabled, the 'expo_log_filter' + environment variable can be set to filter log output by object name. + Only objects whose name contains the filter string are logged. This + is useful for debugging specific expo objects. + config BOOTMETH_SANDBOX def_bool y depends on SANDBOX diff --git a/boot/scene.c b/boot/scene.c index 3565bfd77de..9cade9aad41 100644 --- a/boot/scene.c +++ b/boot/scene.c @@ -10,6 +10,7 @@ #include <alist.h> #include <dm.h> +#include <env.h> #include <expo.h> #include <malloc.h> #include <mapmem.h> @@ -42,6 +43,37 @@ static const char *const scene_obj_type_names[] = { "textline", }; +bool scene_chklog(const char *name) +{ + const char *filter, *end, *p; + int len; + + if (!CONFIG_IS_ENABLED(EXPO_LOG_FILTER)) + return true; + + filter = env_get("expo_log_filter"); + if (!filter) + return true; + + /* Check each comma-separated filter */ + while (*filter) { + end = strchrnul(filter, ','); + len = end - filter; + + /* Check if this filter segment appears in name */ + for (p = name; *p; p++) { + if (!strncmp(p, filter, len)) + return true; + } + + if (!*end) + break; + filter = end + 1; + } + + return false; +} + int scene_new(struct expo *exp, const char *name, uint id, struct scene **scnp) { struct scene *scn; diff --git a/boot/scene_internal.h b/boot/scene_internal.h index db11f9c0f60..d3c67777cb1 100644 --- a/boot/scene_internal.h +++ b/boot/scene_internal.h @@ -613,6 +613,18 @@ int scene_calc_arrange(struct scene *scn, struct expo_arrange_info *arr); int scene_txt_generic_init(struct expo *exp, struct scene_txt_generic *gen, const char *name, uint str_id, const char *str); +/** + * scene_chklog() - Check if logging is enabled for an object + * + * This checks the 'expo_log_filter' environment variable. If not set, all + * objects are logged. If set, it contains a comma-separated list of filters; + * only objects whose name contains one of the filter strings are logged. + * + * @name: Object name to check + * Return: true if logging should happen, false to skip + */ +bool scene_chklog(const char *name); + /** * scene_flag_name() - Get the name of a scene flag * diff --git a/doc/develop/expo.rst b/doc/develop/expo.rst index fc642ed3696..7ef714be3da 100644 --- a/doc/develop/expo.rst +++ b/doc/develop/expo.rst @@ -605,6 +605,33 @@ These metrics help identify performance bottlenecks and verify that expo is operating efficiently. The timing information is particularly useful when optimizing display drivers or debugging slow rendering issues. +Log filter +~~~~~~~~~~ + +Expo supports filtering log output by object name, which is useful when +debugging specific objects. Set the ``expo_log_filter`` environment variable +to a substring that matches the object names you want to log. + +To enable log filtering:: + + => setenv expo_log_filter texted + => log filter-add -d console -A -c expo -l debug + +This shows debug logs only for objects whose name contains "texted". + +Multiple filters can be specified as a comma-separated list:: + + => setenv expo_log_filter menu,text + +This logs objects matching either "menu" or "text". + +Remove the filter to see all objects:: + + => setenv expo_log_filter + +This feature requires ``CONFIG_EXPO_LOG_FILTER`` which is enabled by default +for sandbox. + Writing expo tests ------------------ diff --git a/test/boot/expo.c b/test/boot/expo.c index 97b9bf82bb7..4febdf87cde 100644 --- a/test/boot/expo.c +++ b/test/boot/expo.c @@ -1260,6 +1260,43 @@ static int expo_scene_obj_type_name(struct unit_test_state *uts) } BOOTSTD_TEST(expo_scene_obj_type_name, 0); +/* Test scene_chklog() */ +static int expo_scene_chklog(struct unit_test_state *uts) +{ + /* Without filter, all objects should be logged */ + env_set("expo_log_filter", NULL); + ut_assert(scene_chklog("my-menu")); + ut_assert(scene_chklog("textline")); + + /* With a single filter, only matching objects should be logged */ + env_set("expo_log_filter", "menu"); + ut_assert(scene_chklog("my-menu")); + ut_assert(scene_chklog("menu-item")); + ut_assert(!scene_chklog("textline")); + ut_assert(!scene_chklog("other")); + + /* With comma-separated filters, any match should pass */ + env_set("expo_log_filter", "menu,text"); + ut_assert(scene_chklog("my-menu")); + ut_assert(scene_chklog("textline")); + ut_assert(scene_chklog("textedit")); + ut_assert(!scene_chklog("other")); + ut_assert(!scene_chklog("image")); + + /* Test with three filters */ + env_set("expo_log_filter", "menu,text,img"); + ut_assert(scene_chklog("my-menu")); + ut_assert(scene_chklog("textline")); + ut_assert(scene_chklog("img-logo")); + ut_assert(!scene_chklog("other")); + + /* Clear the filter */ + env_set("expo_log_filter", NULL); + + return 0; +} +BOOTSTD_TEST(expo_scene_chklog, 0); + /* Test scene_find_obj_within() */ static int expo_find_obj_within(struct unit_test_state *uts) { -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Text in a textedit can extend past the right boundary because it is measured before its bbox is set correctly. Call scene_obj_get_hw() after setting the edit text's bbox in scene_txted_calc_dims() to ensure correct word-wrapping. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/scene_textedit.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/boot/scene_textedit.c b/boot/scene_textedit.c index 160b9457a50..2075cfbd417 100644 --- a/boot/scene_textedit.c +++ b/boot/scene_textedit.c @@ -73,6 +73,11 @@ int scene_txted_calc_dims(struct scene_obj_txtedit *ted, struct udevice *cons) if (ret < 0) return log_msg_ret("sbb", ret); + /* Measure the edit text now that its bbox is set correctly */ + ret = scene_obj_get_hw(scn, ted->tin.edit_id, NULL); + if (ret < 0) + return log_msg_ret("hw", ret); + return 0; } -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Add detailed per-line measurement logging using log_content(), showing each line's bounding box and text content after measurement. Use scene_chklog() to filter which objects produce log output. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/scene.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/boot/scene.c b/boot/scene.c index 9cade9aad41..b64ddf51630 100644 --- a/boot/scene.c +++ b/boot/scene.c @@ -535,13 +535,27 @@ int scene_obj_get_hw(struct scene *scn, uint id, int *widthp) limit = obj->flags & SCENEOF_SIZE_VALID ? obj->req_bbox.x1 - obj->req_bbox.x0 : -1; - log_debug("obj %s limit %d\n", obj->name, limit); ret = vidconsole_measure(scn->expo->cons, gen->font_name, gen->font_size, str, limit, &bbox, &gen->lines); if (ret) return log_msg_ret("mea", ret); + if (scene_chklog(obj->name)) { + log_debug("obj %s limit %d: %d lines, width %d height %d\n", + obj->name, limit, gen->lines.count, + bbox.x1, bbox.y1); + for (int i = 0; i < gen->lines.count; i++) { + const struct vidconsole_mline *mline; + + mline = alist_get(&gen->lines, i, + struct vidconsole_mline); + log_content("line %d: %d,%d %d,%d '%.*s'\n", i, + mline->bbox.x0, mline->bbox.y0, + mline->bbox.x1, mline->bbox.y1, + mline->len, str + mline->start); + } + } if (widthp) *widthp = bbox.x1; -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Factor out the render_deps code from scene_textline_render_deps() into a shared helper scene_txtin_render_deps(). This handles cursor display when a text-input object is open, including entry restore/save and cursor positioning. For now, only textlines are supported. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/scene.c | 3 +-- boot/scene_internal.h | 25 +++++++++++++------------ boot/scene_textline.c | 29 ----------------------------- boot/scene_txtin.c | 29 +++++++++++++++++++++++++++++ 4 files changed, 43 insertions(+), 43 deletions(-) diff --git a/boot/scene.c b/boot/scene.c index b64ddf51630..ae3851cc82c 100644 --- a/boot/scene.c +++ b/boot/scene.c @@ -993,8 +993,7 @@ int scene_render_deps(struct scene *scn, uint id) (struct scene_obj_menu *)obj); break; case SCENEOBJT_TEXTLINE: - scene_textline_render_deps(scn, - (struct scene_obj_textline *)obj); + scene_txtin_render_deps(scn, obj, scene_obj_txtin(obj)); break; } } diff --git a/boot/scene_internal.h b/boot/scene_internal.h index d3c67777cb1..96bc4e06ad6 100644 --- a/boot/scene_internal.h +++ b/boot/scene_internal.h @@ -372,18 +372,6 @@ int scene_render_deps(struct scene *scn, uint id); */ int scene_menu_render_deps(struct scene *scn, struct scene_obj_menu *menu); -/** - * scene_textline_render_deps() - Render a textline and its dependencies - * - * Renders the textline and all of its attached objects - * - * @scn: Scene to render - * @tline: textline to render - * Returns: 0 if OK, -ve on error - */ -int scene_textline_render_deps(struct scene *scn, - struct scene_obj_textline *tline); - /** * scene_iter_objs() - Iterate through all scene objects * @@ -558,6 +546,19 @@ void scene_txtin_calc_bbox(struct scene_obj *obj, struct scene_txtin *tin, struct vidconsole_bbox *bbox, struct vidconsole_bbox *edit_bbox); +/** + * scene_txtin_render_deps() - Render dependencies for a text-input object + * + * Renders the edit text on top of the background if open + * + * @scn: Scene containing the object + * @obj: Object to render + * @tin: Text-input info + * Return: 0 if OK, -ve on error + */ +int scene_txtin_render_deps(struct scene *scn, struct scene_obj *obj, + struct scene_txtin *tin); + /** * scene_obj_calc_bbox() - Calculate bounding boxes for an object * diff --git a/boot/scene_textline.c b/boot/scene_textline.c index f940be3ed28..46ab53e30c9 100644 --- a/boot/scene_textline.c +++ b/boot/scene_textline.c @@ -152,35 +152,6 @@ bool scene_textline_within(const struct scene *scn, return scene_within(scn, tline->tin.edit_id, x, y); } -int scene_textline_render_deps(struct scene *scn, - struct scene_obj_textline *tline) -{ - const bool open = tline->obj.flags & SCENEOF_OPEN; - struct udevice *cons = scn->expo->cons; - uint i; - - /* if open, render the edit text on top of the background */ - if (open) { - int ret; - - ret = vidconsole_entry_restore(cons, &scn->entry_save); - if (ret) - return log_msg_ret("sav", ret); - scene_render_obj(scn, tline->tin.edit_id); - - /* move cursor back to the correct position */ - for (i = scn->cls.num; i < scn->cls.eol_num; i++) - vidconsole_put_char(cons, '\b'); - ret = vidconsole_entry_save(cons, &scn->entry_save); - if (ret) - return log_msg_ret("sav", ret); - - vidconsole_show_cursor(cons); - } - - return 0; -} - /** * scene_textline_putch() - Output a character to the vidconsole * diff --git a/boot/scene_txtin.c b/boot/scene_txtin.c index 99fad16c11f..2e7c496310d 100644 --- a/boot/scene_txtin.c +++ b/boot/scene_txtin.c @@ -57,6 +57,35 @@ int scene_txtin_arrange(struct scene *scn, struct expo_arrange_info *arr, return x; } +int scene_txtin_render_deps(struct scene *scn, struct scene_obj *obj, + struct scene_txtin *tin) +{ + const bool open = obj->flags & SCENEOF_OPEN; + struct udevice *cons = scn->expo->cons; + uint i; + + /* if open, render the edit text on top of the background */ + if (open) { + int ret; + + ret = vidconsole_entry_restore(cons, &scn->entry_save); + if (ret) + return log_msg_ret("sav", ret); + scene_render_obj(scn, tin->edit_id); + + /* move cursor back to the correct position */ + for (i = scn->cls.num; i < scn->cls.eol_num; i++) + vidconsole_put_char(cons, '\b'); + ret = vidconsole_entry_save(cons, &scn->entry_save); + if (ret) + return log_msg_ret("sav", ret); + + vidconsole_show_cursor(cons); + } + + return 0; +} + void scene_txtin_calc_bbox(struct scene_obj *obj, struct scene_txtin *tin, struct vidconsole_bbox *bbox, struct vidconsole_bbox *edit_bbox) -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Combine the TEXTLINE and TEXTEDIT cases in scene_obj_render() and scene_apply_theme() since they have identical handling. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/scene.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/boot/scene.c b/boot/scene.c index ae3851cc82c..b05291138ed 100644 --- a/boot/scene.c +++ b/boot/scene.c @@ -786,6 +786,7 @@ static int scene_obj_render(struct scene_obj *obj, bool text_mode) break; } case SCENEOBJT_TEXTLINE: + case SCENEOBJT_TEXTEDIT: if (obj->flags & SCENEOF_OPEN) scene_render_background(obj, true, false); break; @@ -796,10 +797,6 @@ static int scene_obj_render(struct scene_obj *obj, bool text_mode) obj->bbox.y1, box->width, vid_priv->colour_fg, box->fill); break; } - case SCENEOBJT_TEXTEDIT: - if (obj->flags & SCENEOF_OPEN) - scene_render_background(obj, true, false); - break; } return 0; @@ -1621,10 +1618,7 @@ int scene_apply_theme(struct scene *scn, struct expo_theme *theme) case SCENEOBJT_MENU: case SCENEOBJT_BOX: case SCENEOBJT_TEXTLINE: - break; case SCENEOBJT_TEXTEDIT: - scene_txted_set_font(scn, obj->id, NULL, - theme->font_size); break; case SCENEOBJT_TEXT: scene_txt_set_font(scn, obj->id, NULL, -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Factor out the open code from scene_textline_open() into a shared helper scene_txtin_open() This sets up the text editor ready for use, including copying the backup text, setting cursor position, and initialising the CLI line state. Update scene_obj_open() to call scene_txtin_open() directly for both textline and textedit objects. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/scene.c | 5 ++--- boot/scene_internal.h | 10 +++++---- boot/scene_textline.c | 45 --------------------------------------- boot/scene_txtin.c | 49 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 57 insertions(+), 52 deletions(-) diff --git a/boot/scene.c b/boot/scene.c index b05291138ed..6b5c934a2e7 100644 --- a/boot/scene.c +++ b/boot/scene.c @@ -1670,11 +1670,10 @@ static int scene_obj_open(struct scene *scn, struct scene_obj *obj) case SCENEOBJT_MENU: case SCENEOBJT_TEXT: case SCENEOBJT_BOX: - case SCENEOBJT_TEXTEDIT: break; case SCENEOBJT_TEXTLINE: - ret = scene_textline_open(scn, - (struct scene_obj_textline *)obj); + case SCENEOBJT_TEXTEDIT: + ret = scene_txtin_open(scn, obj, scene_obj_txtin(obj)); if (ret) return log_msg_ret("op", ret); break; diff --git a/boot/scene_internal.h b/boot/scene_internal.h index 96bc4e06ad6..8899ba538ae 100644 --- a/boot/scene_internal.h +++ b/boot/scene_internal.h @@ -569,15 +569,17 @@ int scene_txtin_render_deps(struct scene *scn, struct scene_obj *obj, int scene_obj_calc_bbox(struct scene_obj *obj, struct vidconsole_bbox *bbox); /** - * scene_textline_open() - Open a textline object + * scene_txtin_open() - Open a text-input object * * Set up the text editor ready for use * - * @scn: Scene containing the textline - * @tline: textline object + * @scn: Scene containing the object + * @obj: Object to open + * @tin: Text-input info * Return: 0 if OK, -ve on error */ -int scene_textline_open(struct scene *scn, struct scene_obj_textline *tline); +int scene_txtin_open(struct scene *scn, struct scene_obj *obj, + struct scene_txtin *tin); /** * scene_textline_close() - Close a textline object diff --git a/boot/scene_textline.c b/boot/scene_textline.c index 46ab53e30c9..2199b3d0641 100644 --- a/boot/scene_textline.c +++ b/boot/scene_textline.c @@ -152,48 +152,3 @@ bool scene_textline_within(const struct scene *scn, return scene_within(scn, tline->tin.edit_id, x, y); } -/** - * scene_textline_putch() - Output a character to the vidconsole - * - * This is used as the putch callback for CLI line editing, so that characters - * are sent to the correct vidconsole. - * - * @cls: CLI line state - * @ch: Character to output - */ -static void scene_textline_putch(struct cli_line_state *cls, int ch) -{ - struct scene *scn = container_of(cls, struct scene, cls); - - vidconsole_put_char(scn->expo->cons, ch); -} - -int scene_textline_open(struct scene *scn, struct scene_obj_textline *tline) -{ - struct udevice *cons = scn->expo->cons; - struct scene_obj_txt *txt; - int ret; - - /* Copy the text into the scene buffer in case the edit is cancelled */ - memcpy(abuf_data(&scn->buf), abuf_data(&tline->tin.buf), - abuf_size(&scn->buf)); - - /* get the position of the editable */ - txt = scene_obj_find(scn, tline->tin.edit_id, SCENEOBJT_NONE); - if (!txt) - return log_msg_ret("cur", -ENOENT); - - vidconsole_set_cursor_pos(cons, txt->obj.bbox.x0, txt->obj.bbox.y0); - vidconsole_entry_start(cons); - cli_cread_init(&scn->cls, abuf_data(&tline->tin.buf), tline->tin.line_chars); - scn->cls.insert = true; - scn->cls.putch = scene_textline_putch; - ret = vidconsole_entry_save(cons, &scn->entry_save); - if (ret) - return log_msg_ret("sav", ret); - - /* make sure the cursor is visible */ - vidconsole_readline_start(true); - - return 0; -} diff --git a/boot/scene_txtin.c b/boot/scene_txtin.c index 2e7c496310d..a4c373c14fa 100644 --- a/boot/scene_txtin.c +++ b/boot/scene_txtin.c @@ -10,8 +10,10 @@ #include <expo.h> #include <log.h> +#include <menu.h> #include <video_console.h> #include <linux/errno.h> +#include <linux/string.h> #include "scene_internal.h" int scene_txtin_init(struct scene_txtin *tin, uint size, uint line_chars) @@ -86,6 +88,53 @@ int scene_txtin_render_deps(struct scene *scn, struct scene_obj *obj, return 0; } +/** + * scene_txtin_putch() - Output a character to the vidconsole + * + * This is used as the putch callback for CLI line editing, so that characters + * are sent to the correct vidconsole. + * + * @cls: CLI line state + * @ch: Character to output + */ +static void scene_txtin_putch(struct cli_line_state *cls, int ch) +{ + struct scene *scn = container_of(cls, struct scene, cls); + + vidconsole_put_char(scn->expo->cons, ch); +} + +int scene_txtin_open(struct scene *scn, struct scene_obj *obj, + struct scene_txtin *tin) +{ + struct udevice *cons = scn->expo->cons; + struct scene_obj_txt *txt; + int ret; + + /* Copy the text into the scene buffer in case the edit is cancelled */ + memcpy(abuf_data(&scn->buf), abuf_data(&tin->buf), + abuf_size(&scn->buf)); + + /* get the position of the editable */ + txt = scene_obj_find(scn, tin->edit_id, SCENEOBJT_NONE); + if (!txt) + return log_msg_ret("cur", -ENOENT); + + vidconsole_set_cursor_pos(cons, txt->obj.bbox.x0, txt->obj.bbox.y0); + vidconsole_entry_start(cons); + cli_cread_init(&scn->cls, abuf_data(&tin->buf), tin->line_chars); + scn->cls.insert = true; + scn->cls.putch = scene_txtin_putch; + ret = vidconsole_entry_save(cons, &scn->entry_save); + if (ret) + return log_msg_ret("sav", ret); + + /* make sure the cursor is visible */ + vidconsole_readline_start(true); + + return 0; +} + void scene_txtin_calc_bbox(struct scene_obj *obj, struct scene_txtin *tin, struct vidconsole_bbox *bbox, struct vidconsole_bbox *edit_bbox) -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Add scene_txtin_close() to handle closing a text-input object. This hides the cursor by calling vidconsole_readline_end(). Update scene_textline_send_key() to use the new helper when closing on BKEY_QUIT. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/scene_internal.h | 15 +++++++++++---- boot/scene_textline.c | 3 +-- boot/scene_txtin.c | 6 ++++++ 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/boot/scene_internal.h b/boot/scene_internal.h index 8899ba538ae..e732650b99f 100644 --- a/boot/scene_internal.h +++ b/boot/scene_internal.h @@ -582,15 +582,22 @@ int scene_txtin_open(struct scene *scn, struct scene_obj *obj, struct scene_txtin *tin); /** - * scene_textline_close() - Close a textline object + * scene_txtin_close() - Close a text-input object * * Close out the text editor after use * - * @scn: Scene containing the textline - * @tline: textline object + * @scn: Scene containing the object + */ +void scene_txtin_close(struct scene *scn); + +/** + * scene_obj_calc_bbox() - Calculate bounding boxes for an object + * + * @obj: Object to process + * @bbox: Returns bounding boxes for object * Return: 0 if OK, -ve on error */ -int scene_textline_close(struct scene *scn, struct scene_obj_textline *tline); +int scene_obj_calc_bbox(struct scene_obj *obj, struct vidconsole_bbox *bbox); /** * scene_calc_arrange() - Calculate sizes needed to arrange a scene diff --git a/boot/scene_textline.c b/boot/scene_textline.c index 2199b3d0641..9b5c40462fd 100644 --- a/boot/scene_textline.c +++ b/boot/scene_textline.c @@ -114,8 +114,7 @@ int scene_textline_send_key(struct scene *scn, struct scene_obj_textline *tline, memcpy(abuf_data(&tline->tin.buf), abuf_data(&scn->buf), abuf_size(&scn->buf)); - /* cursor is not needed now */ - vidconsole_readline_end(); + scene_txtin_close(scn); } else { event->type = EXPOACT_QUIT; log_debug("menu quit\n"); diff --git a/boot/scene_txtin.c b/boot/scene_txtin.c index a4c373c14fa..d1aa24dac9f 100644 --- a/boot/scene_txtin.c +++ b/boot/scene_txtin.c @@ -104,6 +104,12 @@ static void scene_txtin_putch(struct cli_line_state *cls, int ch) vidconsole_put_char(scn->expo->cons, ch); } +void scene_txtin_close(struct scene *scn) +{ + /* cursor is not needed now */ + vidconsole_readline_end(); +} + int scene_txtin_open(struct scene *scn, struct scene_obj *obj, struct scene_txtin *tin) { -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Move SCENEOBJT_TEXTEDIT after SCENEOBJT_TEXTLINE in the enum so it can be highlighted like other text-input objects. Update scene_obj_type_names array to match. Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/scene.c | 2 +- include/expo.h | 2 +- test/boot/expo.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/boot/scene.c b/boot/scene.c index 6b5c934a2e7..46bc9b64e64 100644 --- a/boot/scene.c +++ b/boot/scene.c @@ -38,9 +38,9 @@ static const char *const scene_obj_type_names[] = { "image", "text", "box", - "textedit", "menu", "textline", + "textedit", }; bool scene_chklog(const char *name) diff --git a/include/expo.h b/include/expo.h index c854d78255b..248f2363fe1 100644 --- a/include/expo.h +++ b/include/expo.h @@ -242,11 +242,11 @@ enum scene_obj_t { SCENEOBJT_IMAGE, SCENEOBJT_TEXT, SCENEOBJT_BOX, - SCENEOBJT_TEXTEDIT, /* types from here on can be highlighted */ SCENEOBJT_MENU, SCENEOBJT_TEXTLINE, + SCENEOBJT_TEXTEDIT, }; /** diff --git a/test/boot/expo.c b/test/boot/expo.c index 4febdf87cde..c8c97057d54 100644 --- a/test/boot/expo.c +++ b/test/boot/expo.c @@ -1254,7 +1254,7 @@ static int expo_scene_obj_type_name(struct unit_test_state *uts) ut_asserteq_str("textedit", scene_obj_type_name(SCENEOBJT_TEXTEDIT)); /* Test invalid type (out of range) */ - ut_asserteq_str("unknown", scene_obj_type_name(SCENEOBJT_TEXTLINE + 1)); + ut_asserteq_str("unknown", scene_obj_type_name(SCENEOBJT_TEXTEDIT + 1)); return 0; } -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> The scene_txted_arrange() function sets the textedit's height to zero, which causes problems when highlighting since the bounding box has no height. Fix this by calculating the height from the edit text's dimensions, similar to how scene_textline_arrange() does it. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/scene_textedit.c | 44 ++++++++++++++++++++++++++++++++++--------- test/boot/expo.c | 2 +- 2 files changed, 36 insertions(+), 10 deletions(-) diff --git a/boot/scene_textedit.c b/boot/scene_textedit.c index 2075cfbd417..d6ef725db3e 100644 --- a/boot/scene_textedit.c +++ b/boot/scene_textedit.c @@ -54,21 +54,31 @@ int scene_txted_set_font(struct scene *scn, uint id, const char *font_name, int scene_txted_calc_dims(struct scene_obj_txtedit *ted, struct udevice *cons) { + const struct expo_theme *theme = &ted->obj.scene->expo->theme; struct scene *scn = ted->obj.scene; struct scene_obj_txt *txt; - int ret; + int x0, ret; txt = scene_obj_find(scn, ted->tin.edit_id, SCENEOBJT_NONE); if (!txt) return log_msg_ret("txt", -ENOENT); /* - * Set the edit text's bbox to match the textedit's bbox. This ensures - * SCENEOF_SIZE_VALID is set so vidconsole_measure() applies the width - * limit for word-wrapping/clipping. + * Set the edit text's bbox to fit within the textedit's bbox, after + * the label. This ensures SCENEOF_SIZE_VALID is set so + * vidconsole_measure() applies the width limit for word-wrapping. */ + x0 = ted->obj.req_bbox.x0; + if (ted->tin.label_id) { + int width; + + ret = scene_obj_get_hw(scn, ted->tin.label_id, &width); + if (ret < 0) + return log_msg_ret("lab", ret); + x0 += width + theme->textline_label_margin_x; + } ret = scene_obj_set_bbox(scn, ted->tin.edit_id, - ted->obj.req_bbox.x0, ted->obj.req_bbox.y0, + x0, ted->obj.req_bbox.y0, ted->obj.req_bbox.x1, ted->obj.req_bbox.y1); if (ret < 0) return log_msg_ret("sbb", ret); @@ -84,7 +94,8 @@ int scene_txted_calc_dims(struct scene_obj_txtedit *ted, struct udevice *cons) int scene_txted_arrange(struct scene *scn, struct expo_arrange_info *arr, struct scene_obj_txtedit *ted) { - int x; + struct scene_obj *edit; + int x, y; int ret; x = scene_txtin_arrange(scn, arr, &ted->obj, &ted->tin); @@ -92,14 +103,29 @@ int scene_txted_arrange(struct scene *scn, struct expo_arrange_info *arr, return log_msg_ret("arr", x); /* constrain the edit text to fit within the textedit bbox */ - ret = scene_obj_set_bbox(scn, ted->tin.edit_id, x, ted->obj.req_bbox.y0, + y = ted->obj.req_bbox.y0; + ret = scene_obj_set_bbox(scn, ted->tin.edit_id, x, y, ted->obj.req_bbox.x1, ted->obj.req_bbox.y1); if (ret < 0) return log_msg_ret("edi", ret); + /* Re-measure text with the correct limit (bbox may have changed) */ + ret = scene_obj_get_hw(scn, ted->tin.edit_id, NULL); + if (ret < 0) + return log_msg_ret("hw", ret); + + edit = scene_obj_find(scn, ted->tin.edit_id, SCENEOBJT_NONE); + if (!edit) + return log_msg_ret("fnd", -ENOENT); + x += edit->dims.x; + y += edit->dims.y; + + /* + * Set dims based on content size, but don't call scene_obj_set_size() + * as that would overwrite the user-specified req_bbox + */ ted->obj.dims.x = x - ted->obj.req_bbox.x0; - ted->obj.dims.y = 0; - scene_obj_set_size(scn, ted->obj.id, ted->obj.dims.x, ted->obj.dims.y); + ted->obj.dims.y = y - ted->obj.req_bbox.y0; return 0; } diff --git a/test/boot/expo.c b/test/boot/expo.c index c8c97057d54..79e77b83b0a 100644 --- a/test/boot/expo.c +++ b/test/boot/expo.c @@ -1570,7 +1570,7 @@ static int expo_render_textedit(struct unit_test_state *uts) expo_set_scene_id(exp, SCENE1); ut_assertok(scene_arrange(scn)); ut_assertok(expo_render(exp)); - ut_asserteq(19860, video_compress_fb(uts, dev, false)); + ut_asserteq(19841, video_compress_fb(uts, dev, false)); abuf_uninit(&buf); abuf_uninit(&logo_copy); -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Update scene_render_deps() to call scene_txtin_render_deps() for textedit objects by grouping SCENEOBJT_TEXTEDIT with SCENEOBJT_TEXTLINE. This enables proper cursor handling when the textedit is open. Add a test for textedit highlighting. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/scene.c | 2 +- test/boot/expo.c | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/boot/scene.c b/boot/scene.c index 46bc9b64e64..306625361b4 100644 --- a/boot/scene.c +++ b/boot/scene.c @@ -983,13 +983,13 @@ int scene_render_deps(struct scene *scn, uint id) case SCENEOBJT_IMAGE: case SCENEOBJT_TEXT: case SCENEOBJT_BOX: - case SCENEOBJT_TEXTEDIT: break; case SCENEOBJT_MENU: scene_menu_render_deps(scn, (struct scene_obj_menu *)obj); break; case SCENEOBJT_TEXTLINE: + case SCENEOBJT_TEXTEDIT: scene_txtin_render_deps(scn, obj, scene_obj_txtin(obj)); break; } diff --git a/test/boot/expo.c b/test/boot/expo.c index 79e77b83b0a..d7430dc4284 100644 --- a/test/boot/expo.c +++ b/test/boot/expo.c @@ -1572,6 +1572,12 @@ static int expo_render_textedit(struct unit_test_state *uts) ut_assertok(expo_render(exp)); ut_asserteq(19841, video_compress_fb(uts, dev, false)); + /* highlight the textedit and re-render */ + scene_set_highlight_id(scn, OBJ_TEXTED); + ut_assertok(scene_arrange(scn)); + ut_assertok(expo_render(exp)); + ut_asserteq(21662, video_compress_fb(uts, dev, false)); + abuf_uninit(&buf); abuf_uninit(&logo_copy); -- 2.43.0
participants (1)
-
Simon Glass