feat: 503 conditional rendering of primitives #514

alvarosabu · 2024-01-18T06:02:46Z

Closes #503

Object prop is not reactive
Conditional rendering is not working. v-if

andretchen0

Glad to have this update!

I only found a small bit of code that looks like it can be simplified. I won't hold up the PR.

andretchen0 · 2024-01-18T18:49:38Z

src/core/nodeOps.ts

@@ -39,7 +39,8 @@ export const nodeOps: RendererOptions<TresObject, TresObject> = {
      if (props?.object === undefined) logError('Tres primitives need a prop \'object\'')
      const object = props.object as TresObject


props.object, object and instance all point to the same JS object here, so it looks like this can be simplified:

[....] if (tag === 'primitive') { if (props.object === undefined) logError('Tres primitives need a prop \'object\'') instance = props.object as TresObject instance.userData.tres__primitive = true } else { [...]

Hey @andretchen0, thanks for checking it, I ended up modifying this part because we need to clone the prop.object to avoid overwriting the original three object when swapping happens.

andretchen0 · 2024-01-18T19:46:16Z

@alvarosabu

Do you need a hand with the failing tests and linter?

…erties

alvarosabu · 2024-01-19T10:03:10Z

It's pretty much done @andretchen0 if you take another look before merging 🙏🏻

andretchen0

I don't know if it can be worked around, but cloning the Three objects means that references in setup are lost. Details in the comments.

andretchen0 · 2024-01-20T15:09:36Z

src/core/nodeOps.ts

@@ -39,7 +39,8 @@ export const nodeOps: RendererOptions<TresObject, TresObject> = {
      if (props?.object === undefined) logError('Tres primitives need a prop \'object\'')
      const object = props.object as TresObject
      name = object.type
-      instance = Object.assign(object, { type: name, attach: props.attach, primitive: true })
+      instance = Object.assign(object.clone(), { type: name }) as TresObject


I'm not sure if there's a workaround, but using a clone here means that references in a component's setup won't work.

I've added a comment to playground/src/pages/primitives.vue with some code that demonstrates the problem.

@andretchen0 Thats a good point thanks for raising it, the problem with the reference comes when we try to make the :object reactive to swap the instance, which completely changes the ref.

Not sure how to deal with that.

andretchen0 · 2024-01-20T15:39:16Z

playground/src/pages/primitives.vue

+    color: '#82DBC5',
+  }),
+)
+


In nodeOps.ts, I mentioned that cloning means that setup references are lost. Below is some demonstration code that can be pasted here.

useRenderLoop().onLoop(() => { torus.rotateX(0.01) torusKnot.rotateY(0.01) })

Both torus and torusKnot continue to exist after the clone, but they no longer point to the on-screen objects, so the on-screen objects don't update.

If the objects aren't clone()d in nodeOps, the on-screen shapes will rotate – but other errors pop up.

I don't know if there's a workaround here. Just pointing it out.

@andretchen0 This argument makes me think we should abandon the idea of trying to make :object prop reactive, swiping instances without losing the ref is getting too complicated.

Users could still use primitives with conditional rendering, I wanted primitive to work similar to component but honestly at this point I don't know how to achieve it.

@alvarosabu

I don't understand the internals of Tres well enough to have an opinion on the feasibility here.

I'll start reading the source after I finish <AnimatedSprite /> for Cientos.

I wanted primitive to work similar to component but honestly at this point I don't know how to achieve it.

I agree that this is a worthwhile goal. It'd be great if it "just works".

@andretchen0 let me know if you are up to a pair-programming call to discuss the internals when you get free let me know, we definitely can use your knowledge and your quality feedback inside of the custom renderer code. 💚

@alvarosabu

Sure thing. That could be fun!

Hey @andretchen0 I created a thread on the core team discord channel discussing this but to give a summary:

I found a way of making it work (animations), the only constraint is that the object passed through :object needs to be of the same type (mesh -> mesh) (group -> group). If we do (mesh -> group), it doesn't work anymore.

