[Bugfix] UIDrawable base x/y/pos/grid_pos setters don't propagate dirty flags to parent #290

New issue

Closed

opened 2026-03-08 16:35:46 +00:00 by john · 0 comments

john commented

2026-03-08 16:35:46 +00:00

Owner

Bug

UIDrawable::set_float_member() (the base class setter for x and y properties, used by ALL drawable types) calls drawable->onPositionChanged() but does NOT call markCompositeDirty() or markDirty().

Similarly, UIDrawable::set_pos() and UIDrawable::set_grid_pos() call onPositionChanged() without any dirty propagation.

None of the onPositionChanged() overrides (Frame, Caption, Sprite, Grid, Arc) call any dirty-flag method either — they only sync the SFML object's position.

Affected code paths

Setter	File:Line	Calls markDirty?
`UIDrawable::set_float_member()` (x, y)	UIDrawable.cpp:508-516	No — only `onPositionChanged()`
`UIDrawable::set_pos()`	UIDrawable.cpp:604	No — only `onPositionChanged()`
`UIDrawable::set_grid_pos()`	UIDrawable.cpp:839	No — only `onPositionChanged()`
`UIFrame::onPositionChanged()`	UIFrame.cpp:118-122	No
`UICaption::onPositionChanged()`	UICaption.cpp:162-166	No
`UISprite::onPositionChanged()`	UISprite.cpp:245-249	No
`UIGrid::onPositionChanged()`	UIGrid.cpp:642-647	No
`UIArc::onPositionChanged()`	UIArc.cpp:230-234	No

Contrast with correct path

UIDrawable::applyAlignment() (line 2132-2134) does it right:

position = sf::Vector2f(x + offset_x, y + offset_y);
onPositionChanged();
markCompositeDirty();  // <-- This is what's missing everywhere else

Also, UIDrawable::set_rotation() (line 632-633) correctly calls markDirty(). UIDrawable::set_origin() (line 706) correctly calls markDirty(). Only position changes are missing it.

Fix

The cleanest fix is to add markCompositeDirty() to UIDrawable::set_float_member() after the position cases, and to set_pos() and set_grid_pos(). Position changes need markCompositeDirty() (not full markContentDirty()) since the child's rendered content hasn't changed — only its position within the parent's composite.

// In UIDrawable::set_float_member(), after case 0 and case 1:
drawable->onPositionChanged();
drawable->markCompositeDirty();  // ADD THIS

Impact

Affects ALL drawable types when they are children of a Frame with clip_children=True or cache_subtree=True. Moving any child (.x = ..., .y = ..., .pos = ...) won't visually update.

#288 — UICollection mutations don't invalidate parent cache
#289 — Caption-specific property setters missing markDirty()

## Bug `UIDrawable::set_float_member()` (the base class setter for `x` and `y` properties, used by ALL drawable types) calls `drawable->onPositionChanged()` but does NOT call `markCompositeDirty()` or `markDirty()`. Similarly, `UIDrawable::set_pos()` and `UIDrawable::set_grid_pos()` call `onPositionChanged()` without any dirty propagation. None of the `onPositionChanged()` overrides (Frame, Caption, Sprite, Grid, Arc) call any dirty-flag method either — they only sync the SFML object's position. ### Affected code paths | Setter | File:Line | Calls markDirty? | |--------|-----------|-----------------| | `UIDrawable::set_float_member()` (x, y) | UIDrawable.cpp:508-516 | **No** — only `onPositionChanged()` | | `UIDrawable::set_pos()` | UIDrawable.cpp:604 | **No** — only `onPositionChanged()` | | `UIDrawable::set_grid_pos()` | UIDrawable.cpp:839 | **No** — only `onPositionChanged()` | | `UIFrame::onPositionChanged()` | UIFrame.cpp:118-122 | **No** | | `UICaption::onPositionChanged()` | UICaption.cpp:162-166 | **No** | | `UISprite::onPositionChanged()` | UISprite.cpp:245-249 | **No** | | `UIGrid::onPositionChanged()` | UIGrid.cpp:642-647 | **No** | | `UIArc::onPositionChanged()` | UIArc.cpp:230-234 | **No** | ### Contrast with correct path `UIDrawable::applyAlignment()` (line 2132-2134) does it right: ```cpp position = sf::Vector2f(x + offset_x, y + offset_y); onPositionChanged(); markCompositeDirty(); // <-- This is what's missing everywhere else ``` Also, `UIDrawable::set_rotation()` (line 632-633) correctly calls `markDirty()`. `UIDrawable::set_origin()` (line 706) correctly calls `markDirty()`. Only position changes are missing it. ### Fix The cleanest fix is to add `markCompositeDirty()` to `UIDrawable::set_float_member()` after the position cases, and to `set_pos()` and `set_grid_pos()`. Position changes need `markCompositeDirty()` (not full `markContentDirty()`) since the child's rendered content hasn't changed — only its position within the parent's composite. ```cpp // In UIDrawable::set_float_member(), after case 0 and case 1: drawable->onPositionChanged(); drawable->markCompositeDirty(); // ADD THIS ``` ### Impact Affects ALL drawable types when they are children of a Frame with `clip_children=True` or `cache_subtree=True`. Moving any child (`.x = ...`, `.y = ...`, `.pos = ...`) won't visually update. ### Related - #288 — UICollection mutations don't invalidate parent cache - #289 — Caption-specific property setters missing markDirty()