_gx_widget_block_move() drops the repaint when the canvas has no views

[fdesbiens]

When _gx_canvas_drawing_initiate() reports GX_NO_VIEWS, _gx_widget_block_move() closes the draw context and returns GX_SUCCESS having done nothing: it neither moves the block nor marks the area dirty. The region is never repainted and the caller is told the move succeeded.

Line references are against dev at d6a06679, where this code is unchanged.

The defect

common/src/gx_widget_block_move.c:136-139:

        else if (status == GX_NO_VIEWS)
        {
            _gx_canvas_drawing_complete(canvas, GX_FALSE);
        }

The asymmetry is what makes it a defect. Every other path that declines to move the block hands the area to the dirty list first:

Line	Situation	_gx_system_dirty_partial_add()?
87	transparent widget — cannot block move	yes
106	shift larger than the widget	yes
133	_gx_canvas_block_move() failed	yes
136	GX_NO_VIEWS from _gx_canvas_drawing_initiate()	no
implicit else	GX_DRAW_NESTING_EXCEEDED — no context pushed	no

There is a third instance of the same shape at lines 117-127, on the inner initiate that redraws the portion which could not be moved: if inner_status is neither GX_SUCCESS nor GX_NO_VIEWS, that portion is neither redrawn nor invalidated.

The delta is exactly one lost call

_gx_canvas_block_move() (common/src/gx_canvas_block_move.c) walks context->gx_draw_context_view_head and returns GX_FAILURE unless a viewport contains the block. That head is GX_NULL precisely when _gx_canvas_drawing_initiate() returned GX_NO_VIEWS, so the loop never runs and the call always fails on this path.

So pre-#158, GX_NO_VIEWS deterministically fell into the _gx_canvas_block_move()-failed branch, which does invalidate:

        _gx_canvas_drawing_initiate(canvas, widget, block);          /* return discarded */
        status = _gx_canvas_block_move(block, x_shift, y_shift, &dirty);

        if (status == GX_SUCCESS) { ... }
        else
        {
            /* block move was not successful, just invalidate the widget client */
            _gx_system_dirty_partial_add(widget, block);
            _gx_canvas_drawing_complete(canvas, GX_FALSE);
        }

No blit was ever performed with a NULL view list, before or after. The entire behavioural delta of #158 on this path is the loss of _gx_system_dirty_partial_add().

#158 is right about the hazard it was fixing — on GX_DRAW_NESTING_EXCEEDED no context was pushed, so calling _gx_canvas_drawing_complete() would corrupt the outer context (#148). The problem is only that "do not complete" was implemented as "do not do anything", including the part that had nothing to do with the draw context.

Reachability is established

Probing status at gx_widget_block_move.c:112 under gdb across all 88 animation, scroll, list and tree-view test binaries in default_build_coverage (0 = GX_SUCCESS, 3 = GX_NO_VIEWS): 37 reach _gx_widget_block_move() and 16 of those hit GX_NO_VIEWS — each exactly once.

guix_all_widgets_drop_list_background_draw        2 x 0    1 x 3
guix_all_widgets_drop_list_event_process          2 x 0    1 x 3
guix_all_widgets_drop_list_pixelmap_set           2 x 0    1 x 3
guix_all_widgets_scrollbar_16bpp                221 x 0    1 x 3
guix_all_widgets_scrollbar_8bpp                 221 x 0    1 x 3
guix_drop_list_24xrgb                             2 x 0    1 x 3
guix_drop_list_open                               2 x 0    1 x 3
guix_horizontal_list_event_process_pen_up         2 x 0    1 x 3
guix_scroll_thumb_event_process                   3 x 0    1 x 3
guix_scrollbar_24xrgb                           222 x 0    1 x 3
guix_scrollbar_reset                              2 x 0    1 x 3
guix_scrollbar_thumb_24xrgb                     760 x 0    1 x 3
guix_scrollbar_value_calculate                    2 x 0    1 x 3
guix_scrollbar_value_set                          3 x 0    1 x 3
guix_window_client_scroll                         3 x 0    1 x 3
guix_window_client_scroll_api                     4 x 0    1 x 3

The other 21 only ever saw GX_SUCCESS. GX_DRAW_NESTING_EXCEEDED was never observed at this call site.

So drop lists, scrollbars and window client scrolling reach the defective path in routine use — this is not an exotic corner.

_gx_widget_block_move() has eight in-tree callers plus the public gx_widget_block_move() API via gxe_widget_block_move.c:96: gx_window_scroll.c:118, gx_horizontal_list_scroll.c:290, gx_vertical_list_scroll.c:288, gx_tree_view_scroll.c:111, gx_multi_line_text_view_scroll.c:127, gx_animation_update.c:220, gx_animation_drag_tracking.c:195, gx_animation_slide_landing.c:283.

Why the suite is still green

All 16 of those tests pass, and their golden files predate #158, so the captured frames are byte-identical before and after. The lost invalidation is not observable in their capture areas.

That is not evidence the defect is harmless. A canvas with no views has nothing visible to repaint at that instant; the question is whether any caller relies on the dirty area still being queued for the next refresh, when views may have been recomputed. No current test exercises that sequence — which is exactly why this is filed rather than fixed.

