feat(sdk): object.attach(child) keeps the child where it stands; the editor uses it (#758) - #784
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #758
What changed
SceneNode.attach(child)(packages/sdk-core/src/scene/core/node.ts, logic innodeAttach.tsasnodeCopy.tsdoes forcopy): re-parents a node while keeping its world matrix. Both world matrices are resolved through the node's ownupdateWorldMatrix; the new local matrix is, in the reference's order, the inverse of the parent's world matrix times the former parent's world matrix times the child's local matrix (invertMatrix4,multiplyMatrix4), split into position, rotation and scale by the existingdecomposeMatrix4, thenadd. No second inverse or decompose. When a subclass'sadddeclines the child, its pose is left as it was;Object3D.attachdeclines itself before any work.Object3D.attachreads the new pose from the tree back intoposition,quaternionandscalewithout firing their listeners (angles follow the quaternion on the next read), then tells the world once;lookAtreads its pose back through the samereadPose. The field-to-tree binding moved intoobjectPose.ts(bindPose), next toreadPose.nodeError.tsrefuseSceneRootreplaces three copies of the scene-root check innode.ts.reparentCommand(site/app/editor/commands.ts) callsparent.attach(object). Its own inverse and decompose are gone; undo still puts back the saved local pose.attachCommand(a plainadd) is renamedaddCommandso it does not read as the world-preservingattach.docs/SDK.mddocumentsattach, and everyapi.<language>.jsontranslates it forSceneNodeandObject3D.tests/integration/sdk-facade.test.tsunchanged). The attach scratch arrays are marked/* @__PURE__ */so a bundle that never attaches drops them.docs/SDK.mdandnodeAttach.ts).Proof
node.test.ts: a child under a parent that is moved, rotated and scaled non-uniformly, attached to another such parent, keeps its world matrix to within 1e-12 and still recomposes it after an update. Attached back, its local matrix returns. A cycle is refused.object3d.test.ts: same check throughObject3D.position,quaternionandscalerecompose the kept world matrix, the angles follow, and attaching a node to itself is declined with nothing changed.object3d.test.ts: 16 attaches of unrounded poses, under automatic and manual update, give to the bit the position, quaternion, scale (and, under manual update, the matrix) of the reference's order, inverse(new parent world) x old parent world x local. The test fails on cd05303 (inverse(new parent world) x child world: last-bit differences) and passes after 9bcf2b7.scripts/docs-scene-editor.test.ts: the editor's reparent keeps the world position, and undo puts back the old parent and the exact saved pose. The existing editor tests still pass.git submodule update --init,pnpm run build:native,pnpm run compile:cachesandpnpm run build:pnpm run check:changedandpnpm run test:changedboth pass with no failures.pnpm run generate:apiandpnpm run check:i18npass.tscon the core, site, tools and root configs passes, and eslint on the changed files is clean.node scripts/check-pr-size.ts: 239 hand-written lines.origin/develop(with Streaming without holes: texture levels survive a device loss in the world cache and yield to the pages (#745) #777):pnpm run validate --group quickpasses.Local review before push
simplifyskill, run twice (coder, then reviewer; 4 agents each: reuse, simplification, efficiency, altitude). The coder's run fixed five things:Object3D.attachno longer decomposes a second time (it reads the tree's slots quietly, with one world notice); a declined attach is guarded; the attach scratch is marked@__PURE__, so the maths-only bundle is back at 5_082; the reformatting used to fit 200 lines is reverted, and the room comes fromnodeAttach.ts,objectPose.tsand one root check; one shared closeness helper. The reviewer's run fixed three:lookAtreads back throughreadPose(quiet, one notice) instead of a listener round trip;Object3D.attachdeclines only itself (child === this) before any work;assertClosechecks lengths with a plain loop. Kept on purpose: the inverse computed before a declined add, the copy of the local matrix (16 numbers, the pose under manual update), the immediate subtree update.code-review --fixskill, run twice. The coder's run renamedattachCommandtoaddCommandand named the refused root action. The reviewer's run found 7 findings and fixed the main one:attachbuilt the local matrix as inverse(new parent world) x child world, not in the reference's order (inverse(new parent world) x old parent world x child local). A throwaway comparison with the witness found 200 of 200 random attaches off in the last bits before the fix and 0 after; a bit-exact test inobject3d.test.tsnow pins the order. Left out: the adopted-storage matrix ofTransformNode(outside this issue); an editor undo restoring only position, quaternion and scale for a manual-update object (editor objects auto-update); a pose notice after a subclass'sadddeclines a child (values unchanged); translations ofSceneNode.attachadding "its local pose rewritten" (true, wider than the English); the local matrix copy under auto update (negligible). Not covered by tests: the manual-update path and a subclass'sadddeclining at theSceneNodelevel. CI fix (native job,engine-structure.test.ts):nodeAttach.fixture.tsimportednode:assert/strict, which sdk-core's non-test files may not. As in the other sdk-core fixtures, it now imports nothing and returns the first mismatch (mismatch), and the tests assert on it; the structure test is unchanged.Closes #758; the diff follows the lead's design note (one decompose, existing matrix helpers); the three tests fail ondevelop(noattach); labelssdk,🟡 normal,in review; docs/SDK.md, the API reference and 14 translations follow; no image, format, streaming or example change; maths-only bundle 5_082.Not proven / left out
attachon a node whosematrix.elementspoint at the caller's own storage (the adopted-matrix path ofTransformNode) is not handled: the storage is neither read before nor written after.attachis a CPU scene-graph call; WebGPU and WebGL2 see an ordinary pose change.Lead verification
object.attach(child)re-parents a node while keeping its world matrix: delivered inpackages/sdk-core/src/scene/core/node.ts:105(attach, in the file's one-line doc style) andpackages/sdk-core/src/scene/core/nodeAttach.ts:20(attachSceneNode: the new parent's inverse × the old parent's world × the child's local, one decompose, a declined attach leaving the pose untouched), withObject3D.attachreading the pose back quietly (world/object/objectPose.ts,lookAtsharing that readback); proved byattach moves a child under another parent where it stands in the world,attach keeps the world matrix, and position, rotation and scale hold the new poseandattach gives the reference's pose to the bit: new parent's inverse × old parent × local(16 attaches, automatic and manual update, fails on the first order).docs/SDK.md(attach besideaddandreparent, the shear limit stated) and the 14site/content/reference/api.<lang>.json; proved bycheck:i18nand the reference tests.site/app/editor/commands.ts:62-73(redo isparent.attach(object), its own invert, multiply and decompose removed; undo keeps its exact saved pose); proved by the editor's reparent, undo and redo test inscripts/docs-scene-editor.test.ts.attach) and the bit-exact one on the earlier order.attachis an ordinary pose change for both backends.invertMatrix4,multiplyMatrix4,decomposeMatrix4(once) and the quietQuaternion.setreused;refuseSceneRootgathers three copies of the root check; searched by the /simplify reuse agents, no twin.docs/SDK.md, the API reference and its 14 translations.in reviewset;to measureafter the merge (a public engine member). The maths-only bundle keeps its 5_082 bytes (@__PURE__scratch). Streaming without holes: rules and objectives for geometry, memory and shadows #483 checklist and CONTRIBUTING.md §Streaming, memory and shadows: a call, no per-frame work.