From e456b7eb9877843ebf88f8b1d39e9c881ac613ee Mon Sep 17 00:00:00 2001 From: Naruto TAKAHASHI Date: Sat, 25 Jul 2026 17:16:49 +0900 Subject: [PATCH] fix: correct stale animation state, mask suppression and editor scans MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A review pass over ss_player/ turned up a set of independent defects. They are unrelated to each other but all small, so they land together. Memory safety - _fetchAnimation() left _currentAnimationData pointing into the previous resource's FlatBuffer on all four of its failure paths. setSSABResource() drops the last Ref to the old resource before re-fetching, so a failed borrow/bind then left update() and _emit_effect_slot() dereferencing freed memory on the next tick. Clear it up front instead, and make the effect slot's fps read null-safe. - The passed-event index returned by the runtime was fed straight into events()->Get(). A not-found (-1) or an index belonging to a previously bound animation read past the mapped buffer; bounds-check it. Masking - Pure-mask suppression tested only the first part of a batch and then skipped the whole batch. Normal/Shape/Mesh batches group parts by texture and blend, so a draw_as_mask part could either drag its whole batch out of the frame or, at rank 1..n, draw its own colour over the scene. Test per part inside the emit loops instead; the single-part kinds keep a batch-level check. - That test was also gated on _mask_coverage_valid, which is never true for instance children (the parent owns the coverage pass) nor on the first frame after a coverage target is acquired. Mask art was therefore visible in both cases. Suppression now depends only on the writer list, backed by a per-part lookup table so it stays O(1). - _mask_local_to_uv was rebuilt from the current frame's writer bbox, but the coverage texture maskable shaders sample is the previous frame's render (UPDATE_ONCE latency). A moving or resizing mask was therefore sampled through a bbox its texels were never rasterized with. Hold the transform back one frame so it matches the texture being read. Editor - ss_resource_inspector.cpp called EditorInterface::get_resource_filesystem() unguarded, which does not compile as a custom module against Godot 4.4/4.5. Use the same #if / get_resource_file_system() fallback as the other two call sites. - SSImportControl::stop_intercepting() returned early when get_window() was null, leaving the editor's own files_dropped handlers disconnected and is_intercepting stuck true. Always clear the intercept state. - Dropping a folder called importer->queue_scan_and_import() without a null check, unlike every sibling path. - _ensure_output_dir_exists() registered a freshly created directory with scan_changes()/scan_sources(), which are mtime-driven and miss new folders on Windows. Use a full scan(), as the importer's own sync phase does. - The generated AnimationLibrary keyed `frame` to total_frame at t=length, one past the last valid frame. Key the last valid frame at its own start time so the interpolated rate is exactly one frame per 1/fps. Player API - get_part_transform() ignored the flip/offset matrix that the player applies to its internal root canvas item, so a SpriteStudioPartAttachment2D detached as soon as flip_h or offset was set. Compose it, and say so in the API docs. Performance - _emit_normal_batch's flush_default_run() trimmed only the index array; the vertex/uv/color/custom0 accumulators are peak-retained, so every small run uploaded the largest batch's vertex count and derived its surface AABB from stale positions. Trim them to the run and restore afterwards. - SSABResource::is_valid() ran a full FlatBuffers verification of the whole buffer, and nearly every accessor gates on it. Cache the verdict and reset it wherever the buffer is replaced. Also documents, in ss_mask_write.gdshader, that additive blending only implements OR while no texel receives the same bit twice — a single writer whose own triangles overlap corrupts the neighbouring bit. Godot's canvas render modes offer no saturating blend, so fixing that needs a stencil-style write rather than a shader change. --- docs/en/api/player.md | 2 +- docs/ja/api/player.md | 2 +- ss_player/shaders/ss_mask_write.gdshader | 9 +++ ss_player/ss_import_dock.cpp | 44 +++++++---- ss_player/ss_internal_player.cpp | 93 ++++++++++++++++++++---- ss_player/ss_internal_player.h | 15 +++- ss_player/ss_player_node_2d.cpp | 16 +++- ss_player/ss_player_node_2d.h | 4 + ss_player/ss_resource_inspector.cpp | 19 ++++- ss_player/ssab_resource.cpp | 16 ++++ ss_player/ssab_resource.h | 6 ++ 11 files changed, 187 insertions(+), 39 deletions(-) diff --git a/docs/en/api/player.md b/docs/en/api/player.md index bca4790..1209a8b 100644 --- a/docs/en/api/player.md +++ b/docs/en/api/player.md @@ -57,7 +57,7 @@ func _ready() -> void: * `get_part_names() -> PackedStringArray`: Every part name in the current animation. * `get_part_index(part_name: String) -> int`: Resolves a part name to its part index, or `-1` if it does not exist. -* `get_part_transform(part_name: String) -> Transform2D`: The part's local `Transform2D` on the current frame. Returns the identity when the part is unknown. +* `get_part_transform(part_name: String) -> Transform2D`: The part's `Transform2D` on the current frame, in the player node's local space (`flip_h` / `flip_v` / `offset` included). Returns the identity when the part is unknown. * `is_part_hidden(part_name: String) -> bool`: Whether the part is hidden on the current frame. ## Part overrides diff --git a/docs/ja/api/player.md b/docs/ja/api/player.md index 2f1d6c8..918db77 100644 --- a/docs/ja/api/player.md +++ b/docs/ja/api/player.md @@ -57,7 +57,7 @@ func _ready() -> void: * `get_part_names() -> PackedStringArray`: 現在のアニメーションに含まれる全パーツ名。 * `get_part_index(part_name: String) -> int`: パーツ名をパーツインデックスへ解決します。存在しない場合は `-1`。 -* `get_part_transform(part_name: String) -> Transform2D`: 現在のフレームでのパーツのローカル `Transform2D`。パーツが不明な場合は単位行列を返します。 +* `get_part_transform(part_name: String) -> Transform2D`: 現在のフレームでのパーツの `Transform2D`（プレイヤーノードのローカル空間。`flip_h` / `flip_v` / `offset` を含みます）。パーツが不明な場合は単位行列を返します。 * `is_part_hidden(part_name: String) -> bool`: 現在のフレームでそのパーツが非表示かどうか。 ## パーツオーバーライド diff --git a/ss_player/shaders/ss_mask_write.gdshader b/ss_player/shaders/ss_mask_write.gdshader index 32297a9..ef7bcfb 100644 --- a/ss_player/shaders/ss_mask_write.gdshader +++ b/ss_player/shaders/ss_mask_write.gdshader @@ -7,6 +7,15 @@ R"GLSL( // RGBA8 channel (e.g. R for bits 0..7, value (1<<(bit%8))/255). Additive // blending then ORs the disjoint bits together across writers. // +// LIMITATION: add is only equivalent to OR while no texel receives the same bit +// twice. That holds across writers (their bits are disjoint) and for the quad / +// pentagon and triangulated-shape geometry we emit, but a single writer whose +// own triangles overlap (e.g. a bone-deformed mesh folded onto itself) adds its +// bit twice into the shared texels, which carries into the neighbouring bit — +// those texels then read as a different writer's mask. Godot's canvas render +// modes offer no saturating (max) blend, so fixing this properly needs a +// stencil-style write rather than a shader change. +// // Cutout: a texel contributes to the mask only where the writer sprite's alpha // is strictly greater than `mask_threshold`. The player derives the threshold // from the part's `mask` attribute (PartState.mask()); mask == 0 maps to diff --git a/ss_player/ss_import_dock.cpp b/ss_player/ss_import_dock.cpp index dc2c3eb..af12868 100644 --- a/ss_player/ss_import_dock.cpp +++ b/ss_player/ss_import_dock.cpp @@ -290,22 +290,25 @@ void SSImportControl::start_intercepting() { void SSImportControl::stop_intercepting() { if (!is_intercepting) return; - auto window = get_window(); - if (!window) return; + // Drop the intercept state up-front: every exit path below must leave this + // control un-intercepting, or a later start_intercepting() early-returns and + // the editor's own files_dropped handlers stay disconnected for the session. + is_intercepting = false; - if (window->is_connected("files_dropped", Callable(this, "_on_window_files_dropped"))) { - window->disconnect("files_dropped", Callable(this, "_on_window_files_dropped")); - } + auto window = get_window(); + if (window) { + if (window->is_connected("files_dropped", Callable(this, "_on_window_files_dropped"))) { + window->disconnect("files_dropped", Callable(this, "_on_window_files_dropped")); + } - for (int i = 0; i < original_drop_handlers.size(); i++) { - const Callable &handler = original_drop_handlers[i]; - if (handler.is_valid() && !window->is_connected("files_dropped", handler)) { - window->connect("files_dropped", handler); + for (int i = 0; i < original_drop_handlers.size(); i++) { + const Callable &handler = original_drop_handlers[i]; + if (handler.is_valid() && !window->is_connected("files_dropped", handler)) { + window->connect("files_dropped", handler); + } } } original_drop_handlers.clear(); - - is_intercepting = false; } #ifdef SPRITESTUDIO_GODOT_EXTENSION @@ -359,6 +362,10 @@ void SSImportControl::_on_window_files_dropped(const Vector &p_files) { if (!dirs.is_empty()) { // A folder was dropped: scan it (and any loose .sspj) and import all. + if (!importer) { + ERR_PRINT("SSImportControl: importer is not set."); + return; + } importer->queue_scan_and_import(dirs, sspj_files, path_line_edit->get_text()); } else { _start_import(sspj_files); @@ -701,11 +708,16 @@ void SSImportControl::_ensure_output_dir_exists() { auto *efs = EditorInterface::get_singleton()->get_resource_file_system(); #endif if (!efs) return; -#ifdef SPRITESTUDIO_GODOT_EXTENSION - efs->scan_sources(); -#else - efs->scan_changes(); -#endif + // Full scan: scan_sources()/scan_changes() are mtime-driven and do not + // reliably notice a brand-new directory (parent-mtime granularity on + // Windows), so the folder would not show up in the FileSystem dock until an + // unrelated rescan. The importer's own sync phase uses a full scan for the + // same reason (see SSImporter::_poll_fs_sync). A scan issued while one is + // already running is dropped, so skip it then — the in-flight scan (and the + // importer's own, on the next import) covers the new folder. + if (!efs->is_scanning()) { + efs->scan(); + } } #endif // #ifdef TOOLS_ENABLED diff --git a/ss_player/ss_internal_player.cpp b/ss_player/ss_internal_player.cpp index 2fae113..c4887bf 100644 --- a/ss_player/ss_internal_player.cpp +++ b/ss_player/ss_internal_player.cpp @@ -721,11 +721,18 @@ void SsInternalPlayer::update(float delta_seconds) { // — a timing-dependent loss seen in the editor preview but not a running // project. Events must fire whenever the runtime reports them, regardless of // whether we also redraw. - if (_currentAnimationData && _currentAnimationData->events() != nullptr) { + const auto* events = _currentAnimationData ? _currentAnimationData->events() : nullptr; + if (events != nullptr) { + const uint32_t events_size = events->size(); int event_count = ss_runtime_get_passed_event_count(runtime_ctx); for (int i = 0; i < event_count; i++) { int event_idx = ss_runtime_get_passed_event_index(runtime_ctx, i); - auto events_per_frame = _currentAnimationData->events()->Get(event_idx); + // The index comes from the runtime and is only meaningful against the + // animation currently bound there; a not-found (-1) or an index left + // over from a different animation would read past the FlatBuffer. + if (event_idx < 0 || (uint32_t)event_idx >= events_size) continue; + auto events_per_frame = events->Get((uint32_t)event_idx); + if (!events_per_frame) continue; if (auto users = events_per_frame->users()) { for (uint32_t j = 0; j < users->size(); j++) { @@ -819,12 +826,22 @@ void SsInternalPlayer::update(float delta_seconds) { bool SsInternalPlayer::_build_mask_writers(const DrawFrame& f) { _mask_writers.clear(); + // Clear before any early return: a part that stopped masking this frame must + // stop being suppressed by the emit paths. + if (!_part_pure_mask.is_empty()) { + memset(_part_pure_mask.ptr(), 0, _part_pure_mask.size()); + } if (!f.frameData || !f.binary) return false; auto parts_meta = f.binary->parts(); auto draw_order = f.frameData->draw_order(); if (!parts_meta || !draw_order) return false; const int total_meta = (int)parts_meta->size(); + if ((int)_part_pure_mask.size() != total_meta) { + _part_pure_mask.resize(total_meta); + if (total_meta > 0) memset(_part_pure_mask.ptr(), 0, total_meta); + } + uint8_t* pure_mask_flags = _part_pure_mask.ptr(); const uint32_t n = draw_order->size(); for (uint32_t rank = 0; rank < n; rank++) { const int p_idx = (int)draw_order->Get(rank); @@ -842,7 +859,14 @@ bool SsInternalPlayer::_build_mask_writers(const DrawFrame& f) { const bool writes = pure_mask || pd->mask_write(); if (!writes) continue; - if ((int)_mask_writers.size() >= MAX_MASK_WRITERS) break; // bitmap holds 32 + // Flag it even when the writer list is already full: a pure mask that does + // not fit the bitmap still masks nothing, so it must not fall back to + // drawing its own colour over the scene. + if (pure_mask) pure_mask_flags[p_idx] = 1; + + // Bitmap holds 24 writers; keep scanning past that so the pure masks that + // did not fit are still flagged above. + if ((int)_mask_writers.size() >= MAX_MASK_WRITERS) continue; MaskWriter w; w.part_index = p_idx; @@ -1110,7 +1134,14 @@ 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); - _mask_local_to_uv = uv; + // 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; // Frame mask state consumed by the maskable emit path (P3). _mask_coverage_tex = rs->viewport_get_texture(_mask_target->viewport); @@ -1167,10 +1198,8 @@ bool SsInternalPlayer::_part_in_mask_scope(uint16_t rank) const { } bool SsInternalPlayer::_is_pure_mask_part(int p_idx) const { - for (int i = 0; i < _mask_writers.size(); i++) { - if (_mask_writers[i].part_index == p_idx) return !_mask_writers[i].is_clipping; - } - return false; + if (p_idx < 0 || (uint32_t)p_idx >= _part_pure_mask.size()) return false; + return _part_pure_mask[p_idx] != 0; } void SsInternalPlayer::_set_mask_uv_uniform(Ref mat, const Transform2D& local_to_uv) { @@ -1346,16 +1375,16 @@ void SsInternalPlayer::_drawAnimation(float frame_no, float delta_seconds, bool // Pure masks feed only the coverage bitmap and must not draw their own // colour (PartTypeMask has no draw branch below; this also suppresses // shape/text/nines *_mask color draws so they read as holes, not fills). - if (_mask_coverage_valid && batch->count() > 0 - && _is_pure_mask_part((int)draw_order_data[batch->start_rank()])) { - continue; - } + // Normal / Shape / Mesh batches group several parts by texture + blend + // and can mix mask and non-mask parts, so those test per part inside + // their emit loops; the single-part kinds are handled here. if (kind == ss::runtime::DrawBatchKind_Normal) { _emit_normal_batch(f, ci, batch, draw_order_data); } else if (kind == ss::runtime::DrawBatchKind_Shape) { _emit_shape_batch(f, ci, batch, draw_order_data); } else if (kind == ss::runtime::DrawBatchKind_Instance) { int p_idx = (int)draw_order_data[batch->start_rank()]; + if (_is_pure_mask_part(p_idx)) continue; const auto* part = (p_idx >= 0 && p_idx < (int)_parts_by_idx.size()) ? _parts_by_idx[p_idx] : nullptr; if (!part) continue; const float* drawing_m = f.get_world_matrix(p_idx); @@ -1363,6 +1392,7 @@ void SsInternalPlayer::_drawAnimation(float frame_no, float delta_seconds, bool _emit_instance_slot(f, ci, p_idx, drawing_m, batch->start_rank()); } else if (kind == ss::runtime::DrawBatchKind_Effect) { int p_idx = (int)draw_order_data[batch->start_rank()]; + if (_is_pure_mask_part(p_idx)) continue; const auto* part = (p_idx >= 0 && p_idx < (int)_parts_by_idx.size()) ? _parts_by_idx[p_idx] : nullptr; if (!part) continue; const float* drawing_m = f.get_world_matrix(p_idx); @@ -1675,7 +1705,10 @@ void SsInternalPlayer::_emit_effect_slot(const DrawFrame& f, RID ci, int p_idx, // The returned EffectDrawPlan carries everything Godot needs to draw: // commands keyed by cellmap_hash + blend, plus flat verts/uvs/colors/indices. const ss_effect_event_info ev = ss_runtime_get_active_effect_event(runtime_ctx, (uint32_t)p_idx); - const float fps = _currentAnimationData->fps() > 0 ? (float)_currentAnimationData->fps() : 60.0f; + // _currentAnimationData is null whenever _fetchAnimation bailed out; the + // effect simulator still needs a sane frame rate to step with. + const float fps = (_currentAnimationData && _currentAnimationData->fps() > 0) + ? (float)_currentAnimationData->fps() : 60.0f; const ss_effect_step_result step = ss_effect_slot_step( slot.effect_slot, ev, f.frame_no, f.delta_seconds, fps, f.parent_looped, /*y_flip*/ true, /*vert_stride*/ 2); @@ -2259,6 +2292,11 @@ void SsInternalPlayer::_emit_normal_batch(const DrawFrame& f, RID ci, SsFloatArray& custom0 = _normal_custom0; SsIntArray& indices = _normal_indices; + // The accumulators are peak-retained (grown above, never shrunk), so their + // size is the largest batch seen so far, not this run's. Remember the + // capacity to restore after each run trims itself down for the emit. + const int verts_cap = verts.size(); // uvs / colors track this exactly + Vector2* verts_ptr = verts.ptrw(); Vector2* uvs_ptr = uvs.ptrw(); Color* colors_ptr = colors.ptrw(); @@ -2316,7 +2354,16 @@ void SsInternalPlayer::_emit_normal_batch(const DrawFrame& f, RID ci, } else { run_ci = _acquire_per_part_canvas_item(); // fresh CI, draw_index = _draw_seq++ } + // Trim every stream to what this run actually wrote. mesh_add_surface_from_arrays + // uploads whatever it is handed and derives the surface AABB from + // ARRAY_VERTEX, so leaving the peak-retained tail in place would upload + // the previous (larger) batch's vertex count on every small run and size + // the culling rect against geometry that is not drawn. indices.resize(ibase); + verts.resize(vbase); + uvs.resize(vbase); + colors.resize(vbase); + custom0.resize(vbase * 4); _apply_partcolor_material(rs, run_ci, s_default_shader_id_hash, ssab_blend); rs->canvas_item_set_transform(run_ci, Transform2D()); _emit_partcolor_mesh(rs, run_ci, indices, verts, colors, uvs, custom0, tex_rid); @@ -2324,6 +2371,10 @@ void SsInternalPlayer::_emit_normal_batch(const DrawFrame& f, RID ci, // re-fetch the write pointers (the arrays were shared with the emitted // mesh during the call above). indices.resize(index_count); + verts.resize(verts_cap); + uvs.resize(verts_cap); + colors.resize(verts_cap); + custom0.resize(verts_cap * 4); verts_ptr = verts.ptrw(); uvs_ptr = uvs.ptrw(); colors_ptr = colors.ptrw(); @@ -2336,6 +2387,9 @@ void SsInternalPlayer::_emit_normal_batch(const DrawFrame& f, RID ci, for (uint16_t k = 0; k < count; k++) { int p_idx = (int)draw_order_data[batch->start_rank() + k]; if (p_idx < 0 || p_idx >= (int)_parts_by_idx.size()) continue; + // Pure masks feed the coverage bitmap only; drawing their colour would + // paint the mask art over the scene instead of cutting a hole in it. + if (_is_pure_mask_part(p_idx)) continue; const auto* part = _parts_by_idx[p_idx]; if (!part) continue; @@ -2492,6 +2546,8 @@ void SsInternalPlayer::_emit_shape_batch(const DrawFrame& f, RID ci, for (uint16_t k = 0; k < count; k++) { int p_idx = (int)draw_order_data[batch->start_rank() + k]; if (p_idx < 0 || p_idx >= (int)_parts_by_idx.size()) continue; + // A shape flagged as a mask is coverage-only (see _emit_normal_batch). + if (_is_pure_mask_part(p_idx)) continue; const auto* part = _parts_by_idx[p_idx]; if (!part) continue; @@ -2600,6 +2656,8 @@ void SsInternalPlayer::_emit_mesh_batch(const DrawFrame& f, RID ci, for (uint16_t k = 0; k < count; k++) { int p_idx = (int)draw_order_data[batch->start_rank() + k]; if (p_idx < 0 || p_idx >= (int)_parts_by_idx.size()) continue; + // A mesh flagged as a mask is coverage-only (see _emit_normal_batch). + if (_is_pure_mask_part(p_idx)) continue; const auto* part = _parts_by_idx[p_idx]; if (!part) continue; @@ -2760,6 +2818,12 @@ void SsInternalPlayer::_fetchAnimation() { _free_per_part_canvas_items(); _free_mask_targets(); + // Drop the previous animation up-front: it points into the OLD resource's + // FlatBuffer, which setSSABResource may already have freed by releasing the + // last Ref. Every failure path below returns without re-assigning it, and + // update() / _emit_effect_slot dereference it on the next tick. + _currentAnimationData = nullptr; + if (runtime_res != nullptr) { ss_resource_destroy(runtime_res); runtime_res = nullptr; @@ -2806,6 +2870,9 @@ void SsInternalPlayer::_fetchAnimation() { bool setup = ss_runtime_setup_animation_by_hash(runtime_ctx, _animationSelectedHash); if (!setup) { ERR_PRINT("SSAB Setup Animation Failed by hash: " + String::num_int64(_animationSelectedHash)); + // The runtime has no animation bound, so nothing may consume this frame's + // animation data either. + _currentAnimationData = nullptr; return; } diff --git a/ss_player/ss_internal_player.h b/ss_player/ss_internal_player.h index f936bb0..f576dd8 100644 --- a/ss_player/ss_internal_player.h +++ b/ss_player/ss_internal_player.h @@ -518,6 +518,10 @@ class SsInternalPlayer { // premultiplied-blend coverage accumulator), so cap at 24 writers / frame. static constexpr int MAX_MASK_WRITERS = 24; Vector _mask_writers; + // Per part index (parallel to `_parts_by_idx`): 1 when the part is a "pure" + // mask this frame. Rebuilt with `_mask_writers` so the emit paths can test it + // once per part without walking the writer list. + LocalVector _part_pure_mask; // Populate `_mask_writers` from this frame's draw_order + static PartData. // Returns true if at least one writer is present (i.e., masking is active // this frame). Only the top-root player owns the mask state. @@ -547,6 +551,12 @@ class SsInternalPlayer { // matrices already applied) -> coverage UV [0,1]. Set per frame by the // coverage pass and read by maskable shaders. 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 @@ -593,7 +603,10 @@ class SsInternalPlayer { bool _part_in_mask_scope(uint16_t rank) const; // True if the part is a "pure" mask (PartTypeMask or a shape/text/nines // *_mask): it feeds the coverage bitmap but must not draw its own colour. - // write_mask (clipping) writers are NOT pure — they draw AND mask. + // write_mask (clipping) writers are NOT pure — they draw AND mask. Valid for + // any player with writers this frame, coverage rendered or not (an instance + // child's mask parts must stay invisible even though the parent owns the + // coverage pass), so callers must NOT gate it on `_mask_coverage_valid`. bool _is_pure_mask_part(int p_idx) const; void _apply_mask_uniforms(Ref mat, uint16_t rank, bool visible_inside); void _set_mask_uv_uniform(Ref mat, const Transform2D& local_to_uv); diff --git a/ss_player/ss_player_node_2d.cpp b/ss_player/ss_player_node_2d.cpp index fa6ea0d..4442f8b 100644 --- a/ss_player/ss_player_node_2d.cpp +++ b/ss_player/ss_player_node_2d.cpp @@ -136,8 +136,12 @@ int SpriteStudioPlayer2D::get_part_index(const String& part_name) const { Transform2D SpriteStudioPlayer2D::get_part_transform(const String& part_name) const { Transform2D xf; int idx = _internal->resolve_part_index(part_name); - if (idx >= 0) { - _internal->try_get_part_local_transform(idx, xf); + if (idx >= 0 && _internal->try_get_part_local_transform(idx, xf)) { + // The runtime's world matrix is relative to the internal root canvas + // item, which carries flip / offset. Compose it here so the result is + // the part's transform in THIS node's local space — otherwise an + // attachment pinned to a part detaches the moment the player is flipped. + xf = _make_root_transform() * xf; } return xf; } @@ -294,12 +298,16 @@ void SpriteStudioPlayer2D::set_offset(const Vector2& p_offset) { Vector2 SpriteStudioPlayer2D::get_offset() const { return _offset; } -void SpriteStudioPlayer2D::_update_root_transform() { +Transform2D SpriteStudioPlayer2D::_make_root_transform() const { Transform2D xform; xform.columns[0].x = _flip_h ? -1.0 : 1.0; xform.columns[1].y = _flip_v ? -1.0 : 1.0; xform.columns[2] = _offset; - _internal->setRootTransform(xform); + return xform; +} + +void SpriteStudioPlayer2D::_update_root_transform() { + _internal->setRootTransform(_make_root_transform()); } void SpriteStudioPlayer2D::_push_coverage_screen_scale() { diff --git a/ss_player/ss_player_node_2d.h b/ss_player/ss_player_node_2d.h index 565edc6..9d969f1 100644 --- a/ss_player/ss_player_node_2d.h +++ b/ss_player/ss_player_node_2d.h @@ -164,6 +164,10 @@ class SpriteStudioPlayer2D : public Node2D { Vector2 _offset; AnimationProcessMode _process_mode = ANIMATION_PROCESS_IDLE; + // flip_h / flip_v / offset as one matrix. Applied to the internal root canvas + // item (so it is NOT part of the Node2D transform) and composed onto part + // transforms by get_part_transform, which must report where a part is drawn. + Transform2D _make_root_transform() const; void _update_root_transform(); void _push_coverage_screen_scale(); diff --git a/ss_player/ss_resource_inspector.cpp b/ss_player/ss_resource_inspector.cpp index 3d9c279..a5a6f33 100644 --- a/ss_player/ss_resource_inspector.cpp +++ b/ss_player/ss_resource_inspector.cpp @@ -187,8 +187,16 @@ void SSResourceInspectorPlugin::_on_generate_animation_library_pressed(const Str int track_frame = anim->add_track(Animation::TYPE_VALUE); anim->track_set_path(track_frame, NodePath(".:frame")); anim->track_insert_key(track_frame, 0.0, 0.0f); - anim->track_insert_key(track_frame, length, (float)total_frames); - + // The player exposes frames 0..total_frame-1, and the last frame occupies + // the final 1/fps slice of `length`. Keying the last VALID frame at its own + // start time keeps the interpolated rate at exactly one frame per 1/fps and + // holds it until the clip ends; keying `total_frames` at `length` instead + // would push one frame past the animation and get clamped by the runtime. + const int max_frame = total_frames - 1; + if (max_frame > 0) { + anim->track_insert_key(track_frame, (double)max_frame / fps, (float)max_frame); + } + anim->value_track_set_update_mode(track_frame, Animation::UPDATE_CONTINUOUS); anim->track_set_interpolation_type(track_frame, Animation::INTERPOLATION_LINEAR); @@ -203,7 +211,12 @@ void SSResourceInspectorPlugin::_on_generate_animation_library_pressed(const Str #endif if (err == OK) { - EditorInterface::get_singleton()->get_resource_filesystem()->scan(); +#if defined(SPRITESTUDIO_GODOT_EXTENSION) || (VERSION_MAJOR >= 4 && VERSION_MINOR >= 6) + auto *efs = EditorInterface::get_singleton()->get_resource_filesystem(); +#else + auto *efs = EditorInterface::get_singleton()->get_resource_file_system(); +#endif + if (efs) efs->scan(); #ifdef SPRITESTUDIO_GODOT_EXTENSION UtilityFunctions::print("Generated AnimationLibrary: " + out_path); #else diff --git a/ss_player/ssab_resource.cpp b/ss_player/ssab_resource.cpp index f2218f0..ffda816 100644 --- a/ss_player/ssab_resource.cpp +++ b/ss_player/ssab_resource.cpp @@ -21,6 +21,18 @@ void SSABResource::_bind_methods() { ClassDB::bind_method(D_METHOD("get_sound_info", "sound_list_name_hash", "sound_name_hash"), &SSABResource::get_sound_info); } bool SSABResource::is_valid() const { + // Verification walks the whole buffer, and nearly every accessor below gates + // on it (selecting a player in the editor alone triggers several, and each + // Instance child does one per animation lookup), so a multi-megabyte SSAB + // would be re-verified many times per operation. The buffer is immutable + // between load_from_file / copy_from, both of which reset the cache. + if (_valid_cache < 0) { + _valid_cache = _verify_binary() ? 1 : 0; + } + return _valid_cache != 0; +} + +bool SSABResource::_verify_binary() const { if (binary.size() == 0) { return false; } @@ -58,6 +70,8 @@ bool SSABResource::is_valid() const { Error SSABResource::load_from_file(const String &path) { Error error = OK; _parent_dir = path.get_base_dir(); + // New buffer: drop the cached verification verdict before touching `binary`. + _valid_cache = -1; #ifdef SPRITESTUDIO_GODOT_EXTENSION binary = FileAccess::get_file_as_bytes(path); if (binary.size() == 0) { @@ -72,6 +86,7 @@ Error SSABResource::load_from_file(const String &path) { if (!is_valid()) { binary.clear(); + _valid_cache = -1; return ERR_INVALID_DATA; } @@ -332,6 +347,7 @@ Error SSABResource::copy_from(const Ref &p_resource) { const Ref &ssabFile = static_cast &>(p_resource); this->binary = ssabFile->binary; + _valid_cache = -1; return OK; } #endif diff --git a/ss_player/ssab_resource.h b/ss_player/ssab_resource.h index 15e397e..56f0899 100644 --- a/ss_player/ssab_resource.h +++ b/ss_player/ssab_resource.h @@ -80,6 +80,12 @@ class SSABResource : public Resource { #endif private: String _parent_dir; + // Cached result of the whole-buffer FlatBuffers verification run by + // is_valid(): -1 = not verified yet, 0 = invalid, 1 = valid. Reset to -1 + // wherever `binary` is replaced (load_from_file / copy_from). + mutable int8_t _valid_cache = -1; + // The actual verification; is_valid() is the cached front-end. + bool _verify_binary() const; // (sound_list_name_hash << 32 | sound_name_hash) -> loaded AudioStream. // Populated lazily by get_sound_stream so repeated events share one stream. HashMap> _sound_cache;