Skip to content

fix(vat-perf): align PerfSpawnerVAT with VATInstance bind-sidecar logic - #647

Merged
fernandotonon merged 1 commit into
masterfrom
fix/vat-perf-spawner
May 20, 2026
Merged

fix(vat-perf): align PerfSpawnerVAT with VATInstance bind-sidecar logic#647
fernandotonon merged 1 commit into
masterfrom
fix/vat-perf-spawner

Conversation

@fernandotonon

Copy link
Copy Markdown
Owner

Summary

PR #646 fixed VATInstance.gd (the single-instance VAT player) to:

  • Read the per-vertex Ogre bind sidecar (<basename>_ogre_bind.bin)
  • Bucket by quantized position, tiebreak ambiguous matches by continuous (normal, UV) distance
  • Drop the historical `NORMAL = -normalize(n)` negation

`PerfSpawnerVAT.gd` is the MultiMesh variant of the same script — used by the website's "1000× VAT" perf tab — and was missed in that commit. It still ran the naive `running_offset + j` identity UV2 plus the inverted normal, so the perf scene rendered as scattered triangles + back-lit shading once the rest of the alignment fixes landed on master.

This PR inlines the bind-sidecar loader (`_load_ogre_bind_sidecar`), the position-bucket builder (`_pos_key`), and the continuous-distance tiebreaker (`_best_candidate`) into PerfSpawnerVAT — same logic as VATInstance, kept inline rather than shared via `class_name` because the spawner is its own self-contained MultiMesh path with no reason to import a single-instance helper class.

Also drops the `NORMAL = -normalize(n)` negation in the inline shader, mirroring VATInstance.

`PerfSpawnerSkeleton.gd` doesn't touch VAT (pure skeletal animation via Godot's stock SkinnedMeshRenderer), so no change there.

Test plan

  • `Godot --headless --script perf_test.gd`: 1000 instances spawn in ~29 ms, no `unmatched` warning (all 5828/5828 verts resolved)
  • Godot web re-export bundled
  • Visual smoke on the deployed "1000× VAT" tab after merge

Related

Follow-up to PR #646.

🤖 Generated with Claude Code

PR #646 fixed VATInstance.gd to read the per-vertex Ogre bind
sidecar (`<basename>_ogre_bind.bin`) and remap each Godot vertex
back to its baker-side column index, so the bake survives
Godot's import-side vertex reorder. PerfSpawnerVAT.gd is the
MultiMesh variant of the same script and was missed in that
commit — it still used the naive `running_offset + j` identity
UV2 + the historical `NORMAL = -normalize(n)` negation, so the
1000-instance perf scene rendered with scattered triangles AND
inverted lighting.

This commit:

  - Inlines the bind sidecar loader, the position-bucket lookup,
    and the continuous-distance tiebreaker from VATInstance.gd
    (kept inline rather than shared via class_name to avoid a
    cross-dep between the single-instance player and the spawner).
  - Drops the `NORMAL = -normalize(n)` negation in the inline
    shader, mirroring the VATInstance shader fix.

Verified by spawning the 1000-instance scene headless: all
5828/5828 verts match the sidecar with no fallback. PerfSpawnerSkeleton
doesn't touch VAT (pure skeletal animation through Godot's stock
SkinnedMeshRenderer), so no change needed there.

Re-runs the Godot web export so the website's perf-VAT tab picks
up the fix on next deploy.
@coderabbitai

coderabbitai Bot commented May 20, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@fernandotonon has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 49 minutes and 38 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a4ede5c2-281b-4fde-b8d2-a00a72b2748d

📥 Commits

Reviewing files that changed from the base of the PR and between 10b8e09 and 09f9aaf.

📒 Files selected for processing (3)
  • tools/godot-vat-demo/scripts/PerfSpawnerVAT.gd
  • website/public/demo/index.html
  • website/public/demo/index.pck
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/vat-perf-spawner

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sonarqubecloud

Copy link
Copy Markdown

@fernandotonon
fernandotonon merged commit 1f97c0c into master May 20, 2026
20 checks passed
@fernandotonon
fernandotonon deleted the fix/vat-perf-spawner branch May 20, 2026 19:31
fernandotonon added a commit that referenced this pull request May 20, 2026
Bumps the project version to 3.3.0 in CMakeLists.txt (single source
of truth — sync-doc-versions-from-cmake.sh propagates to README and
the website's pinned action ref). The 3.x.y → 3.3.0 minor bump
reflects the VAT pipeline (PRs #640 / #644 / #646 / #647 landing
together):

  - `qtmesh vat <file> --anim <name> -o <dir>` CLI subcommand
  - OpenVAT-format 16-bit position+normal bake
  - Vertex-order-aligned source.gltf + Ogre bind sidecar so engine
    importers can realign UV2 to the bake's column order
  - Drop-in shader templates for Godot/Unity/Unreal at
    tools/vat-shaders/
  - Live website demo at /#vat-demo (Showcase + 1000× VAT vs
    1000× skeletal perf comparison)

Website updates:

  - Deep-linking to any section (e.g. `/#vat-demo`, `/#install`,
    `/#cli`) now works on both initial load (React-mount-aware
    scroll-into-view via requestAnimationFrame) and during
    in-session hashchanges (smooth scroll).
  - Each Section title gains a hover-visible `#` anchor link so
    visitors can grab a shareable URL without dev-tools.
  - New "VAT" tab on the home page's CLI examples panel with a
    full `qtmesh vat` command demo.
  - New `cmd-vat` reference section in the docs (synopsis,
    options, examples, sidebar entry).
  - `scroll-behavior: smooth` + `scroll-margin-top: 1.5rem` on
    `section[id]` so anchor-scroll lands with breathing room
    rather than flush against the viewport top edge.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant