From 3fea6418ffda7e52916b56a29b1515c98864408c Mon Sep 17 00:00:00 2001 From: John McCardle Date: Thu, 22 Jan 2026 22:54:50 -0500 Subject: [PATCH] Fix UIFrame RenderTexture positioning and toggling issues - Fix #223: Use `position` instead of `box.getPosition()` for render_sprite positioning. The box was being set to (0,0) for texture rendering and never restored, causing frames to render at wrong positions. - Fix #224: Add disableRenderTexture() method and call it when toggling clip_children or cache_subtree off. This properly cleans up the texture and prevents stale rendering. - Fix #225: Improve dirty propagation in markContentDirty() to propagate to parent even when already dirty, if the parent was cleared (rendered) since last propagation. Prevents child changes from being invisible. - Fix #226: Add fallback to standard rendering when RenderTexture can't be created (e.g., zero-size frame). Prevents inconsistent state. Closes #223, closes #224, closes #225, closes #226 Co-Authored-By: Claude Opus 4.5 --- src/UIDrawable.cpp | 17 +++- src/UIDrawable.h | 6 ++ src/UIFrame.cpp | 24 ++++- .../issue_223_clip_children_test.py | 95 +++++++++++++++++++ 4 files changed, 135 insertions(+), 7 deletions(-) create mode 100644 tests/regression/issue_223_clip_children_test.py diff --git a/src/UIDrawable.cpp b/src/UIDrawable.cpp index a43fe59..8ed785d 100644 --- a/src/UIDrawable.cpp +++ b/src/UIDrawable.cpp @@ -419,11 +419,20 @@ void UIDrawable::enableRenderTexture(unsigned int width, unsigned int height) { } render_sprite.setTexture(render_texture->getTexture()); } - + use_render_texture = true; render_dirty = true; } +void UIDrawable::disableRenderTexture() { + if (!use_render_texture) return; + + render_texture.reset(); + render_sprite = sf::Sprite(); // Clear stale texture reference + use_render_texture = false; + render_dirty = true; +} + void UIDrawable::updateRenderTexture() { if (!use_render_texture || !render_texture) { return; @@ -907,14 +916,14 @@ bool UIDrawable::contains_point(float x, float y) const { // #144: Content dirty - texture needs rebuild void UIDrawable::markContentDirty() { - if (render_dirty) return; // Already dirty, no need to propagate - + 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) { + if (p && (!was_dirty || !p->render_dirty)) { p->markContentDirty(); // Parent also needs to rebuild to include our changes } } diff --git a/src/UIDrawable.h b/src/UIDrawable.h index fe43215..cffcebd 100644 --- a/src/UIDrawable.h +++ b/src/UIDrawable.h @@ -248,6 +248,12 @@ protected: // Enable RenderTexture for this drawable void enableRenderTexture(unsigned int width, unsigned int height); void updateRenderTexture(); + + // Disable RenderTexture for this drawable (public for property setters) +public: + void disableRenderTexture(); + +protected: public: // #144: Dirty flag system - content vs composite diff --git a/src/UIFrame.cpp b/src/UIFrame.cpp index 98fda0f..c624cd0 100644 --- a/src/UIFrame.cpp +++ b/src/UIFrame.cpp @@ -124,6 +124,13 @@ void UIFrame::render(sf::Vector2f offset, sf::RenderTarget& target) } } + // Fall back to standard rendering if texture not available (e.g., zero size) + if (!use_render_texture) { + use_texture = false; // Force standard path for this render call + } + } + + if (use_texture) { // Update RenderTexture if dirty if (use_render_texture && render_dirty) { // Clear the RenderTexture @@ -158,7 +165,8 @@ void UIFrame::render(sf::Vector2f offset, sf::RenderTarget& target) // Draw the RenderTexture sprite (single blit!) if (use_render_texture) { - render_sprite.setPosition(offset + box.getPosition()); + // Use `position` instead of box.getPosition() - box was set to (0,0) for texture rendering + render_sprite.setPosition(offset + position); target.draw(render_sprite); } } else { @@ -389,13 +397,19 @@ int UIFrame::set_clip_children(PyUIFrameObject* self, PyObject* value, void* clo PyErr_SetString(PyExc_TypeError, "clip_children must be a boolean"); return -1; } - + bool new_clip = PyObject_IsTrue(value); if (new_clip != self->data->clip_children) { self->data->clip_children = new_clip; + + // Disable texture if no longer needed (and cache_subtree isn't using it) + if (!new_clip && !self->data->cache_subtree) { + self->data->disableRenderTexture(); + } + self->data->markDirty(); // Mark as needing redraw } - + return 0; } @@ -423,7 +437,11 @@ int UIFrame::set_cache_subtree(PyUIFrameObject* self, PyObject* value, void* clo self->data->enableRenderTexture(static_cast(size.x), static_cast(size.y)); } + } else if (!self->data->clip_children) { + // Disable texture if clip_children also doesn't need it + self->data->disableRenderTexture(); } + self->data->markDirty(); // Mark as needing redraw } diff --git a/tests/regression/issue_223_clip_children_test.py b/tests/regression/issue_223_clip_children_test.py new file mode 100644 index 0000000..94ef4da --- /dev/null +++ b/tests/regression/issue_223_clip_children_test.py @@ -0,0 +1,95 @@ +"""Test clip_children positioning and toggling fixes (#223, #224, #225, #226) + +Issue #223: Frame appears at wrong position with clip_children +Issue #224: Stale rendering after toggling clip_children off +Issue #225: Child changes don't update parent's cached texture +Issue #226: Zero-size frame with clip_children causes inconsistent state + +This test verifies the property/positioning behavior. Visual rendering verification +requires a game loop and should be done with the demo system. +""" +import mcrfpy +import sys + +errors = [] + +# Test 1: Position with clip_children=True (#223) +frame = mcrfpy.Frame(pos=(100, 100), size=(200, 200), clip_children=True) +if frame.x != 100: + errors.append(f"#223 Test 1: Expected x=100, got {frame.x}") +if frame.y != 100: + errors.append(f"#223 Test 1: Expected y=100, got {frame.y}") + +# Add a child +child = mcrfpy.Caption(text="Hello", pos=(10, 10)) +frame.children.append(child) + +# Test 2: Toggle clip_children off (#224) +frame.clip_children = False +if frame.x != 100: + errors.append(f"#224 Test 2 after toggle off: Expected x=100, got {frame.x}") +if frame.y != 100: + errors.append(f"#224 Test 2 after toggle off: Expected y=100, got {frame.y}") + +# Toggle back on +frame.clip_children = True +if frame.x != 100: + errors.append(f"#224 Test 2 after toggle on: Expected x=100, got {frame.x}") + +# Test 3: Zero-size frame with clip_children (#226) +zero_frame = mcrfpy.Frame(pos=(50, 50), size=(0, 0), clip_children=True) +if zero_frame.x != 50: + errors.append(f"#226 Test 3: Expected x=50, got {zero_frame.x}") +if zero_frame.y != 50: + errors.append(f"#226 Test 3: Expected y=50, got {zero_frame.y}") + +# Test 4: cache_subtree toggle similar to clip_children +cache_frame = mcrfpy.Frame(pos=(200, 200), size=(100, 100), cache_subtree=True) +if cache_frame.x != 200: + errors.append(f"Test 4: Expected x=200, got {cache_frame.x}") + +cache_frame.cache_subtree = False +if cache_frame.x != 200: + errors.append(f"Test 4 after toggle: Expected x=200, got {cache_frame.x}") + +# Test 5: Both clip_children and cache_subtree +both_frame = mcrfpy.Frame(pos=(300, 300), size=(100, 100), clip_children=True, cache_subtree=True) + +# Turn off clip_children (cache_subtree should keep texture active) +both_frame.clip_children = False +if both_frame.x != 300: + errors.append(f"Test 5 after clip toggle: Expected x=300, got {both_frame.x}") + +# Turn off cache_subtree too +both_frame.cache_subtree = False +if both_frame.x != 300: + errors.append(f"Test 5 after full toggle: Expected x=300, got {both_frame.x}") + +# Test 6: Moving a frame with clip_children +frame.x = 150 +frame.y = 150 +if frame.x != 150: + errors.append(f"Test 6: Expected x=150, got {frame.x}") +if frame.y != 150: + errors.append(f"Test 6: Expected y=150, got {frame.y}") + +# Test 7: Child modification (dirty propagation #225) +# This tests that modifying a child marks the parent dirty +# We can't directly test render_dirty flag from Python, but we can verify +# that the API doesn't crash and positions stay correct +frame2 = mcrfpy.Frame(pos=(400, 400), size=(100, 100), clip_children=True) +child2 = mcrfpy.Caption(text="Test", pos=(5, 5)) +frame2.children.append(child2) + +# Modify child text +child2.text = "Changed" +if frame2.x != 400: + errors.append(f"Test 7: Expected x=400 after child change, got {frame2.x}") + +# Report results +if errors: + print("FAIL: " + "; ".join(errors)) + sys.exit(1) +else: + print("PASS: clip_children positioning and toggling tests") + sys.exit(0)