Skip to content

Track engine-dev API changes and fix edit-history race conditions#874

Merged
slimbuck merged 5 commits intoplaycanvas:mainfrom
slimbuck:engine-dev
Apr 24, 2026
Merged

Track engine-dev API changes and fix edit-history race conditions#874
slimbuck merged 5 commits intoplaycanvas:mainfrom
slimbuck:engine-dev

Conversation

@slimbuck
Copy link
Copy Markdown
Member

Summary

Adopts upcoming PlayCanvas engine API changes and fixes a race in edit history that could be triggered by rapid undo/redo during/after splat transforms.

Engine API updates

  • camera.ts: rename camera.renderPassescamera.framePasses to match the engine's renamed property.
  • shaders/splat-shader.ts: pass view-space depth (-center.view.z) as the second argument to prepareOutputFromGamma, matching the engine's updated tonemapping signature.

Edit history race fix

  • edit-history.ts: serialize edit.add / edit.undo / edit.redo through an internal promise chain. Previously, rapid Ctrl+Z / Ctrl+Shift+Z could race with an in-flight updatePositions GPU readback and corrupt the sorter's centers buffer in centers-overlay mode.
  • splats-transform-handler.ts: fire edit.add for the transform op before awaiting updatePositions(). Because events.fire synchronously enqueues onto the serialized history chain, any undo pressed while the readback is still pending is now guaranteed to revert the transform op (rather than the prior selection op). The post-drag visual restoration (selectionAlpha, outline, underlay) is moved after the await since the GPU state is already applied during update().

Camera pose ordering

  • editor.ts: in camera.setPose, assign fov before calling scene.camera.setPose(...) so the camera distance is computed using the new fovFactor.

Test plan

  • Verify rendering still works against the engine-dev branch (no console errors, splat tonemapping looks correct).
  • Stress-test undo/redo: perform several splat transforms, then mash Ctrl+Z / Ctrl+Shift+Z while drags are still resolving — sorter / centers buffer should remain stable.
  • Confirm undoing immediately after a transform reverts the transform (not the previous selection).
  • Trigger camera.setPose with a fov change and confirm the resulting framing uses the new FOV.

@slimbuck slimbuck requested a review from Copilot April 24, 2026 10:49
@slimbuck slimbuck self-assigned this Apr 24, 2026
@slimbuck slimbuck added the bug Something isn't working label Apr 24, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the editor to stay compatible with upcoming PlayCanvas engine API changes while addressing an undo/redo race that can occur around asynchronous splat position readbacks.

Changes:

  • Update camera render-pass wiring to use the engine’s renamed framePasses API.
  • Update splat shader tonemapping call to match the engine’s updated prepareOutputFromGamma signature (adds view-space depth).
  • Introduce serialized processing for history edits (add/undo/redo) and adjust splat transform history recording order to reduce undo/redo races.

Reviewed changes

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

Show a summary per file
File Description
src/splats-transform-handler.ts Reorders transform history recording vs. async updatePositions() and moves visual restoration after the await.
src/shaders/splat-shader.ts Passes view-space depth into tonemapping helper to match engine API.
src/editor.ts Applies fov before setPose so camera distance calculations use the updated FOV.
src/edit-history.ts Adds a promise-chain queue to serialize edit history mutations triggered via events.
src/camera.ts Renames renderPasses usage to framePasses per engine rename.

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

Comment thread src/splats-transform-handler.ts
Comment thread src/edit-history.ts Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 6 out of 6 changed files in this pull request and generated 3 comments.


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

Comment thread src/edit-history.ts
Comment thread src/edit-history.ts Outdated
Comment thread src/splats-transform-handler.ts Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 6 out of 6 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

src/edit-history.ts:95

  • _redo() increments cursor before awaiting editOp.do(). If do() fails, cursor will point past the operation that wasn’t actually applied, which can break subsequent undo/redo (and is especially problematic now that the queue logs errors and keeps executing later ops). Consider incrementing cursor only after a successful do() (and similarly for the suppressed-op path), or rolling back cursor on failure.
    private async _redo(suppressOp = false) {
        const editOp = this.history[this.cursor++];
        if (!suppressOp) {
            await editOp.do();
        }
        this.events.fire('edit.apply', editOp);
        this.fireEvents();
    }

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

Comment thread src/edit-history.ts Outdated
Comment thread src/edit-history.ts
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 6 out of 6 changed files in this pull request and generated no new comments.


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

@slimbuck slimbuck marked this pull request as ready for review April 24, 2026 17:55
@slimbuck slimbuck requested a review from a team April 24, 2026 17:55
@slimbuck slimbuck merged commit d87a9b5 into playcanvas:main Apr 24, 2026
6 checks passed
@slimbuck slimbuck deleted the engine-dev branch April 24, 2026 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants