Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: 423 enable on demand render mode usage #436

Open
wants to merge 38 commits into
base: v4
Choose a base branch
from

Conversation

alvarosabu
Copy link
Member

@alvarosabu alvarosabu commented Jun 6, 2024

Closes #430

Controls

  • OrbitControls
  • CameraControls
  • MapControls
  • TransformControls
  • KeyboardControls
  • PointerLockControls
  • ScrollControls

Abstractions

Staging

Materials

  • WobbleMaterial
  • GlassMaterial (Doesnt need it)
  • HolographicMaterial
  • ReflectionMaterial
  • CustomShaderMaterial

Loaders

  • SVG (Doesnt need it)
  • useGLTF (Doesnt need it)
  • useVideoTexture (Doesnt need it)

Shapes

  • CatmullRomCurve3 (Doesnt need it)
  • Line2 (Doesnt need it)
  • Superformula (Doesnt need it)
  • RoundedBox (Doesnt need it)

Misc

  • HTML (Doesnt need it)
  • Stats/StagsGI (Doesnt need it)
  • BakeShadows (Doesnt need it)
  • GLTFExporter

Copy link

stackblitz bot commented Jun 6, 2024

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@alvarosabu alvarosabu marked this pull request as draft June 6, 2024 15:34
@alvarosabu alvarosabu self-assigned this Jun 6, 2024
@alvarosabu
Copy link
Member Author

alvarosabu commented Jun 15, 2024

Hey @andretchen0 something weird happened with some demos when I tried to test and implement on-demand invalidation. For some reason render-mode="on-demand" doesn't seem to do anything on this demo. But I can't find anything on the respective components that could be automatically invalidating the scene 😵‍💫

render-mode="manual" does work as expected.

Demos:

  • Lensflare
  • AnimatedSprites
  • Fit

@andretchen0
Copy link
Contributor

andretchen0 commented Jun 18, 2024

Hey @alvarosabu ,

Looking at the code, there are 2 cases:

Invalidated by the demo

  • In LensflareDemo, on-demand is invalidated by setting props in the demo: <TresPointLight :position="[x, 0, z]">
  • This is also the case in FitDemo: <TresGroup :rotation="[rx0, ry0, rz0]" :position="[x0, y0, z0]" :scale="[sx0, sy0, sz0]">

Invalidated by the component

  • For AnimatedSprite, the component itself will typically set several props when the shown frame changes, causing invalidation in on-render, e.g.: <TresSprite :scale="[scaleX, scaleY, 1]" :position="[positionX, positionY, 0]" >

How can I help?

  • What would a solution look like here?
  • Would you like me to code it up and push here?

@alvarosabu
Copy link
Member Author

Hey @alvarosabu ,

Looking at the code, there are 2 cases:

Invalidated by the demo

  • In LensflareDemo, on-demand is invalidated by setting props in the demo: <TresPointLight :position="[x, 0, z]">
  • This is also the case in FitDemo: <TresGroup :rotation="[rx0, ry0, rz0]" :position="[x0, y0, z0]" :scale="[sx0, sy0, sz0]">

Invalidated by the component

  • For AnimatedSprite, the component itself will typically set several props when the shown frame changes, causing invalidation in on-render, e.g.: <TresSprite :scale="[scaleX, scaleY, 1]" :position="[positionX, positionY, 0]" >

How can I help?

  • What would a solution look like here?
  • Would you like me to code it up and push here?

Ohhhh ok I wanted to confirm if it was invalidated due to the prop changes, you are right. That would mean that potentially we don't need to add anything extra for the AnimatedSprite since the invalidation happens internally.

I will try to remove the elements on the demo that invalidate (without committing changes) so I can test the internal invalidation for the other two 👍

@alvarosabu alvarosabu marked this pull request as ready for review June 26, 2024 08:22
@alvarosabu alvarosabu added feature New feature or request dx performance p3-significant High-priority enhancement (priority) labels Jun 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dx feature New feature or request p3-significant High-priority enhancement (priority) performance
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

None yet

2 participants