Screen_Recording_2024-02-02_at_12.43.44.mov

Thanks! I'll go read the Discord.

src/core/nodeOps.ts

andretchen0

Again, I'm not sure if this can be worked around, but there's still a problem with references in <script>.

andretchen0 · 2024-02-08T02:44:46Z

src/core/nodeOps.ts

    if (node) {
      let root = node
      let key = prop
+      if (node.__tres.primitive && key === 'object' && prevValue !== null) {
+        // If the prop 'object' is changed, we need to re-instance the object and swap the old one with the new one
+        const newInstance = nodeOps.createElement('primitive', undefined, undefined, { 


It seems like this doesn't break references in <script> per se. The :objects still exist. But they're copied. That means something like the following (probably) wouldn't have the user's desired effect in the playground example:

useRenderLoop().onLoop(({elapsed}) => { torus.position.y = Math.sin(elapsed) })

Whenever the primitive object becomes torus, torus is copied, along with its attributes. But the copy doesn't move like torus moves, because it's a different Torus.

@andretchen0 we will need to explicitly mention on the docs that the user needs to animate the ref (primitiveRef) but not the object (torus) following the same DX as they were using a <TorusMesh ref="torusRef" />

Screen.Recording.2024-02-08.at.10.14.03.mov

There are several reasons why we need to clone the object:

Since we soft-swipe the instances when :object changes (we maintain the reference but copy all the attributes) that would modify the original object from the setup leading to unwanted side effects.
It is not possible to pass a ref or reactive as @JaimeTorrealba mentioned in the thread. With clone, we can ensure that swiping between models is possible.

This is what happens without the clone

Screen.Recording.2024-02-08.at.10.24.31.mov

It's either has this limitation of having to use the ref coming from the custom renderer to animate or :object can't be reactive and users will need to use v-if and v-else to swipe models:

<primitive v-if="showA" :object="modelA" /> <primitive v-else :object="modelB" />

I balance through the first option.

@alvarosabu

I think the PR as it currently stands is an improvement. I don't want to stand in the way of improvements.

I approved the PR.

This sort of error is an itch I can't stop scratching, so if you want to keep scratching ...

Since we soft-swipe the instances when :object changes (we maintain the reference but copy all the attributes) that would modify the original object from the setup leading to unwanted side effects.

Can you elaborate on unwanted side effects?

Technical issues aside, there's a point that isn't clear for me conceptually. To illustrate:

<script> ... const userMesh = new Mesh(...) userMesh.position.x = 100 const userMesh2 = new Mesh(...) userMesh2.position.x = 200 </script> <template> ... <primitive ref="primitiveRef" :object="userMesh" :position-x="300" /> ... </template>

Should userMesh's x be 300? (300 is the value in the primitive)

And later, if the user does primitiveRef.value.object = userMesh2, does that set userMesh2's x to 300?

For R3F, the answer to both is "yes", it seems. Here's a StackBlitz. Notice that the rotation of the TorusRing increments with every frame, but resets whenever it's made the primitive.object. And the Torus would normally be placed at x = 2000, but its x is set to the primitive's value.

That seems like the right solution to me.

tres/src/core/nodeOps.ts

Line 197 in 8408351

const newInstance = nodeOps.createElement('primitive', undefined, undefined, {

I understand that this will make a clone, which is required at the moment, but is there a reason why it's necessary to go through nodeOps.createElement? Is it updating something on the Vue side, maybe?

Can you elaborate on unwanted side effects?

One side-effect is that is not possible to pass a ref or reactive as @JaimeTorrealba mentioned in the thread on Discord. With clone, we can ensure that swiping between models is possible.

Should userMesh's x be 300? (300 is the value in the primitive)

Yes, the prop has priority because it will overwrite the mesh positions on the setup.

And later, if the user does primitiveRef.value.object = userMesh2, does that set userMesh2's x to 300?

Users shouldn't overwrite vue template refs, wouldn't it be kind of an antipattern? I also see the users complaining about having to do this extra step of primitiveRef.value.object = userMesh2 vs :object being reactive.

const newInstance = nodeOps.createElement('primitive', undefined, undefined, {

That was to not having to duplicate the same code of the createElement inside of the pathProp since primitives have some special treatment, like localstate flags and such.

* feat: 474 vue chrome devtools plugin (#479) * feat: vue chrome devtools * feat: editable scenes from devtools * chore(lint): fix lint errors * feat: highlight material * chore(lint): fix * chore: release v4.0.0-next.0 * feat: update to three `v160` and vue `v3.4` (#488) * fix(types): added `Object3DEventMap` to `Object3D` generics for point event handling (#491) * feat: 140 on demand rendering (#497) * feat: conditional rendering * chore: remove subscribe system * feat: on-demand automatic invalidation with prop changes * feat: invalidate once first when is `renderMode !== 'always'` * docs: performance page, on-demand rendering * chore: fix windowsize issue * chore(lint): fix maximum line length issues * feat: invalidate on-demand on window resize * feat: add advance method for manual mode * feat: fix manual first render with advance * docs: performance manual mode * docs: add badge with version * chore: correct typos and PR suggestions * chore: tell dont ask fix * feat: render state instead of internal * feat: remove default camera warning (#499) * feat: remove annoying defautl camera warning * chore: remove logWarning * feat: 492 set tone mapping default to acesfilmictonemapping (#498) * feat: set ACESFilmicToneMapping as default toneMapping * chore: usage of nullish coealescing operator instead of ternaries * feat: 516 localstate for custom renderer node instances instead of userdata (#522) * feat: conditional rendering * chore: remove subscribe system * feat: on-demand automatic invalidation with prop changes * feat: invalidate once first when is `renderMode !== 'always'` * docs: performance page, on-demand rendering * chore: fix windowsize issue * chore(lint): fix maximum line length issues * feat: invalidate on-demand on window resize * feat: add advance method for manual mode * feat: fix manual first render with advance * docs: performance manual mode * docs: add badge with version * chore: correct typos and PR suggestions * chore: tell dont ask fix * feat: render state instead of internal * feat: add __tres local state to nodeOps instances * feat: add context to root on instances localstate * feat: camera registration ops from node local state ctx * feat: event handling registration from localState of nodes * feature: disposable flag on node localstate * feat: remove userData from types * chore: remove unused import * fix(test): fake localstate `.__tres` on tests * fix(types): fix nodeOps instances localstate type * fix: camera aspect * Update orthographic camera aspect when screen size updates * Give user a "manual" flag to keep Tres from updating camera * feat: 503 conditional rendering of primitives (#514) * feat(nodeOps): switch instance logic for reactive `object` prop * chore: playground primitives with models * chore: fix linter * chore: fix tests and linters, primitive object is now reactive * chore: refactor instance swaping logic to overwrite set and copy properties * chore: tests * chore: remove console.log * chore: remove unused import watch * feat: add primitive conditional to patch object prop * fix: `nodeOps` is now a function (#579) * fix: `nodeOps` is now a function * chore(test): updated tests for `nodeOps` * chore: next package json version * chore: release v4.0.0-next.1 * fix: refactor nodeOps to return methods at the end of the function (#602) * fix: refactor nodeOps to return methods at the end of the function * chore: fix lint * chore: internal playground organisation (#601) * chore: new internal playground org and testing pages * chore: fix lint * chore: better styling of playground landing page * chore: lint * chore: deps update * chore: internal primitive model test playground * chore: fix lint * chore: release v4.0.0-next.2 * chore: misc routes * fix: do not change pierced props case (#608) * chore: lint fix * chore: problem with package version * chore: fix lint * chore: rebuild pnpm-lock * test(nodeOps): clean up tests * test(nodeOps): organize tests * test: add coverage plugin * test: add coverage to package.json script * test(nodeOps): improve test coverage * feat: devtools renderer improvements (#614) * feat: renderer programs when selecting scene on devtools * feat: renderer.info * chore: fix lint * docs: devtools update * chore: fix lint issues * feat(events)!: pointerevents manager and state (#529) * new file: playground/src/components/Box.vue new file: playground/src/pages/raycaster/Propogation.vue * Started work on interactive Event Propogation playground example modified: src/components/TresCanvas.vue * Import and use `useEventStore` * defineEmits for all expected pointer events so we may emit propogated events off of the canvasa modified: src/composables/index.ts new file: src/composables/useEventStore/index.ts * Started work on an event store. I'm not sure this counts as a store just yet * Wired up majority of pointer events * Added event propogation * Does not require using userData scene props or nodeOps for registering objects to scene modified: src/composables/useRaycaster/index.ts * Added new event listeners to power newly supported pointer events. We now check whole scene/children when calling intersectObjects. * Created new EventHooks for new events * Added `forceUpdate` function that allows for pointer-move events to work without mouth movement (good for when camera is moving but mouse is not) modified: src/core/nodeOps.ts * Added supported events to array so they don't get received as props * (temporarily) unhook current pointer event solution to iterate on useEventStore modified: src/utils/index.ts * Added Camel-to-kebab case util * Support multiple event listeners, add support for .stop event modifier * Set stopProgation variable to false by default, whoops * fix typo * fix: remove `createGlobalState` from `useEventStore`, allowing events to work while multiple TresCanvas' are being used * fix(perf): remove extraneous intersectObjects/getIntersects calls by moving intersects into a ref that is updated on pointer-move * chore(lint): fix lint issues * feat: enhance events manager to include duplicates checking, pointer-missed support, and forced updating Per file changelog: modified: playground/src/components/Box.vue * Added a pointer-missed handler for testing modified: playground/src/pages/TheBasic.vue * uses forceUpdate from EventManager to fire events even when the mouse hasn't moved modified: playground/src/pages/raycaster/Propagation.vue * Didn't mean to undo the lint changes, adds a pointer-missed event on the canvas for extra testing modified: src/components/TresCanvas.vue * Adds `pointer-missed` as possible event for canvas emits modified: src/composables/index.ts * Update export deleted: src/composables/useEventStore/index.ts * Rename `useEventStore` to `useTresEventManager` modified: src/composables/useRaycaster/index.ts * Check for empty intersects on hit test, wire up pointerMissed events eventHook * Fix forceUpdate to call onPointerMove instead of triggering an EventHook modified: src/composables/useTresContextProvider/index.ts * Add TresEventManager type new file: src/composables/useTresEventManager/index.ts * add onPointerMissed * create (de)registerPointerMissedObj methods so we can track objects in the scene listening to this event * Note: These are passed to nodeOps via TresContext * Implement duplicates checking for eventPropogation modified: src/core/nodeOps.ts * register/deregister pointerMissed objects * chore: lint * docs: new event docs * chore: fix lint * feat: enhance event object details and use in Box example to change material color. Add ability to force event system updates even when mouse hasn't moved. Enhance pointer-enter/leave events. Update types Box.vue * Added pointer-missed handler * set the materials flash color using the object coming off of the event instead of a ref UseRaycaster * Flesh out event details to include * all mouse event properties * intersections * tres camera * camera raycaster * source event * mouse position delta * stopPropagating stub * and unprojectedPoint (this needs work, cant get the math to work) UseTresContextProvider * Add TresEventManager type to TresContext useTresEventManager * Add forceUpdate method to allow apps to force an event system update even when the mouse hasnt moved * Add pointerMissed event * Properly implement pointer-enter/pointer-leave events * Before now, pointer-enter | leave were only called on first object in intersection, now we execute the events for all entered/left objects * Use stopPropagating property included on event object * chore: lint * chore: fix lint issues --------- Co-authored-by: alvarosabu <[email protected]> * feat: 499 better memory management (#606) * chore: memory management playground * feat: recursively free cpu and gpu memory allocation on remove * chore: clumsy attempt to dispose on unmount * chore: lint fix * feat: remove scene root on disposal * chore: fix lint * docs: added disposal guide on `performance` docs * chore: fix lint * chore: type issues (#663) * fix: fix some internal types * chore: fix linters * fix: typescript issues on event manager * chore: release v4.0.0-rc.0 * fix: make on* callbacks settable (#672) * fix: make on- callbacks settable * test: test setting not calling * feat: 633 use loop (#673) * feat: createRenderLoop unique to context * feat: onLoop returns current state * feat: ensuring callback excecution with index order * feat: take control of render loop logic * docs: updated composable docs * feat: change error to deprecation warning towards v5 * chore: add link to new composable docs on deprecation warning * chore: remove depcreation warning of existing useRenderLoop * feat: `useFrame` and `useRender` instead of `onLoop` * chore: fix lint * feat: applied useFrame to directives * chore: fix lint * feat: `useUpdate` instead of `useFrame` and useRender pausing. * chore: testing fbo * feat: reserve index 1 for late-updates * chore: fix lint * feat: useLoop composable for the win * chore: change onLoop name for register * chore: unit tests for loop * chore: change order for registration to make index optional * chore: fix lint * feat: pauseRender and resumeRender * docs: useLoop guide * docs: updated basic animations recipe to `useLoop` * docs: correct pause render methods on docs * Update docs/api/composables.md Co-authored-by: Tino Koch <[email protected]> * Update docs/api/composables.md Co-authored-by: Tino Koch <[email protected]> * Update docs/api/composables.md Co-authored-by: Tino Koch <[email protected]> * Update docs/api/composables.md Co-authored-by: Tino Koch <[email protected]> * Update docs/api/composables.md Co-authored-by: Tino Koch <[email protected]> * Update docs/api/composables.md Co-authored-by: Tino Koch <[email protected]> * Update docs/api/composables.md Co-authored-by: Tino Koch <[email protected]> * chore: refactor subscribers to `priorityEventHooks` * Update docs/api/composables.md Co-authored-by: Tino Koch <[email protected]> * feat: just return `off` on the loop registration methods * docs: update docs to add `off` unregister callback method * feat: remove `v-rotate` * docs: added context warning for `v-always-look-at` * Update docs/api/composables.md Co-authored-by: Tino Koch <[email protected]> * Update docs/api/composables.md Co-authored-by: Tino Koch <[email protected]> * chore: remove leftover of isntance.provide * chore: remove subscribers from context * chore: abstract `wrapCallback` and move render loop register to `useRender` * chore: fix lint * chore: testing off * Revert "chore: abstract `wrapCallback` and move render loop register to `useRender`" This reverts commit 24cec65. * chore: return bound `off` method and use createPriorityEvent for render with defaultFn fallback * feat: deprecate and remove `vAlwaysLookAt` and `vRotate` BREAKING_CHANGE: Directives `vAlwaysLookAt` and `vRotate` due incompatibility with new `useLoop` and the refactor of the render loop logic. * feat: set context to loop to avoid wrapping the callbacks * feat: dispose render hook before taking over --------- Co-authored-by: Tino Koch <[email protected]> * chore(playground): adding missing import and removing the directives that were deprecated * chore(playground): use new composable on animations * fix(utils): reorder object disposal to avoid issue with Helper `dispose` methods (#683) * chore: updated deps * chore: release v4.0.0-rc.1 * fix: manual rendering blank (#685) * fix: increate time to advance on manual mode * chore: correct playground * fix: 686 useloop callback state missing controls (#687) * fix(loop): take plain snapshots of ctx * fix: types for useloop * chore: lint * docs: add RectAreaLightHelper to vLightHelper docs * chore(deps): update deps 24-0-2024 * chore: release v4.0.0-rc.2 * fix: start loop if user calls useRenderLoop (#695) * docs: change motivation * chore(deps): last update before release --------- Co-authored-by: Peter <[email protected]> Co-authored-by: Garrett Walker <[email protected]> Co-authored-by: Tino Koch <[email protected]> Co-authored-by: Jaime Torrealba <[email protected]> Co-authored-by: Jaime A Torrealba C <[email protected]>

alvarosabu added 2 commits January 18, 2024 06:49

feat(nodeOps): switch instance logic for reactive object prop

cf979fa

chore: playground primitives with models

5a24eea

alvarosabu added enhancement dx v4 labels Jan 18, 2024

alvarosabu requested review from Tinoooo and andretchen0 January 18, 2024 06:02

chore: fix linter

5ed02b6

alvarosabu marked this pull request as draft January 18, 2024 06:16

andretchen0 approved these changes Jan 18, 2024

View reviewed changes

alvarosabu added 2 commits January 19, 2024 10:12

chore: fix tests and linters, primitive object is now reactive

8d52f32

chore: refactor instance swaping logic to overwrite set and copy prop…

2df1e65

…erties

alvarosabu marked this pull request as ready for review January 19, 2024 10:02

andretchen0 requested changes Jan 20, 2024

View reviewed changes

alvarosabu mentioned this pull request Jan 31, 2024

feat: 516 localstate for custom renderer node instances instead of userdata #522

Merged

alvarosabu added 4 commits February 2, 2024 16:34

Merge branch 'v4' into feature/503-conditional-rendering-of-primitives

3490c4a

chore: tests

37ab904

chore: remove console.log

b0ba190

chore: remove unused import watch

de730ef

alvarosabu requested a review from andretchen0 February 5, 2024 14:16

alvarosabu mentioned this pull request Feb 5, 2024

fix: 449 memory management #466

Closed

andretchen0 reviewed Feb 5, 2024

View reviewed changes

src/core/nodeOps.ts Outdated Show resolved Hide resolved

feat: add primitive conditional to patch object prop

8408351

alvarosabu requested a review from andretchen0 February 7, 2024 11:49

andretchen0 reviewed Feb 8, 2024

View reviewed changes

andretchen0 approved these changes Feb 8, 2024

View reviewed changes

alvarosabu merged commit 79d8a76 into v4 Feb 21, 2024
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: 503 conditional rendering of primitives #514

feat: 503 conditional rendering of primitives #514

alvarosabu commented Jan 18, 2024 •

edited

Loading

andretchen0 left a comment •

edited

Loading

andretchen0 Jan 18, 2024

alvarosabu Jan 19, 2024

andretchen0 commented Jan 18, 2024

alvarosabu commented Jan 19, 2024

andretchen0 left a comment

andretchen0 Jan 20, 2024 •

edited

Loading

alvarosabu Jan 21, 2024

andretchen0 Jan 20, 2024 •

edited

Loading

alvarosabu Jan 23, 2024

andretchen0 Jan 23, 2024

alvarosabu Jan 25, 2024

andretchen0 Jan 25, 2024

alvarosabu Feb 5, 2024

andretchen0 Feb 5, 2024

andretchen0 left a comment

andretchen0 Feb 8, 2024

alvarosabu Feb 8, 2024

andretchen0 Feb 8, 2024 •

edited

Loading

andretchen0 Feb 8, 2024

andretchen0 Feb 8, 2024

alvarosabu Feb 21, 2024

		@@ -39,7 +39,8 @@ export const nodeOps: RendererOptions<TresObject, TresObject> = {
		if (props?.object === undefined) logError('Tres primitives need a prop \'object\'')
		const object = props.object as TresObject

feat: 503 conditional rendering of primitives #514

feat: 503 conditional rendering of primitives #514

Conversation

alvarosabu commented Jan 18, 2024 • edited Loading

andretchen0 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andretchen0 commented Jan 18, 2024

alvarosabu commented Jan 19, 2024

andretchen0 left a comment

Choose a reason for hiding this comment

andretchen0 Jan 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andretchen0 Jan 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andretchen0 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andretchen0 Feb 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alvarosabu commented Jan 18, 2024 •

edited

Loading

andretchen0 left a comment •

edited

Loading

andretchen0 Jan 20, 2024 •

edited

Loading

andretchen0 Jan 20, 2024 •

edited

Loading

andretchen0 Feb 8, 2024 •

edited

Loading