[Bugfix] Grid layer property setters (visible, z_index) don't invalidate the render cache #376

Closed
opened 2026-07-14 11:23:27 +00:00 by john · 0 comments
Owner

Found while unrotting tests/regression/issue_148_layer_dirty_flags.py (part of the #341/#350/#372 batch). Independently reproduced.

Defect

Setting layer.visible does not invalidate the grid's render cache, in both directions. Hiding a layer leaves the rendered image byte-identical; re-showing it does not restore it. The change only takes effect once some unrelated mutation happens to dirty the layer.

Reproduction (confirmed)

g  = mcrfpy.Grid(grid_size=(10,10), pos=(0,0), size=(160,160))
cl = mcrfpy.ColorLayer(name='bg'); g.add_layer(cl)
cl.fill(mcrfpy.Color(255,0,0,255))

h1 = sha256(screenshot())   # bb4e4488f992df0a  -- red
cl.visible = False
h2 = sha256(screenshot())   # bb4e4488f992df0a  -- IDENTICAL. layer still drawn.
cl.set((0,0), same_color)   # any data mutation -> markDirty()
h3 = sha256(screenshot())   # e899dc4ec6ad358a  -- the hide finally takes effect

Root cause

src/GridLayers.cppPyGridLayerAPI::ColorLayer_set_visible (~line 1723) and TileLayer_set_visible:

int v = PyObject_IsTrue(value);
if (v < 0) return -1;
self->data->visible = v;
return 0;

They never call markDirty(). GridLayer::markDirty() (line 163) is what bumps parent_grid->content_generation, and content_generation is the key the UIGridView render early-out (#351) uses to decide whether to re-composite. Every data mutator (fill/set/edit) calls it; the visible setter does not. So the view re-blits its stale cached RenderTexture.

The semantics are clearly intended: GridLayer::render() early-outs on the flag (if (!visible) return;, lines 436 and 622), so the hide renders correctly the moment anything else dirties the layer. Only the invalidation is missing.

Same latent defect in z_index

ColorLayer_set_z_index (line 1703) writes z_index and carries an explicit // TODO: Trigger re-sort in parent grid. Confirmed empirically: reordering two overlapping ColorLayers via z_index does not change the render either.

Both are instances of the same class: layer property setters don't invalidate the grid cache.

Fix

Two lines per setter:

if (v != self->data->visible) { self->data->visible = v; self->data->markDirty(); }

and the equivalent for z_index (which additionally needs the parent grid's layer re-sort).

Why it was never caught

tests/regression/issue_148_layer_dirty_flags.py is named for exactly this class of defect, but had rotted (it called add_layer(name=...), which no longer accepts kwargs) and was silently scored as passing. It exercised the flags via data mutations only, never via the visible setter alone.

Note this became observable only after #351 added the render early-out — before that, the unconditional redraw masked it. Same masking story as the entity set_position bug #351 fixed.

Found while unrotting `tests/regression/issue_148_layer_dirty_flags.py` (part of the #341/#350/#372 batch). Independently reproduced. ## Defect Setting `layer.visible` does **not** invalidate the grid's render cache, in **both directions**. Hiding a layer leaves the rendered image byte-identical; re-showing it does not restore it. The change only takes effect once some *unrelated* mutation happens to dirty the layer. ## Reproduction (confirmed) ```python g = mcrfpy.Grid(grid_size=(10,10), pos=(0,0), size=(160,160)) cl = mcrfpy.ColorLayer(name='bg'); g.add_layer(cl) cl.fill(mcrfpy.Color(255,0,0,255)) h1 = sha256(screenshot()) # bb4e4488f992df0a -- red cl.visible = False h2 = sha256(screenshot()) # bb4e4488f992df0a -- IDENTICAL. layer still drawn. cl.set((0,0), same_color) # any data mutation -> markDirty() h3 = sha256(screenshot()) # e899dc4ec6ad358a -- the hide finally takes effect ``` ## Root cause `src/GridLayers.cpp` — `PyGridLayerAPI::ColorLayer_set_visible` (~line 1723) and `TileLayer_set_visible`: ```cpp int v = PyObject_IsTrue(value); if (v < 0) return -1; self->data->visible = v; return 0; ``` They never call `markDirty()`. `GridLayer::markDirty()` (line 163) is what bumps `parent_grid->content_generation`, and `content_generation` is the key the `UIGridView` render early-out (#351) uses to decide whether to re-composite. Every *data* mutator (`fill`/`set`/`edit`) calls it; the *visible* setter does not. So the view re-blits its stale cached RenderTexture. The semantics are clearly intended: `GridLayer::render()` early-outs on the flag (`if (!visible) return;`, lines 436 and 622), so the hide renders correctly the moment anything else dirties the layer. Only the **invalidation** is missing. ## Same latent defect in z_index `ColorLayer_set_z_index` (line 1703) writes `z_index` and carries an explicit `// TODO: Trigger re-sort in parent grid`. Confirmed empirically: reordering two overlapping ColorLayers via `z_index` does not change the render either. Both are instances of the same class: **layer property setters don't invalidate the grid cache.** ## Fix Two lines per setter: ```cpp if (v != self->data->visible) { self->data->visible = v; self->data->markDirty(); } ``` and the equivalent for `z_index` (which additionally needs the parent grid's layer re-sort). ## Why it was never caught `tests/regression/issue_148_layer_dirty_flags.py` is *named for exactly this class of defect*, but had rotted (it called `add_layer(name=...)`, which no longer accepts kwargs) and was silently scored as passing. It exercised the flags via data mutations only, never via the `visible` setter alone. Note this became *observable* only after #351 added the render early-out — before that, the unconditional redraw masked it. Same masking story as the entity `set_position` bug #351 fixed.
john closed this issue 2026-07-14 12:01:04 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
john/McRogueFace#376
No description provided.