perf(grid): clean-state render early-out for UIGridView; closes #351
UIGridView::render() re-rasterized its RenderTexture (clear + all layer draws + entity draws + display) every frame unconditionally -- the #255 early-out and markCompositeDirty() machinery existed only on the superseded UIGrid::render() path, dead since the #252 GridView shim. Idle grids paid a full re-raster per frame for nothing. Fix: a clean-state early-out that re-blits the cached texture when nothing affecting the raster changed. Two signals, chosen so a missed invalidation can't silently produce a stale frame: - Camera params (center_x/y, zoom, camera_rotation, box size, fill_color, render-tex size) are compared directly on the view -- impossible to forget. - Grid content changes bump a new GridData::content_generation counter at the existing chokes: GridData::markDirty/markCompositeDirty (entities, animation), GridLayer::markDirty (all tile/color/texture edits), layer add/remove. Also fixes a latent bug: the Python entity setters set_position and set_spritenumber updated the spatial hash but never invalidated rendering (masked only because the view redrew every frame). They, and the entity add/remove/die sites, now mark the grid dirty. Perspective overlays (checked authoritatively on the UIGrid, since the view's copy isn't resynced at runtime) and grid children are conservative always-render carve-outs -- tracked precisely later in #352. Regression test issue_351_gridview_early_out_test.py screenshots before/after each mutation kind (entity move via set_position and via animation, tile edit, color edit, entity add/die, camera pan, zoom) and asserts the pixels changed -- guarding against false early-outs -- plus an idle pair that must stay identical. Suite 308/308. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
452b668ab8
commit
0c61c70ccd
7 changed files with 201 additions and 0 deletions
|
|
@ -10,6 +10,7 @@
|
|||
// GridData is never independently heap-allocated (always a UIGrid base
|
||||
// subobject), so the downcast is valid; remove once #252 allows pure GridData.
|
||||
void GridData::markDirty() {
|
||||
content_generation++; // #351 - content changed; invalidate view early-out
|
||||
static_cast<UIGrid*>(this)->UIDrawable::markDirty();
|
||||
if (auto view = owning_view.lock()) {
|
||||
view->markDirty();
|
||||
|
|
@ -17,6 +18,7 @@ void GridData::markDirty() {
|
|||
}
|
||||
|
||||
void GridData::markCompositeDirty() {
|
||||
content_generation++; // #351 - content changed; invalidate view early-out
|
||||
static_cast<UIGrid*>(this)->UIDrawable::markCompositeDirty();
|
||||
if (auto view = owning_view.lock()) {
|
||||
view->markCompositeDirty();
|
||||
|
|
@ -176,6 +178,7 @@ std::shared_ptr<ColorLayer> GridData::addColorLayer(int z_index, const std::stri
|
|||
layer->name = name;
|
||||
layers.push_back(layer);
|
||||
layers_need_sort = true;
|
||||
content_generation++; // #351 - layer set changed
|
||||
return layer;
|
||||
}
|
||||
|
||||
|
|
@ -185,6 +188,7 @@ std::shared_ptr<TileLayer> GridData::addTileLayer(int z_index, std::shared_ptr<P
|
|||
layer->name = name;
|
||||
layers.push_back(layer);
|
||||
layers_need_sort = true;
|
||||
content_generation++; // #351 - layer set changed
|
||||
return layer;
|
||||
}
|
||||
|
||||
|
|
@ -197,6 +201,7 @@ void GridData::removeLayer(std::shared_ptr<GridLayer> layer)
|
|||
if (layer) {
|
||||
layer->parent_grid = nullptr;
|
||||
}
|
||||
content_generation++; // #351 - layer set changed
|
||||
}
|
||||
|
||||
void GridData::sortLayers()
|
||||
|
|
|
|||
|
|
@ -148,6 +148,13 @@ public:
|
|||
// =========================================================================
|
||||
std::weak_ptr<UIGridView> owning_view;
|
||||
|
||||
// #351 - Monotonic counter bumped whenever grid content drawn into a view's
|
||||
// RenderTexture changes (entities added/removed/moved, sprite changes, layer
|
||||
// edits, layer add/remove). UIGridView compares it against the generation it
|
||||
// last rendered to skip re-rasterizing an unchanged grid. Camera parameters
|
||||
// live on the view and are compared there directly, not counted here.
|
||||
uint64_t content_generation = 0;
|
||||
|
||||
// #313 - Render invalidation from the data layer. Entities hold
|
||||
// shared_ptr<GridData> but still need to invalidate rendering when their
|
||||
// visual state changes. These set the dirty flags on the UIGrid subobject
|
||||
|
|
|
|||
|
|
@ -163,6 +163,7 @@ void GridLayer::initChunks() {
|
|||
void GridLayer::markDirty() {
|
||||
// Mark ALL chunks as dirty
|
||||
std::fill(chunk_dirty.begin(), chunk_dirty.end(), true);
|
||||
if (parent_grid) parent_grid->content_generation++; // #351 - view early-out
|
||||
}
|
||||
|
||||
void GridLayer::markDirty(int cell_x, int cell_y) {
|
||||
|
|
@ -173,6 +174,7 @@ void GridLayer::markDirty(int cell_x, int cell_y) {
|
|||
if (chunk_idx >= 0 && chunk_idx < static_cast<int>(chunk_dirty.size())) {
|
||||
chunk_dirty[chunk_idx] = true;
|
||||
}
|
||||
if (parent_grid) parent_grid->content_generation++; // #351 - view early-out
|
||||
}
|
||||
|
||||
int GridLayer::getChunkIndex(int cell_x, int cell_y) const {
|
||||
|
|
|
|||
|
|
@ -386,6 +386,7 @@ int UIEntity::init(PyUIEntityObject* self, PyObject* args, PyObject* kwds) {
|
|||
self->data->grid = grid_ptr;
|
||||
grid_ptr->entities->push_back(self->data);
|
||||
grid_ptr->spatial_hash.insert(self->data);
|
||||
grid_ptr->markDirty(); // #351 - entity added; re-raster view
|
||||
}
|
||||
}
|
||||
return 0;
|
||||
|
|
@ -468,6 +469,7 @@ int UIEntity::set_position(PyUIEntityObject* self, PyObject* value, void* closur
|
|||
// Update spatial hash if grid exists (#115)
|
||||
if (self->data->grid) {
|
||||
self->data->grid->spatial_hash.update(self->data, old_x, old_y);
|
||||
self->data->grid->markCompositeDirty(); // #351 - entity moved; re-raster view
|
||||
}
|
||||
|
||||
return 0;
|
||||
|
|
@ -558,6 +560,7 @@ int UIEntity::set_spritenumber(PyUIEntityObject* self, PyObject* value, void* cl
|
|||
}
|
||||
//self->data->sprite.sprite_index = val;
|
||||
self->data->sprite.setSpriteIndex(val); // todone - I don't like ".sprite.sprite" in this stack of UIEntity.UISprite.sf::Sprite
|
||||
if (self->data->grid) self->data->grid->markDirty(); // #351 - sprite changed; re-raster view
|
||||
return 0;
|
||||
}
|
||||
|
||||
|
|
@ -898,6 +901,7 @@ int UIEntity::set_grid(PyUIEntityObject* self, PyObject* value, void* closure)
|
|||
if (it != entities->end()) {
|
||||
entities->erase(it);
|
||||
}
|
||||
self->data->grid->markDirty(); // #351 - entity removed; re-raster view
|
||||
self->data->grid.reset();
|
||||
|
||||
// Release identity strong ref -- entity left grid
|
||||
|
|
@ -934,6 +938,7 @@ int UIEntity::set_grid(PyUIEntityObject* self, PyObject* value, void* closure)
|
|||
if (it != old_entities->end()) {
|
||||
old_entities->erase(it);
|
||||
}
|
||||
self->data->grid->markDirty(); // #351 - entity left old grid; re-raster
|
||||
}
|
||||
|
||||
// Add to new grid
|
||||
|
|
@ -941,6 +946,7 @@ int UIEntity::set_grid(PyUIEntityObject* self, PyObject* value, void* closure)
|
|||
new_grid->entities->push_back(self->data);
|
||||
self->data->grid = new_grid;
|
||||
new_grid->spatial_hash.insert(self->data); // #274
|
||||
new_grid->markDirty(); // #351 - entity added to new grid; re-raster
|
||||
// #294: perspective_map is lazy -- the next updateVisibility() call
|
||||
// (or first `entity.perspective_map` access) allocates sized to the
|
||||
// new grid. We deliberately do NOT preserve or clear the old map:
|
||||
|
|
@ -1167,6 +1173,7 @@ PyObject* UIEntity::die(PyUIEntityObject* self, PyObject* Py_UNUSED(ignored))
|
|||
grid->spatial_hash.remove(self->data);
|
||||
|
||||
entities->erase(it);
|
||||
grid->markDirty(); // #351 - entity died; re-raster view
|
||||
// Clear the grid reference
|
||||
self->data->grid.reset();
|
||||
|
||||
|
|
|
|||
|
|
@ -134,6 +134,45 @@ void UIGridView::render(sf::Vector2f offset, sf::RenderTarget& target)
|
|||
|
||||
ensureRenderTextureSize();
|
||||
|
||||
// #351 - clean-state early-out: re-blit the cached RenderTexture when nothing
|
||||
// affecting the raster changed since last frame. Camera params are compared
|
||||
// directly (view-local); grid content changes bump content_generation. The
|
||||
// perspective overlay (checked authoritatively on the UIGrid, since the view's
|
||||
// copy is not resynced at runtime) and any grid children are conservative
|
||||
// always-render carve-outs until tracked precisely (#352).
|
||||
UIGrid* ugrid = static_cast<UIGrid*>(grid_data.get());
|
||||
bool perspective_active = perspective_enabled || (ugrid && ugrid->perspective_enabled);
|
||||
bool can_skip =
|
||||
has_rendered_once
|
||||
&& !perspective_active
|
||||
&& (!grid_data->children || grid_data->children->empty())
|
||||
&& grid_data->content_generation == last_content_gen
|
||||
&& center_x == last_center_x && center_y == last_center_y
|
||||
&& zoom == last_zoom && camera_rotation == last_camera_rotation
|
||||
&& box.getSize() == last_box_size
|
||||
&& fill_color == last_fill_color
|
||||
&& renderTextureSize == last_render_tex_size;
|
||||
if (can_skip) {
|
||||
if (rotation != 0.0f) {
|
||||
output.setOrigin(origin);
|
||||
output.setRotation(rotation);
|
||||
output.setPosition(box.getPosition() + offset + origin);
|
||||
} else {
|
||||
output.setOrigin(0, 0);
|
||||
output.setRotation(0);
|
||||
output.setPosition(box.getPosition() + offset);
|
||||
}
|
||||
if (shader && shader->shader) {
|
||||
sf::Vector2f resolution(box.getSize().x, box.getSize().y);
|
||||
PyShader::applyEngineUniforms(*shader->shader, resolution);
|
||||
if (uniforms) uniforms->applyTo(*shader->shader);
|
||||
target.draw(output, shader->shader.get());
|
||||
} else {
|
||||
target.draw(output);
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
int cell_width = ptex ? ptex->sprite_width : DEFAULT_CELL_WIDTH;
|
||||
int cell_height = ptex ? ptex->sprite_height : DEFAULT_CELL_HEIGHT;
|
||||
|
||||
|
|
@ -305,6 +344,17 @@ void UIGridView::render(sf::Vector2f offset, sf::RenderTarget& target)
|
|||
output.setRotation(0);
|
||||
}
|
||||
|
||||
// #351 - record the inputs this raster was built from for next frame's early-out.
|
||||
has_rendered_once = true;
|
||||
last_content_gen = grid_data->content_generation;
|
||||
last_center_x = center_x;
|
||||
last_center_y = center_y;
|
||||
last_zoom = zoom;
|
||||
last_camera_rotation = camera_rotation;
|
||||
last_box_size = box.getSize();
|
||||
last_fill_color = fill_color;
|
||||
last_render_tex_size = renderTextureSize;
|
||||
|
||||
if (shader && shader->shader) {
|
||||
sf::Vector2f resolution(box.getSize().x, box.getSize().y);
|
||||
PyShader::applyEngineUniforms(*shader->shader, resolution);
|
||||
|
|
|
|||
|
|
@ -64,6 +64,19 @@ public:
|
|||
std::weak_ptr<UIEntity> perspective_entity;
|
||||
bool perspective_enabled = false;
|
||||
|
||||
// #351 - clean-state render early-out cache. These record the inputs the
|
||||
// current RenderTexture was rasterized from; render() re-blits the cached
|
||||
// texture instead of clear+redraw when none changed. Camera params are
|
||||
// view-local (compared directly); grid content changes bump
|
||||
// GridData::content_generation. Perspective overlays and grid children are
|
||||
// dynamic sources not tracked precisely yet (#352) -> conservative full render.
|
||||
bool has_rendered_once = false;
|
||||
uint64_t last_content_gen = 0;
|
||||
float last_center_x = 0.f, last_center_y = 0.f, last_zoom = 0.f, last_camera_rotation = 0.f;
|
||||
sf::Vector2f last_box_size{-1.f, -1.f};
|
||||
sf::Color last_fill_color{0, 0, 0, 0};
|
||||
sf::Vector2u last_render_tex_size{0, 0};
|
||||
|
||||
// Render textures
|
||||
sf::Sprite sprite_proto, output;
|
||||
sf::RenderTexture renderTexture;
|
||||
|
|
|
|||
117
tests/regression/issue_351_gridview_early_out_test.py
Normal file
117
tests/regression/issue_351_gridview_early_out_test.py
Normal file
|
|
@ -0,0 +1,117 @@
|
|||
"""Regression test for #351 - UIGridView clean-state render early-out.
|
||||
|
||||
UIGridView::render() now re-blits its cached RenderTexture instead of clearing
|
||||
and redrawing when nothing that affects the raster changed (camera parameters,
|
||||
compared directly, plus GridData::content_generation for grid content).
|
||||
|
||||
This test guards the DANGEROUS direction: a *false* early-out would display a
|
||||
STALE frame. It screenshots before/after each kind of mutation and asserts the
|
||||
rendered pixels actually changed -- proving every mutation still invalidates the
|
||||
cached raster. It also asserts an idle (no-mutation) pair is byte-identical, so
|
||||
the skip path itself does not corrupt output.
|
||||
|
||||
Headless screenshots are synchronous (#153): screenshot() renders-then-captures,
|
||||
so all mutations + checks run inline (no Timer dance required).
|
||||
"""
|
||||
import mcrfpy
|
||||
from mcrfpy import automation
|
||||
import sys, os, tempfile
|
||||
|
||||
TMPDIR = tempfile.mkdtemp(prefix="mcrf351_")
|
||||
_counter = 0
|
||||
_results = []
|
||||
|
||||
|
||||
def shot():
|
||||
"""Render a frame to PNG and return its bytes (identical pixels -> identical
|
||||
bytes with the deterministic PNG encoder)."""
|
||||
global _counter
|
||||
path = os.path.join(TMPDIR, "s%d.png" % _counter)
|
||||
_counter += 1
|
||||
automation.screenshot(path)
|
||||
with open(path, "rb") as f:
|
||||
return f.read()
|
||||
|
||||
|
||||
def check(label, before, after, expect_change):
|
||||
changed = (before != after)
|
||||
ok = (changed == expect_change)
|
||||
_results.append((label, ok))
|
||||
print(" [%s] %s: changed=%s (expected changed=%s)" %
|
||||
("PASS" if ok else "FAIL", label, changed, expect_change))
|
||||
|
||||
|
||||
def run_tests():
|
||||
texture = mcrfpy.Texture("assets/kenney_tinydungeon.png", 16, 16)
|
||||
|
||||
scene = mcrfpy.Scene("t351")
|
||||
scene.activate()
|
||||
|
||||
grid = mcrfpy.Grid(grid_size=(12, 10), pos=(20, 20), size=(320, 280), texture=texture)
|
||||
|
||||
tl = mcrfpy.TileLayer(name="terrain", z_index=-2, texture=texture)
|
||||
cl = mcrfpy.ColorLayer(name="fog", z_index=1)
|
||||
grid.add_layer(tl)
|
||||
grid.add_layer(cl)
|
||||
tl.fill(0)
|
||||
|
||||
e1 = mcrfpy.Entity((3, 3), grid=grid)
|
||||
e1.sprite_index = 5
|
||||
|
||||
scene.children.append(grid)
|
||||
|
||||
# Baseline raster (first render: full, records early-out state)
|
||||
a = shot()
|
||||
|
||||
# 1. Entity move via draw_pos (set_position -- the latent-bug path #351 fixed)
|
||||
e1.draw_pos = (6, 6)
|
||||
b = shot(); check("entity set_position", a, b, True)
|
||||
|
||||
# 2. Tile edit (TileLayer.set -> GridLayer::markDirty choke)
|
||||
tl.set((2, 2), 40)
|
||||
c = shot(); check("tile set", b, c, True)
|
||||
|
||||
# 3. Color edit (ColorLayer.set, opaque -> clearly visible overlay)
|
||||
cl.set((3, 3), mcrfpy.Color(255, 0, 0, 255))
|
||||
d = shot(); check("color set", c, d, True)
|
||||
|
||||
# 4. Add an entity (default camera -> cell (4,4) is on-screen)
|
||||
e2 = mcrfpy.Entity((4, 4), grid=grid)
|
||||
e2.sprite_index = 3
|
||||
e = shot(); check("entity add", d, e, True)
|
||||
|
||||
# 5. Entity die (removal)
|
||||
e2.die()
|
||||
f = shot(); check("entity die", e, f, True)
|
||||
|
||||
# 6. Camera pan (view center compare, no content bump)
|
||||
cx, cy = grid.center
|
||||
grid.center = (cx + 48, cy)
|
||||
g = shot(); check("camera pan", f, g, True)
|
||||
|
||||
# 7. Zoom (view compare)
|
||||
grid.zoom = 2.0
|
||||
h = shot(); check("zoom", g, h, True)
|
||||
|
||||
# 8. Entity move via animation + step (setProperty -> markCompositeDirty)
|
||||
e1.animate("draw_x", 9.0, 1.0, mcrfpy.Easing.LINEAR)
|
||||
mcrfpy.step(0.5)
|
||||
i = shot(); check("entity animate+step", h, i, True)
|
||||
|
||||
# 9. Idle: no mutation -> two frames must be byte-identical (skip path clean)
|
||||
j = shot()
|
||||
k = shot(); check("idle stable", j, k, False)
|
||||
|
||||
return all(ok for _, ok in _results)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
try:
|
||||
ok = run_tests()
|
||||
print("PASS" if ok else "FAIL")
|
||||
sys.exit(0 if ok else 1)
|
||||
except Exception as ex:
|
||||
import traceback
|
||||
traceback.print_exc()
|
||||
print("FAIL")
|
||||
sys.exit(1)
|
||||
Loading…
Add table
Add a link
Reference in a new issue