Tresjs / tres

Declarative ThreeJS using Vue Components
https://tresjs.org
MIT License
1.93k stars 86 forks source link

Reconsider how useRenderLoop works. #633

Closed alvarosabu closed 1 month ago

alvarosabu commented 3 months ago

Description

useRenderLoop is used to provide the end user a callback method onLoop every frame per second to align with the one that the core uses for rendering.

It's based on useRafn composable from @vueuse/core https://vueuse.org/core/useRafFn/ but due to the feedback of the community, there are few issues related to its current behavior:

Suggested solution

Refactor useRenderLoop to move away from useRafn

Alternative

No response

Additional context

No response

Validations

dghez commented 3 months ago

as stated here https://github.com/Tresjs/tres/issues/624#issuecomment-2042327291 imho have the option to take control of the render-loop entirely and the option to specify the priority are a "must have", as you can do in useFrame in r3f.

If you do

 useFrame(() => {}, 0)

you won't see anything because you are taking control over the render loop, very useful when you want to render manually into render-targets or have custom pipeline or render with layers :)

andretchen0 commented 3 months ago

Also Related

Moving forward

I wonder if we'd be best off simply leaving the current useRenderLoop implementation alone, for the most part.

useRenderLoop's good enough for a lot of circumstances – e.g., fire a function every ~1/n seconds. Even if we pull the renderer.render(...) out of that loop, it won't impact those usecases.

We could then more or less freely implement a new function that's tied to a particular TresCanvas' renderer, e.g., useFrame – to borrow R3F's name. Maybe that function also pauses/resumes with useRenderLoop to maintain most of the current functionality.

... Or maybe we just deprecate useRenderLoop and move on.

alvarosabu commented 3 months ago

Hi, @andretchen0 I was doing the same mental exercise yesterday. Let's analyze the pros and cons of the several options

I managed to create a render loop per instance of TresCanvas. The problem is, opposite to the original useRenderLoop , the useLoop needs to be inside of a subcomponent of TresCanvas because it uses de ContextProvider... It's the only way to make it unique.

That would be a huge breaking change but, it's actually how R3F useFrame works. I just tested on an R3F app and you cannot use any of the hooks on the upper level as we do with useRendererLoop

image

Option 1 - Both composables coexist

useRenderLoop's good enough for a lot of circumstances – e.g., fire a function every ~1/n seconds. Even if we pull the renderer.render(...) out of that loop, it won't impact those use-cases.

With this approach, we avoid having a breaking change that would make the v4 not retro-compatible with v3

Pros

Cons

Option 2 - Deprecate useRenderLoop in favor of useLoop

Pros

Cons

With that being said, after a night of sleep, even if the cons list is high, I'm personally inclined to Option 2 even if requires a little extra from the core team. The value of the benefits overcomes the cons.

I'm pretty sure users will complain not being able to use the loop on the parent component, but we could even figure out some kind of context bridge like you propose on your PoC @andretchen0 https://github.com/orgs/Tresjs/discussions/578#discussioncomment-8697380

Sorry for the long text, wydt?

andretchen0 commented 2 months ago

wydt?

Ideally, I'd like to take some things off the list of cons.


I'd prefer to keep the current API intact, while tying it to a <TresCanvas /> and expanding it to include priority.

API-wise, useRenderLoop takes no arguments currently, so we could expand it like this:

Behind the scenes, we'd reimplement the old API – onBeforeLoop, etc. – using the new priority-based mechanism. At first glance, this seems like a pretty straight-forward refactor – though maybe making the actual changes will turn up a problem I don't currently see.


I'm pretty sure users will complain not being able to use the loop on the parent component

Yeah, this is the real stumbling block for me, too.

It's just really helpful to useRenderLoop() in the <script setup> of a <TresCanvas />. It makes demos and reproductions shorter and more user-friendly – just one file!

but we could even figure out some kind of context bridge like you propose on your PoC

It looks to me like rather than throwing here ...

https://github.com/Tresjs/tres/blob/main/src/composables/useTresContextProvider/index.ts#L174

... we could create a new unbound context and bind it when the next TresCanvas is created.

That'll work for common use cases, I think. We can offer an explicit API for more control for users who want it.


I've been doing some smaller nice-to-haves that I'd been putting off. Once I'm done with that, I'll move back to the core to try to move this forward.

In the meantime, I'd prefer not to hold back v4.

alvarosabu commented 2 months ago

Hi @andretchen0 thanks for your insightful feedback. In the following days I will push the PoC I got working taking in consideration both your feedback and @JaimeTorrealba.

Now that this pandora box was opened. Should we bind all composables to each instance or just the loop?

I personally only see benefits on the render loop one, but the rest can be global inmho. It's true that R3F has a warning on the docs that Hooks can only be use inside of the Canvas context Screenshot 2024-04-13 at 15 12 08

andretchen0 commented 2 months ago

@alvarosabu

In the following days I will push the PoC I got working taking in consideration both your feedback and @JaimeTorrealba.

Ok. Just for reference, I think this discussion has most of the relevant issues related to useRenderLoop and binding context. Maybe that'll be helpful.

I personally only see benefits on the render loop one, but the rest can be global imho.

Off hand, I'm not sure.

I do know that if we're going to have plugins, we need extend/catalogue (or similar) to be bound to a <TresCanvas />.

Tinoooo commented 2 months ago

@alvarosabu @JaimeTorrealba and I had a chat about the while topic. We propose the following structure for useLoop to allow users to register callbacks around a loop's lifecycle.

Types/Interfaces:

interface Options {
  delta: number
  elapsed: number
  clock: Clock
  context: TresContext
}

type EventHookOn = (
  fn: (opts: Options) => void,
  priority?: number
) => {
  off: () => void
}

interface UseLoop {
  onBeforeRender: EventHookOn
  render: (opts: Options) => void
  onAfterRender: EventHookOn
  ...
}

Pros:

alvarosabu commented 2 months ago

To complement what Tino explained in the previous comment:

With this approach, we avoid using magic numbers to denote the different stages of the loop, indexes are now just for priority of execution for before and after render callbacks. Diagram onLoop