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 <noreply@anthropic.com>
This commit is contained in:
John McCardle 2026-01-22 22:54:50 -05:00
commit 3fea6418ff
4 changed files with 135 additions and 7 deletions

View file

@ -419,11 +419,20 @@ void UIDrawable::enableRenderTexture(unsigned int width, unsigned int height) {
} }
render_sprite.setTexture(render_texture->getTexture()); render_sprite.setTexture(render_texture->getTexture());
} }
use_render_texture = true; use_render_texture = true;
render_dirty = 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() { void UIDrawable::updateRenderTexture() {
if (!use_render_texture || !render_texture) { if (!use_render_texture || !render_texture) {
return; return;
@ -907,14 +916,14 @@ bool UIDrawable::contains_point(float x, float y) const {
// #144: Content dirty - texture needs rebuild // #144: Content dirty - texture needs rebuild
void UIDrawable::markContentDirty() { void UIDrawable::markContentDirty() {
if (render_dirty) return; // Already dirty, no need to propagate bool was_dirty = render_dirty;
render_dirty = true; render_dirty = true;
composite_dirty = true; // If content changed, composite also needs update composite_dirty = true; // If content changed, composite also needs update
// Propagate to parent - parent's composite is dirty (child content changed) // 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(); auto p = parent.lock();
if (p) { if (p && (!was_dirty || !p->render_dirty)) {
p->markContentDirty(); // Parent also needs to rebuild to include our changes p->markContentDirty(); // Parent also needs to rebuild to include our changes
} }
} }

View file

@ -248,6 +248,12 @@ protected:
// Enable RenderTexture for this drawable // Enable RenderTexture for this drawable
void enableRenderTexture(unsigned int width, unsigned int height); void enableRenderTexture(unsigned int width, unsigned int height);
void updateRenderTexture(); void updateRenderTexture();
// Disable RenderTexture for this drawable (public for property setters)
public:
void disableRenderTexture();
protected:
public: public:
// #144: Dirty flag system - content vs composite // #144: Dirty flag system - content vs composite

View file

@ -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 // Update RenderTexture if dirty
if (use_render_texture && render_dirty) { if (use_render_texture && render_dirty) {
// Clear the RenderTexture // Clear the RenderTexture
@ -158,7 +165,8 @@ void UIFrame::render(sf::Vector2f offset, sf::RenderTarget& target)
// Draw the RenderTexture sprite (single blit!) // Draw the RenderTexture sprite (single blit!)
if (use_render_texture) { 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); target.draw(render_sprite);
} }
} else { } 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"); PyErr_SetString(PyExc_TypeError, "clip_children must be a boolean");
return -1; return -1;
} }
bool new_clip = PyObject_IsTrue(value); bool new_clip = PyObject_IsTrue(value);
if (new_clip != self->data->clip_children) { if (new_clip != self->data->clip_children) {
self->data->clip_children = new_clip; 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 self->data->markDirty(); // Mark as needing redraw
} }
return 0; return 0;
} }
@ -423,7 +437,11 @@ int UIFrame::set_cache_subtree(PyUIFrameObject* self, PyObject* value, void* clo
self->data->enableRenderTexture(static_cast<unsigned int>(size.x), self->data->enableRenderTexture(static_cast<unsigned int>(size.x),
static_cast<unsigned int>(size.y)); static_cast<unsigned int>(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 self->data->markDirty(); // Mark as needing redraw
} }

View file

@ -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)