From f6718a2f380dec1830fef830685057c9ee0fb837 Mon Sep 17 00:00:00 2001 From: "YITSUSE, Masami" Date: Mon, 27 Jul 2026 17:07:45 +0900 Subject: [PATCH 1/2] fix: render mask coverage in the same frame it is sampled The coverage viewport was created without a parent viewport, so the server treated it as a root and drew it after the window it belongs to (_sort_active_viewports orders children first, parents last, and falls back to creation order for roots). Maskable parts therefore sampled the coverage rasterized on the previous frame. Parent the coverage target to the viewport its borrowing player renders into. The server then draws it ahead of that viewport, so the coverage a part samples is the one rendered this frame, and both the latency and the distortion it caused are gone. The UV transform handed to maskable shaders must move with it. It was held back a frame to match the bbox the sampled texels were actually rasterized with; now that the texels are this frame's, this frame's transform is the matching one, so publish it immediately. Keeping the hold would offset the lookup by a frame in the opposite direction. Same-frame rendering also retires the machinery that covered for the latency: the `fresh` flag that skipped masking for one frame after a (re)acquire, the `_mask_prev` target held one extra frame so a size-class transition could sample the outgoing coverage, and with them the one-frame gap while zooming across a class boundary. A freshly acquired target now carries this frame's coverage, from the first frame of masking onward. Targets are pooled and shared between players that may live under different viewports, so the parent link is re-established on every acquire, and on any host change while a target is borrowed. `viewport_set_parent_viewport` only assigns the parent - it does not flag the sorted viewport list dirty - so activation is toggled to force the re-sort. The link is also cleared on release: freeing a viewport does not detach its children, and a viewport whose parent no longer resolves is dropped from the render order entirely. Verified with examples/mask_uv_test under --fixed-fps 60, classifying pixels rather than eyeballing: D_Translate (20px/frame) now produces the same hole/reference statistics as A_Static on every sampled frame, so the alignment no longer depends on how fast the mask moves. B_Rotate and C_ScaleXY show no uncovered hole either, a sweep across the full scale range crosses size classes without dropping a frame, and masking is correct from the first rendered frame. --- ss_player/ss_internal_player.cpp | 124 +++++++++++++++++-------------- ss_player/ss_internal_player.h | 28 ++++--- ss_player/ss_player_node_2d.cpp | 12 +++ ss_player/ss_player_node_2d.h | 1 + 4 files changed, 96 insertions(+), 69 deletions(-) diff --git a/ss_player/ss_internal_player.cpp b/ss_player/ss_internal_player.cpp index c4887bf..bd432ba 100644 --- a/ss_player/ss_internal_player.cpp +++ b/ss_player/ss_internal_player.cpp @@ -78,7 +78,7 @@ struct SsMaskCoverageTarget { int canvas_items_in_use = 0; int w = 0; int h = 0; - bool fresh = false; // first frame after (re)acquire: skip masking once + RID parent_viewport; // host viewport this target currently renders under }; namespace { @@ -102,35 +102,45 @@ class SsMaskCoveragePool { } } - SsMaskCoverageTarget* acquire(RenderingServer* rs, int w, int h) { + SsMaskCoverageTarget* acquire(RenderingServer* rs, int w, int h, RID parent_viewport) { const uint64_t key = ((uint64_t)(uint32_t)w << 32) | (uint32_t)h; Vector* lst = _free.getptr(key); if (lst && !lst->is_empty()) { SsMaskCoverageTarget* t = (*lst)[lst->size() - 1]; lst->remove_at(lst->size() - 1); - t->fresh = true; + _set_parent(rs, t, parent_viewport); return t; } SsMaskCoverageTarget* t = memnew(SsMaskCoverageTarget); t->w = w; t->h = h; - t->fresh = true; t->viewport = rs->viewport_create(); - // UPDATE_ONCE: re-requested each frame the target is rendered. Maskable - // parts sample the previous frame's result (one-frame latency), which - // sidesteps inter-viewport render ordering. + // UPDATE_ONCE: re-requested each frame the target is rendered. The + // target renders under the borrowing player's host viewport (see + // `_set_parent`), so its coverage is available to that player's parts + // in the same frame. rs->viewport_set_update_mode(t->viewport, RS_VIEWPORT_UPDATE_ONCE); rs->viewport_set_clear_mode(t->viewport, RS_VIEWPORT_CLEAR_ALWAYS); rs->viewport_set_transparent_background(t->viewport, true); rs->viewport_set_disable_3d(t->viewport, true); - rs->viewport_set_active(t->viewport, true); rs->viewport_set_size(t->viewport, w, h); + // Parent before activating: activation is what flags the server's + // active-viewport order dirty (see `_set_parent`). + rs->viewport_set_parent_viewport(t->viewport, parent_viewport); + t->parent_viewport = parent_viewport; + rs->viewport_set_active(t->viewport, true); t->canvas = rs->canvas_create(); rs->viewport_attach_canvas(t->viewport, t->canvas); _all.push_back(t); return t; } + // Keep a still-borrowed target's parent link in sync with its borrower's + // host viewport (the node may have moved to another viewport since acquire). + void reparent(RenderingServer* rs, SsMaskCoverageTarget* t, RID parent_viewport) { + if (t) _set_parent(rs, t, parent_viewport); + } + void release(RenderingServer* rs, SsMaskCoverageTarget* t) { if (!t) { return; @@ -142,11 +152,35 @@ class SsMaskCoveragePool { } } t->canvas_items_in_use = 0; + // Drop the parent link while idle. The server never detaches children + // when a viewport is freed, and a viewport whose parent RID no longer + // resolves is dropped from the render order entirely — so an idle + // target must not keep pointing at a host that may go away. + _set_parent(rs, t, RID()); const uint64_t key = ((uint64_t)(uint32_t)t->w << 32) | (uint32_t)t->h; _free[key].push_back(t); } private: + // Re-parent a target's viewport. Targets are shared between players, which + // may live under different viewports, so this runs on every acquire (and on + // release, to clear the link). + // + // `viewport_set_parent_viewport` only assigns the parent; it does NOT flag + // the server's sorted active-viewport list dirty. `viewport_set_active` + // does, so toggle it to force the re-sort that puts this target ahead of + // its host. Without that, the new parent is ignored until something else + // dirties the order. + void _set_parent(RenderingServer* rs, SsMaskCoverageTarget* t, RID parent_viewport) { + if (t->parent_viewport == parent_viewport) { + return; + } + rs->viewport_set_parent_viewport(t->viewport, parent_viewport); + t->parent_viewport = parent_viewport; + rs->viewport_set_active(t->viewport, false); + rs->viewport_set_active(t->viewport, true); + } + void free_all(RenderingServer* rs) { for (int i = 0; i < _all.size(); i++) { SsMaskCoverageTarget* t = _all[i]; @@ -882,6 +916,19 @@ bool SsInternalPlayer::_build_mask_writers(const DrawFrame& f) { return !_mask_writers.is_empty(); } +void SsInternalPlayer::setHostViewport(RID p_viewport) { + if (_host_viewport == p_viewport) { + return; + } + _host_viewport = p_viewport; + // A target borrowed while under the old viewport would keep rendering (and + // sorting) under it, so move it across immediately. Pooled targets pick the + // current host up at acquire time. + if (_mask_target) { + SsMaskCoveragePool::get().reparent(RenderingServer::get_singleton(), _mask_target, _host_viewport); + } +} + void SsInternalPlayer::_acquire_mask_target(int w, int h) { if (_mask_write_shader.is_null()) { _mask_write_shader.instantiate(); @@ -896,25 +943,17 @@ void SsInternalPlayer::_acquire_mask_target(int w, int h) { } RenderingServer* rs = RenderingServer::get_singleton(); if (_mask_target) { - // Size-class transition: keep the outgoing target one more frame so the - // coverage pass can sample its still-valid coverage while the freshly - // acquired target warms up (see _render_mask_coverage). Any earlier - // leftover (defensive; normally cleared at the pass start) is returned. - if (_mask_prev) { - SsMaskCoveragePool::get().release(rs, _mask_prev); - } - _mask_prev = _mask_target; + // Size-class transition. The freshly acquired target is rendered before + // this player draws (it is parented to the host viewport), so it carries + // this frame's coverage and the outgoing one can go back right away. + SsMaskCoveragePool::get().release(rs, _mask_target); _mask_target = nullptr; } - _mask_target = SsMaskCoveragePool::get().acquire(rs, w, h); + _mask_target = SsMaskCoveragePool::get().acquire(rs, w, h, _host_viewport); } void SsInternalPlayer::_release_mask_target() { RenderingServer* rs = RenderingServer::get_singleton(); - if (_mask_prev) { - SsMaskCoveragePool::get().release(rs, _mask_prev); - _mask_prev = nullptr; - } if (_mask_target) { SsMaskCoveragePool::get().release(rs, _mask_target); _mask_target = nullptr; @@ -957,12 +996,6 @@ Ref SsInternalPlayer::_acquire_mask_write_material() { void SsInternalPlayer::_render_mask_coverage(const DrawFrame& f) { _mask_coverage_valid = false; - // Return the transition leftover held for last frame's sampling; if we are - // still transitioning, _acquire_mask_target re-establishes it below. - if (_mask_prev) { - SsMaskCoveragePool::get().release(RenderingServer::get_singleton(), _mask_prev); - _mask_prev = nullptr; - } if (_mask_writers.is_empty() || !f.frameData) return; // Borrow a pooled target of the size class decided last frame (stable thanks // to quantization), then re-request its one-shot render this frame. @@ -1134,14 +1167,12 @@ void SsInternalPlayer::_render_mask_coverage(const DrawFrame& f) { uv.columns[0] = Vector2(1.0f / bsize.x, 0); uv.columns[1] = Vector2(0, 1.0f / bsize.y); uv.columns[2] = Vector2(-bmin.x / bsize.x, -bmin.y / bsize.y); - // What the shaders sample this frame is the coverage rendered LAST frame (the - // UPDATE_ONCE latency below), rasterized with last frame's bbox -> viewport - // transform. So publish last frame's UV transform and hold this one back; - // publishing `uv` now would offset/scale the lookup by the per-frame bbox - // delta. The same holds on a size-class transition, where `_mask_prev` is - // sampled: it too was rendered with the previous frame's bbox. - _mask_local_to_uv = _mask_local_to_uv_pending; - _mask_local_to_uv_pending = uv; + // The coverage target renders under this player's host viewport, so what the + // shaders sample this frame is the coverage rasterized this frame, with the + // `ct` above. Publish the matching UV transform immediately: holding it back + // a frame would map positions through a bbox the sampled texels were never + // rasterized with, sliding the mask off its target as the bbox moves. + _mask_local_to_uv = uv; // Frame mask state consumed by the maskable emit path (P3). _mask_coverage_tex = rs->viewport_get_texture(_mask_target->viewport); @@ -1169,25 +1200,10 @@ void SsInternalPlayer::_render_mask_coverage(const DrawFrame& f) { _mask_next_h = ss_hysteretic_coverage_dim(bsize.y * _coverage_screen_scale, _mask_target->h, MASK_COVERAGE_MIN_DIM, MASK_COVERAGE_MAX_DIM, MASK_SIZE_SHRINK_HYSTERESIS); - // A freshly (re)acquired target still holds the previous tenant's coverage - // (or nothing), and with one-frame-latency sampling it is not ready this - // frame. On a size-class transition we instead sample `_mask_prev` — last - // frame's target, whose coverage is still valid (same one-frame latency as - // any normal frame, just at the previous resolution for this one frame) — - // so masking never drops for a frame while zooming across a class boundary. - if (_mask_target->fresh) { - _mask_target->fresh = false; - if (_mask_prev) { - _mask_coverage_tex = rs->viewport_get_texture(_mask_prev->viewport); - _mask_coverage_valid = true; - } else { - // Genuinely the first coverage frame (no previous target to fall back - // on) — nothing valid to sample yet, so skip masking this one frame. - _mask_coverage_valid = false; - } - } else { - _mask_coverage_valid = true; - } + // The target was re-requested (UPDATE_ONCE) above and renders ahead of this + // player's host viewport, so its coverage is this frame's — valid from the + // very first frame of masking and across size-class transitions alike. + _mask_coverage_valid = true; } bool SsInternalPlayer::_part_in_mask_scope(uint16_t rank) const { diff --git a/ss_player/ss_internal_player.h b/ss_player/ss_internal_player.h index f576dd8..8c46de9 100644 --- a/ss_player/ss_internal_player.h +++ b/ss_player/ss_internal_player.h @@ -182,6 +182,13 @@ class SsInternalPlayer { // wrapper each frame before update(); 1.0 until then. void setCoverageScreenScale(float p_scale) { _coverage_screen_scale = (p_scale > 0.0f) ? p_scale : 1.0f; } + // Viewport the owning Node2D renders into. The mask coverage target is + // parented to it so the server renders coverage before this player's parts + // sample it, in the same frame. Set by the Node2D wrapper on tree changes + // (an invalid RID when outside the tree, where no coverage pass runs); a + // target already borrowed is re-parented on the spot. + void setHostViewport(RID p_viewport); + void setCellMapOverrideTexture(uint32_t cellmap_name_hash, const Ref& texture); Ref getCellMapTexture(uint32_t cellmap_name_hash) const; @@ -534,29 +541,20 @@ class SsInternalPlayer { // geometry is rendered into it by `_render_mask_coverage`; maskable shaders // sample the result (P3). Acquired by `_acquire_mask_target`, returned by // `_release_mask_target` / `_free_mask_targets`. The viewport renders with - // UPDATE_ONCE (one-frame latency, sidesteps inter-viewport ordering). + // UPDATE_ONCE, parented to `_host_viewport` so it is drawn before the parts + // that sample it. SsMaskCoverageTarget* _mask_target = nullptr; // borrowed; null when idle - // The target used on the PREVIOUS frame, held one extra frame across a - // size-class transition. On the transition frame the freshly acquired - // `_mask_target` has no coverage yet (one-frame viewport latency), so the - // coverage pass samples this still-valid previous target instead of dropping - // masking for a frame. Released at the start of the next coverage pass (and on - // teardown). null when not mid-transition. - SsMaskCoverageTarget* _mask_prev = nullptr; + RID _host_viewport; // viewport the owning node draws into bool _mask_pool_registered = false; // this player counts as a pool user Ref _mask_write_shader; // loaded from SS_MASK_WRITE Vector> _mask_write_materials; // per-instance, pooled int _mask_write_materials_in_use = 0; // Maps this player's local space (the space batch geometry lives in, world // matrices already applied) -> coverage UV [0,1]. Set per frame by the - // coverage pass and read by maskable shaders. Identity until masking runs. + // coverage pass and read by maskable shaders. Derived from the same writer + // bbox the coverage was rasterized with this frame. Identity until masking + // runs. Transform2D _mask_local_to_uv; - // The UV transform derived from THIS frame's writer bbox. Shaders sample the - // coverage rendered on the PREVIOUS frame (UPDATE_ONCE latency), so this is - // held back one frame and only then promoted to `_mask_local_to_uv` — using - // it immediately would map positions through a bbox the sampled texels were - // never rasterized with, sliding the mask off its target as the bbox moves. - Transform2D _mask_local_to_uv_pending; bool _mask_coverage_valid = false; // true if this frame drew coverage // Coverage-bitmap dimension bounds in pixels. The per-axis size is the // mask's on-screen footprint, quantized up to a power-of-two size class in diff --git a/ss_player/ss_player_node_2d.cpp b/ss_player/ss_player_node_2d.cpp index 4442f8b..a7414c0 100644 --- a/ss_player/ss_player_node_2d.cpp +++ b/ss_player/ss_player_node_2d.cpp @@ -2,8 +2,10 @@ #ifdef SPRITESTUDIO_GODOT_EXTENSION #include +#include #else #include "core/config/engine.h" +#include "scene/main/viewport.h" #endif class SpriteStudioPlayer2D::_SignalSink : public SsPlayerEventSink { @@ -322,6 +324,14 @@ void SpriteStudioPlayer2D::_push_coverage_screen_scale() { _internal->setCoverageScreenScale(sx > sy ? sx : sy); } +void SpriteStudioPlayer2D::_push_host_viewport() { + // The mask coverage target is parented to this viewport so the server draws + // it before us. Tree changes are the only thing that can move a node to + // another viewport, so pushing it there keeps the link current. + Viewport* vp = get_viewport(); + _internal->setHostViewport(vp ? vp->get_viewport_rid() : RID()); +} + void SpriteStudioPlayer2D::set_animation_process_mode(int p_mode) { AnimationProcessMode mode = (AnimationProcessMode)p_mode; if (_process_mode == mode) return; @@ -735,6 +745,7 @@ void SpriteStudioPlayer2D::_notification(int p_notification) { // here so editor reloads (which destroy / reattach the canvas) // don't leave the InternalPlayer floating. _internal->setParentCanvasItem(get_canvas_item()); + _push_host_viewport(); if (_process_mode == ANIMATION_PROCESS_IDLE) { set_process_internal(true); } else { @@ -743,6 +754,7 @@ void SpriteStudioPlayer2D::_notification(int p_notification) { break; case NOTIFICATION_EXIT_TREE: _internal->setParentCanvasItem(RID()); + _internal->setHostViewport(RID()); // The pooled AudioStreamPlayer children leave the tree with us and // stop; reset the controller's bookkeeping to match. if (_audio_controller) _audio_controller->stop_all(); diff --git a/ss_player/ss_player_node_2d.h b/ss_player/ss_player_node_2d.h index 9d969f1..2e1da4a 100644 --- a/ss_player/ss_player_node_2d.h +++ b/ss_player/ss_player_node_2d.h @@ -170,6 +170,7 @@ class SpriteStudioPlayer2D : public Node2D { Transform2D _make_root_transform() const; void _update_root_transform(); void _push_coverage_screen_scale(); + void _push_host_viewport(); // Adapter that turns SsInternalPlayer event callbacks into Node-level // emit_signal calls. Lifetime tied to the Node; lives in the cpp file. From 8717c2c20269c433de3290c2df83cf2fc47914aa Mon Sep 17 00:00:00 2001 From: "YITSUSE, Masami" Date: Mon, 27 Jul 2026 17:21:39 +0900 Subject: [PATCH 2/2] fix: keep Permanent part overrides across an animation change Changing animation re-borrowed and re-bound the SSAB: _fetchAnimation destroyed the resource handle, created a fresh borrow of the same buffer and called ss_runtime_bind_resource, on every setAnimation. Binding tells the runtime the part identities may be new, so it drops every part override - correct for a new SSAB, but it also ran for a move between two animations of the same one, taking the Permanent-priority overrides with it. docs/ja/api/player.md documents those as persisting for as long as the same .ssab plays. The runtime's own handling was already right: setup_animation_by_hash calls retain_permanent_overrides_on_setup, which clears Visibility and the NON/NOW Color/Cell overrides and keeps the Permanent ones. It just never had anything left to keep, because the bind ahead of it had cleared the whole table. Re-bind only when the resource itself changed. setSSABResource flags it - covering both a different resource and the same one reloaded from disk through onSSABReloaded, whose buffer may have moved - and _fetchAnimation clears the flag once it has re-bound. An animation change within one SSAB now keeps the borrow. Nothing else is skipped: setup_animation_by_hash collapses to the primary source and resizes every layer (which clears the decode cursors) on its own, and the FlatBuffer builder is reset per frame-data build. Verified with examples/mask_uv_test/perm_override_test.gd, sampling the same pixel in both builds: with a Permanent override set and one `animation = ...`, the part read (0,0,0) before and reads the override colour (41,107,217) after, while the control that never switches reads the override colour in both. Re-assigning the resource still drops the override, by design, and the part keeps drawing. The mask captures are bit-identical across this change. --- ss_player/ss_internal_player.cpp | 45 ++++++++++++++++++++------------ ss_player/ss_internal_player.h | 5 ++++ 2 files changed, 34 insertions(+), 16 deletions(-) diff --git a/ss_player/ss_internal_player.cpp b/ss_player/ss_internal_player.cpp index bd432ba..8a055e4 100644 --- a/ss_player/ss_internal_player.cpp +++ b/ss_player/ss_internal_player.cpp @@ -304,6 +304,9 @@ void SsInternalPlayer::setSSABResource(const Ref& ssabRes) { _ssabRes = ssabRes; _strAnimationSelected = ""; _animationSelectedHash = 0; + // A different resource — or the same one reloaded from disk, which may have + // moved its buffer — invalidates the borrow the runtime holds. + _res_rebind_pending = true; if (!_ssabRes.is_null()) { if (!_ssabRes->is_valid()) { @@ -2814,6 +2817,7 @@ void SsInternalPlayer::_fetchAnimation() { ss_resource_destroy(runtime_res); runtime_res = nullptr; } + _res_rebind_pending = true; _currentAnimationData = nullptr; // Free instance children before their parent canvas items — each // child's _root_ci is parented to a batch CI from @@ -2840,24 +2844,33 @@ void SsInternalPlayer::_fetchAnimation() { // update() / _emit_effect_slot dereference it on the next tick. _currentAnimationData = nullptr; - if (runtime_res != nullptr) { - ss_resource_destroy(runtime_res); - runtime_res = nullptr; - } + // Re-borrow and re-bind only when the SSAB itself changed. Binding tells the + // runtime the part identities may be new, so it drops every part override — + // correct for a new SSAB, wrong for an animation change within one, where + // Permanent-priority overrides are documented to persist (the runtime's + // setup step keeps them). Re-binding here made every `animation = ...` + // discard them. + if (_res_rebind_pending || runtime_res == nullptr) { + if (runtime_res != nullptr) { + ss_resource_destroy(runtime_res); + runtime_res = nullptr; + } - // Borrow (do not copy) the resource's buffer to keep loading zero-copy. The - // buffer must remain valid and unmodified until runtime_res is destroyed; - // every resource change re-creates this borrow via _fetchAnimation. - runtime_res = ss_resource_create_borrow(_ssabRes->get_data_ptr(), _ssabRes->get_data_size()); - if (runtime_res == nullptr) { - ERR_PRINT("SSAB Resource Create Failed"); - return; - } + // Borrow (do not copy) the resource's buffer to keep loading zero-copy. + // The buffer must remain valid and unmodified until runtime_res is + // destroyed; every resource change re-creates this borrow. + runtime_res = ss_resource_create_borrow(_ssabRes->get_data_ptr(), _ssabRes->get_data_size()); + if (runtime_res == nullptr) { + ERR_PRINT("SSAB Resource Create Failed"); + return; + } - bool binded = ss_runtime_bind_resource(runtime_ctx, runtime_res); - if (!binded) { - ERR_PRINT("SSAB Resource Bind Failed"); - return; + bool binded = ss_runtime_bind_resource(runtime_ctx, runtime_res); + if (!binded) { + ERR_PRINT("SSAB Resource Bind Failed"); + return; + } + _res_rebind_pending = false; } // Per-batch canvas_item pool grows on demand inside _drawAnimation via diff --git a/ss_player/ss_internal_player.h b/ss_player/ss_internal_player.h index 8c46de9..dae7c79 100644 --- a/ss_player/ss_internal_player.h +++ b/ss_player/ss_internal_player.h @@ -343,6 +343,11 @@ class SsInternalPlayer { const ss::format::AnimationData* _currentAnimationData = nullptr; void* runtime_ctx = nullptr; void* runtime_res = nullptr; + // Set whenever the assigned SSAB (or its buffer) changes, cleared once + // `_fetchAnimation` has re-borrowed and re-bound it. An animation change + // within the same SSAB must NOT re-bind: binding drops every part override, + // including the Permanent-priority ones documented to survive it. + bool _res_rebind_pending = true; float previous_frame_no = -1.0f; float _speed_rate = 1.0f; bool _sub_frame_enabled = false;