fix(render): content invalidation propagates again; clear hover on focus loss
closes #368 closes #367 #368 -- Frame(cache_subtree=True) silently froze its subtree's CONTENT. markContentDirty() gated its walk up the parent chain on (!was_dirty || !p->render_dirty). That is sound only if render_dirty is reliably cleared once a drawable has been drawn. It is not: clearDirty() had exactly one call site in the whole engine (UIGridView, added by #364 on this branch). UIFrame's non-render-texture path, UICaption, UISprite, UILine, UICircle and UIArc never cleared it, so render_dirty was a write-once latch -- true from an object's first mutation until it died. With the flag stuck true, was_dirty was always true AND the parent's render_dirty was always true, so the guard was always false and content invalidation propagated nowhere. A caching ancestor went on re-blitting a stale composite: text never updated, colours never changed, sprite indices never changed. Only movement survived, because x/y route through markCompositeDirty, whose walk was already unconditional. That asymmetry is why the bug read as intermittent rather than total, and it is why my original issue writeup got the shape wrong -- its repro's third step was a move, so it looked like the latch cleared. It does not. Measured on master: 4/4 recolours and 3/3 caption edits under a cache were ALL dropped, including the first. The fix makes markContentDirty's walk unconditional, matching markCompositeDirty. The alternative -- repair the guard by having every drawable clear the flag honestly -- buys a shorter walk at the price of an invariant that silently breaks the day someone adds a drawable and forgets. Not worth it here: parent chains are shallow, and this is the exact cost markCompositeDirty has always paid on every position change without anyone noticing. A/B on the changed path (200k content mutations, best of 3): depth 1 4 8 16 before 82ns 86ns 82ns 87ns <- flat because it did nothing after 76ns 114ns 175ns 295ns The old build's flatness IS the defect. Entity movement -- the actual hot path -- is unchanged (164ns/move either way), since it was already on the unconditional composite path. Crucible shows no regression (step_swarm 12.9 -> 10.7ms, entity_churn 36.3 -> 31.5ms, within noise). Regression test asserts every consecutive content mutation repaints through a cache at depths 1 and 5, AND that idle frames stay byte-identical -- without that last assertion, "mark everything dirty always" would pass and the cache would be worthless. #367 -- Losing window focus stranded hover exactly as a window-leave did (#363). An unfocused window is delivered no MouseMoved, so the hover walk never runs and whatever was hovered when focus left stays lit while the game sits in the background: on_exit/on_cell_exit never fire and hovered_cell keeps naming a cell the user is no longer pointing at. sf::Event::LostFocus was already produced by every backend -- SDL2Renderer even translates SDL_WINDOWEVENT_FOCUS_LOST into it -- and, like MouseLeft before #363, had nobody listening. Routed to the same PyScene::do_mouse_leave() that #363 added. Once unfocused the engine genuinely does not know where the pointer is, so "hovering nothing" is the only truthful state to hold; the next MouseMoved after focus returns restores it. automation.loseFocus() injects the event, via a new injectWindowEvent() rather than injectMouseEvent -- a focus event has no coordinates and no button, and should not borrow a signature that implies it does. api_surface golden re-baselined: +1 line. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
c696b4ef41
commit
9d67e33370
8 changed files with 379 additions and 5 deletions
|
|
@ -1046,15 +1046,32 @@ bool UIDrawable::contains_point(float x, float y) const {
|
|||
}
|
||||
|
||||
// #144: Content dirty - texture needs rebuild
|
||||
//
|
||||
// #368 - The walk up the parent chain is UNCONDITIONAL. It used to be gated on
|
||||
// `(!was_dirty || !p->render_dirty)`, which is sound only if render_dirty is reliably
|
||||
// cleared once a drawable has been drawn. It is not: clearDirty() is called by the two
|
||||
// classes that actually *cache* a raster (UIFrame's render-texture path, UIGridView),
|
||||
// and by nobody else. For every ordinary Frame/Caption/Sprite/Line/Circle/Arc,
|
||||
// render_dirty is a write-once latch -- true from the first mutation until death.
|
||||
//
|
||||
// With the flag stuck true, `was_dirty` was always true and the parent's render_dirty
|
||||
// was always true, so the guard was always false and content invalidation propagated
|
||||
// NOWHERE. A caching ancestor (Frame(cache_subtree=True), or a GridView) went on
|
||||
// re-blitting a stale composite: text never updated, colors never changed. Only
|
||||
// position changes survived, because markCompositeDirty (below) already walked
|
||||
// unconditionally.
|
||||
//
|
||||
// Restoring the guard would mean making every drawable clear the flag honestly, and
|
||||
// then trusting that every future drawable remembers to. The invariant here is worth
|
||||
// more than the walk it saves: parent chains are shallow, and this is exactly the cost
|
||||
// markCompositeDirty has always paid on every move without anyone noticing.
|
||||
void UIDrawable::markContentDirty() {
|
||||
bool was_dirty = render_dirty;
|
||||
render_dirty = true;
|
||||
composite_dirty = true; // If content changed, composite also needs update
|
||||
|
||||
// Propagate to parent - parent's composite is dirty (child content changed)
|
||||
// Propagate if: we weren't already dirty, OR parent was cleared (rendered) since last propagation
|
||||
auto p = parent.lock();
|
||||
if (p && (!was_dirty || !p->render_dirty)) {
|
||||
if (p) {
|
||||
p->markContentDirty(); // Parent also needs to rebuild to include our changes
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue