Uh oh!
There was an error while loading. Please reload this page.
Fixed text being dropped when the draw context stack overflows - #176
Merged
fdesbiens merged 1 commit intoAug 27, 2026
Merged
Conversation
The fix for issue eclipse-threadx#148 (eclipse-threadx#158) made every caller of _gx_canvas_drawing_initiate() skip its draw when the call returns GX_DRAW_NESTING_EXCEEDED. Four of those callers push a nested context on the widget they are already drawing, for one reason only: to narrow the clipping rectangle to the widget's client area. For them, skipping the draw turns a correct rendering into no rendering at all, which is what made guix_all_widgets_accordion_menu report "Frame 12 is different". The accordion menu screen of the all_widgets demo nests widgets nine levels deep - multi_level_accordion, menu_list, mla_menu_1_accordion, menu_list, text_view_3 - and _gx_system_canvas_refresh() consumes two contexts before the widget tree is walked, so the eight slots of GX_MAX_CONTEXT_NESTING are exhausted before _gx_multi_line_text_view_text_draw() can push its own. Raising the limit in a scratch build makes frame 12 match the existing golden file exactly, which shows that the golden records the correct rendering and that the text is now being lost rather than merely clipped differently. When the stack is full there is nothing to push, but the caller's context is still the right context to draw through: it was created for the same widget and differs only in its clipping rectangle. These four callers now narrow the caller's clipping rectangle, draw, and restore it, instead of dropping the draw. _gx_canvas_drawing_complete() is still not called on overflow, so the stack corruption that eclipse-threadx#158 fixed stays fixed. Applied to _gx_multi_line_text_view_text_draw, _gx_multi_line_text_input_draw, _gx_rich_text_view_text_draw and _gx_single_line_text_input_draw. _gx_widget_block_move and _gx_radial_progress_bar_background_draw are left alone: neither pushes a clip-only context on the widget being drawn. guix_canvas_draw_nesting_overflow_render_no_output closes the coverage gap that eclipse-threadx#158 left. guix_canvas_draw_nesting_overflow_no_output covers detection and state preservation; the new test covers what a caller must render: that the text is still drawn at maximum nesting depth, that the borrowed context is handed back with its nesting count, context pointer and clipping rectangle unchanged, and that the pixels match those produced when a nested context is available. Assisted-by: Claude Code (Opus 5) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
guix_all_widgets_accordion_menuhas been failing onmasterwithFrame 12 is differentsince #158.git bisect runnames #158 (8a35f690, the issue #148 stack-corruption fix) as the first bad commit. That commit made every caller of_gx_canvas_drawing_initiate()skip its draw when the call returnsGX_DRAW_NESTING_EXCEEDED. For callers that push a nested context on the widget they are already drawing, purely to narrow the clipping rectangle to the client area, that turns a correct rendering into no rendering at all.The multi-level accordion screen of the
all_widgetsdemo nests widgets nine levels deep —multi_level_accordion→menu_list→mla_menu_1_accordion→menu_list→text_view_3— and_gx_system_canvas_refresh()consumes two contexts before the widget tree is walked (gx_system_canvas_refresh.c:335and:376). The eight slots ofGX_MAX_CONTEXT_NESTINGare therefore exhausted before_gx_multi_line_text_view_text_draw()can push its own, and the text is dropped.The golden file records the correct rendering
Raising
GX_MAX_CONTEXT_NESTINGin a scratch build makes frame 12 match the existing golden file byte-for-byte. So the golden is the correct output and #158 introduced a visual regression: text GUIX had always drawn is now missing. Regenerating the golden would have blessed that permanently, which is why this PR fixes the code instead and leaves all golden data untouched.Measured with gdb, one run of the test hits
GX_DRAW_NESTING_EXCEEDED68 times: 67 from_gx_widget_children_draw(not changed by #158 — it is cited there as the reference implementation, and it has always dropped the paint) and exactly 1 from_gx_multi_line_text_view_text_draw. One changed call site, one differing frame.Fix
#158 conflated two things: "no context was pushed, so
_gx_canvas_drawing_complete()must not be called" (correct) with "so nothing may be drawn" (not correct for a clip-only push).When a widget initiates a nested context on itself just to narrow the clip, the caller's context is still the right context to draw through: it was created for the same widget, carries the same view list, and differs only in
gx_draw_context_dirty. These call sites now narrow the caller's clipping rectangle, draw, and restore it:_gx_canvas_drawing_complete()is still never called on overflow, so the stack corruption #158 fixed stays fixed. The non-overflow path is unchanged, which is why nothing else in the suite moves.Applied to the four clip-only call sites:
gx_multi_line_text_view_text_draw.ctext_view; the reproduced regressiongx_multi_line_text_input_draw.cinputgx_rich_text_view_text_draw.ctext_viewgx_single_line_text_input_draw.cwidgetDeliberately not changed:
gx_widget_block_move.cgx_radial_progress_bar_background_draw.cwho == GX_NULL; there is no caller context on that canvas to borrowTests
guix_canvas_draw_nesting_overflow_no_output(added by #158) covers overflow detection and state preservation. Nothing covered what a caller must render — the gap that let this regression through.New:
guix_canvas_draw_nesting_overflow_render_no_output. It fills the draw context stack on a real multi line text view, gives the widget a non-zero whitespace so the narrowed clip is strictly smaller than the widget clip, and asserts thatThe checksum sums raw bytes of
gx_canvas_memory, so the test is colour-depth agnostic and behaves the same in every build configuration. Confirmed to fail without the code change:Three of the four fixed call sites have no existing test that reaches the overflow path, so this is their only coverage.
Results
default_build_coveragepartial_canvas_support_builddynamic_bidi_text_buildguix_all_widgets_accordion_menupasses against the unmodified golden file. No golden data was regenerated.The one remaining
default_build_coveragefailure,guix_ml_text_view_32bpp, is a separate pre-existing issue with a different cause (no nesting overflow occurs in it at all; #159 word wrapping is the suspect).guix_bidi_text_draw_32bppindynamic_bidi_text_buildis also pre-existing and produces byte-identical output before and after this change.Follow-up, not in this PR
The 67
_gx_widget_children_drawoverflows are a separate, older defect: the demo's accordion screen needs a peak context depth of 10 (measured by instrumenting_gx_canvas_drawing_initiate()), whileGX_MAX_CONTEXT_NESTINGdefaults to 8. Consequently six frames ofguix_all_widgets_accordion_menu's golden data (15–20) record incompletely painted output — they stop matching as soon as the stack is deep enough for the children to be drawn.Raising the default is well contained (at 12: 304 bytes of static RAM on a 32-bit target, six golden frames to regenerate, and
guix_widget_children_drawto rewrite since it exists specifically to exceed the limit), but it changes a public default and checked-in golden data, so it is kept out of this fix.Closes the regression introduced by #158.