Suggested fix

Restore the fallback on both non-success paths, keeping #148's guarantee that _gx_canvas_drawing_complete() is only called when a context was actually pushed:

        else if (status == GX_NO_VIEWS)
        {
            /* No viewport contains the block, so nothing can be blitted, but the
               area still needs a repaint.  */
            _gx_system_dirty_partial_add(widget, block);
            _gx_canvas_drawing_complete(canvas, GX_FALSE);
        }
        else
        {
            /* GX_DRAW_NESTING_EXCEEDED: no context was pushed, so there is
               nothing to complete — but the area still needs a repaint.  */
            _gx_system_dirty_partial_add(widget, block);
        }

The two branches are not equivalent in status, and a commit message should say so:

• The GX_NO_VIEWS branch is a restoration of pre-Fixed issue 148 draw context stack overflow #158 behaviour.
• The GX_DRAW_NESTING_EXCEEDED branch is new behaviour, not a restoration. Pre-Fixed issue 148 draw context stack overflow #158, _gx_system_current_draw_context still pointed at the outer context there, so _gx_canvas_block_move() could walk the outer context's view list and blit against the wrong clip — part of the Buffer overflow on _gx_system_draw_context_stack[GX_MAX_CONTEXT_NESTING] #148 corruption, not a behaviour to restore. Adding the invalidation is a deliberate improvement.

Calling _gx_system_dirty_partial_add() before _gx_canvas_drawing_complete() with a context pushed is already precedented at line 133, so the ordering is not novel.

Validate before adopting. Marking an area dirty from a path that may itself run during a canvas refresh could re-enter the dirty list. Run the full suite in every configuration, not just default_build_coverage, and watch partial_canvas_support_* in particular: there _gx_canvas_drawing_initiate() has an extra failure mode (GX_INVALID_MEMORY_SIZE) that falls into the same implicit else, and _gx_canvas_block_move() declines outright for partial-frame-buffer canvases.

Ship a regression test with the fix. The drop-list and scrollbar tests above already reach the path, so a *_no_output test that drives a block move on a viewless canvas and asserts the area landed in the dirty list would close the gap that let this through.

Relationship to #176

#158 left five call sites with a path that neither draws nor invalidates. #176 fixed four of them — _gx_multi_line_text_view_text_draw, _gx_multi_line_text_input_draw, _gx_rich_text_view_text_draw, _gx_single_line_text_input_draw — by narrowing the caller's clipping rectangle instead of dropping the draw.

_gx_widget_block_move() was deliberately left alone there, because it does not push a clip-only context on the widget being drawn, so #176's remedy does not apply to it. It is the remaining one, and it needs the different remedy above.

What is established, and what is not

Established:

• The behaviour change above, read from git show 8a35f690 -- common/src/gx_widget_block_move.c and from current source.
• _gx_canvas_block_move() cannot succeed with a NULL view head, so pre-Fixed issue 148 draw context stack overflow #158 the GX_NO_VIEWS case deterministically reached _gx_system_dirty_partial_add(). The whole delta is that one lost call.
• Reachability: 37 of 88 tests reach the function, 16 hit GX_NO_VIEWS, once each.
• Those 16 tests pass, so the lost invalidation is not observable in their capture areas.

Not established:

• That the lost invalidation causes any user-visible defect. There is no failing test behind it.
• That the suggested fix is safe. It is untested — see the caveat above.
• Whether GX_DRAW_NESTING_EXCEEDED or GX_INVALID_MEMORY_SIZE ever reach line 110. Only GX_SUCCESS and GX_NO_VIEWS were observed, and only in default_build_coverage; the partial_canvas_support_* configurations were not swept.
• A hypothesis worth checking: every affected test hits it exactly once, which is the signature of the first block move after a widget is shown. _gx_widget_show() frees the window's view list (gx_widget_show.c:93-97) and views are only recomputed later inside _gx_system_canvas_refresh(). That is the same mechanism behind Fixed animation start regression that prevented animations from running #175. If it holds, this matters most on the first scroll or first drop-list open after a screen becomes visible.

Style note (not a defect)

gx_widget_block_move.c:117 declares UINT inner_status mid-block. Valid C99 and permitted by AGENTS.md, but every other declaration in the file and throughout common/src sits at the top of the function. Worth normalising whenever this file is next touched.

Related

• _gx_system_canvas_refresh() leaks an internal drawing status through the public API #180 — the companion finding from the same audit of Fixed issue 148 draw context stack overflow #158: _gx_system_canvas_refresh() leaking a drawing status. Same class of defect, at the API boundary rather than in the paint path.
• GX_MAX_CONTEXT_NESTING = 8 is too shallow for the shipped all_widgets demo #182 — the same silently-dropped-repaint shape from the other direction (GX_DRAW_NESTING_EXCEEDED rather than GX_NO_VIEWS).
• Fixed text being dropped when the draw context stack overflows #176 — fixed four of the five call sites Fixed issue 148 draw context stack overflow #158 left with a neither-draw-nor-invalidate path. This is the fifth.