Skip to content

fix: improve splat precision#666

Merged
keiya01 merged 7 commits into
mainfrom
fix/splat-precision
Jul 3, 2026
Merged

fix: improve splat precision#666
keiya01 merged 7 commits into
mainfrom
fix/splat-precision

Conversation

@keiya01

@keiya01 keiya01 commented Jul 3, 2026

Copy link
Copy Markdown
Member

Overview

Fixed the precision issue by applying RTC method.

What I've done

What I haven't done

How I tested

Which point I want you to review particularly

Screenshots

Before

Screen.Recording.2026-07-03.at.17.36.36.mov

After

Screen.Recording.2026-07-03.at.17.34.38.mov

Checklist

  • Wrote a clear PR description so reviewers can quickly understand the overview of the changes.
  • Confirmed that the proposed code does not include anything with an unknown or incompatible license.
  • Added tests for the changes.
  • Updated the API documentation.
  • Linked related issues.

@keiya01
keiya01 requested review from a team and Copilot July 3, 2026 09:07
@keiya01 keiya01 self-assigned this Jul 3, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses Gaussian splat jitter at globe (ECEF) scale by introducing an RTC-style floating-origin approach for SparkJS splat rendering, and updates examples/docs accordingly.

Changes:

  • Add a shared, camera-snapped floating-origin controller that recenters splat meshes and shifts the camera passed into Spark to keep float32 shader math numerically stable.
  • Expose a new splat.originCellSize configuration option (shared per transparent scene) and document its tradeoffs.
  • Update the splat example to use setCamera/flyTo framing and add a “far” sample for dynamic-origin testing.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
web/navara_three/example/pages/splat/run.ts Adjusts example camera framing and adds per-sample “Fly to” controls and a far-jitter test sample.
web/navara_three_default_descs/src/meshes/SplatMeshDesc/index.ts Integrates the shared dynamic-origin controller + Spark origin patch; adds originCellSize plumbing.
web/navara_three_default_descs/src/meshes/SplatMeshDesc/splatOriginController.ts Implements the grid-snapped origin and mesh recentering logic.
web/navara_three_default_descs/src/meshes/SplatMeshDesc/splatOriginController.test.ts Adds unit tests for snapping and recenter math/lifecycle.
web/navara_three_default_descs/src/meshes/SplatMeshDesc/sparkOriginPatch.ts Patches SparkRenderer to shift the camera by -origin and invoke the origin updater each frame.
docs/src/content/docs/three_default_descs/Mesh Desc/splat-mesh-desc.md Documents originCellSize and adds guidance about large scale instability.
docs/src/content/docs/ja/three_default_descs/Mesh Desc/splat-mesh-desc.md Japanese documentation for originCellSize + large scale note.
Comments suppressed due to low confidence (2)

web/navara_three_default_descs/src/meshes/SplatMeshDesc/index.ts:80

  • originCellSize is documented as “first splat wins” for the shared renderer, but when a renderer already exists this code silently ignores the requested cellSize. This can be very confusing when multiple splats are created with different originCellSize values.
    web/navara_three_default_descs/src/meshes/SplatMeshDesc/index.ts:249
  • The dynamic-origin controller stores an “original ECEF matrix” at onCreate(), but if the descriptor’s spatial config (matrixWorld/position/rotation/scale) is later updated, the controller will keep using the stale stored matrix when it recenters on cell changes. That can revert/ignore transform updates.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread web/navara_three/example/pages/splat/run.ts
Comment thread web/navara_three/example/pages/splat/run.ts Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 3, 2026 09:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

web/navara_three_default_descs/src/meshes/SplatMeshDesc/index.ts:80

  • When an existing shared SparkRenderer is reused, opts.cellSize (from splat.originCellSize) is ignored because the controller is already created. Since this is now part of the public config surface, consider warning when a different originCellSize is requested but cannot take effect (similar to the existing lod mismatch warning) so users can diagnose why their setting is ignored.

Comment thread web/navara_three/example/pages/splat/run.ts Outdated
Copilot AI review requested due to automatic review settings July 3, 2026 09:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (4)

web/navara_three_default_descs/src/meshes/SplatMeshDesc/index.ts:180

  • splat.originCellSize is passed straight into SplatOriginController as cellSize without validation. If a caller provides 0, a negative number, or a non-finite value, update() will produce NaNs/Infinities and recentering will break. Please validate and fall back to the default (and warn) before constructing the controller.
    web/navara_three_default_descs/src/meshes/SplatMeshDesc/index.ts:101
  • originCellSize is documented as “first splat wins” because the SparkRenderer is shared per transparent scene, but the code currently has no way to detect/report mismatches when later splats request a different cell size. Storing the chosen cell size in renderer.userData enables mismatch warnings in the existing-renderer path.
    web/navara_three_default_descs/src/meshes/SplatMeshDesc/index.ts:80
  • When a shared SparkRenderer already exists, a different originCellSize requested by later splats is silently ignored (since the controller is already constructed). This can be very confusing given the new originCellSize option; please warn similarly to the existing lod mismatch warning.
    web/navara_three_default_descs/src/meshes/SplatMeshDesc/index.ts:239
  • The controller stores a single “original ECEF matrix” per mesh at creation time, but MeshDesc supports runtime updates to matrixWorld / position / rotation / scale. After such an update, the next origin snap will recenter from the stale matrix and effectively undo the update. The controller’s stored matrix needs to be refreshed when spatial config changes occur.

Comment thread web/navara_three/example/pages/splat/run.ts
Copilot AI review requested due to automatic review settings July 3, 2026 11:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

web/navara_three_default_descs/src/meshes/SplatMeshDesc/index.ts:65

  • acquireSparkRenderer accepts cellSize, but when a shared renderer already exists the incoming cellSize is silently ignored (the first-created controller’s cell size effectively wins). Since originCellSize is a per-mesh config field, consider warning when a later splat requests a different cell size for the same transparent scene, similar to the existing lod mismatch warning, to avoid confusing no-op configuration.

Comment thread web/navara_three/example/pages/splat/run.ts
Comment thread web/navara_three/example/pages/splat/run.ts
Copilot AI review requested due to automatic review settings July 3, 2026 11:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (3)

web/navara_three_default_descs/src/meshes/SplatMeshDesc/index.ts:215

  • mesh.matrixWorld may not yet reflect the initial position/rotation/scale (e.g. when placing a splat via the documented position API). Cloning it here can register an identity/stale ECEF matrix, causing incorrect recentering once the RTC origin updates.
    web/navara_three_default_descs/src/meshes/SplatMeshDesc/index.ts:271
  • RTC recentering disables matrixAutoUpdate/matrixWorldAutoUpdate in SplatOriginController, so subsequent handle.update({ position/rotation/scale }) calls (the documented usage) will not update mesh.matrixWorld unless it is explicitly recomputed and re-registered. Additionally, the current guard (this.matrixWorld || this.matrix) prevents refreshing the cached ECEF matrix for position-only splats, so they can snap back to the original placement on the next origin change.
    web/navara_three_default_descs/src/meshes/SplatMeshDesc/index.ts:80
  • When a shared SparkRenderer already exists, opts.cellSize (from splat.originCellSize) is silently ignored even though the docs state the first splat's value wins. This can surprise callers who set different originCellSize values on later splats; consider warning on mismatch (similar to the existing LoD warning) so the behavior is explicit.

Comment thread web/navara_three/example/pages/splat/run.ts
@keiya01
keiya01 merged commit 0a338be into main Jul 3, 2026
4 checks passed
@keiya01
keiya01 deleted the fix/splat-precision branch July 3, 2026 13:29
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.

2 participants