Render issue with continuous resize animation

JakobHock commented 3 weeks ago

Describe the bug

Due to the debounced ref introduced in #512, the canvas only renders once the resize animation has finished. Render issue How it's supposed to work Fixed rendering

It can be fixed by moving renderer size update and the camera projection matrix update to before rendering or using useFps composable from vueuse to dynamically calculate debounceTime

Reproduction

Monitor with refresh rate above 100 hz needed

https://stackblitz.com/edit/stackblitz-starters-jtmew9?file=src%2FApp.vue

Steps to reproduce

Have container of TresCanvas resize continuously.

System Info

System:
    OS: Linux 5.15 Ubuntu 22.04.4 LTS 22.04.4 LTS (Jammy Jellyfish)
    CPU: (12) x64 AMD Ryzen 5 5600X 6-Core Processor
    Memory: 12.28 GB / 15.58 GB
    Container: Yes
    Shell: 5.8.1 - /usr/bin/zsh
  Binaries:
    Node: 22.8.0 - ~/.asdf/installs/nodejs/22.8.0/bin/node
    npm: 10.8.2 - ~/.asdf/plugins/nodejs/shims/npm
    pnpm: 9.9.0 - ~/.local/share/pnpm/pnpm
  npmPackages:
    @tresjs/cientos: ^4.0.2 => 4.0.2 
    @tresjs/nuxt: ^3.0.7 => 3.0.7

Used Package Manager

pnpm

Code of Conduct

[X] I agree to follow this project's Code of Conduct
[X] Read the Contributing Guidelines.
[X] Read the docs.
[X] Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
[X] The provided reproduction is a minimal reproducible example of the bug.

andretchen0 commented 1 week ago

Closing. Can't reproduce in v4.

Hi @JakobHock ,

Your example StackBlitz uses Tres v2:

  "dependencies": {
    "@tresjs/cientos": "2.1.2",
    "@tresjs/core": "2.1.2",
    "@vueuse/core": "^11.1.0",
    "three": "^0.152.2",
    "vue": "^3.3.4"
  },

We're now on v4. There's no issue in v4 as far as I can see:

https://stackblitz.com/edit/stackblitz-starters-tmzph4?file=src%2Fcomponents%2FTheExperience.vue

JakobHock commented 1 week ago

Hello @andretchen0,

sorry i think i went off of some stackblitz template and didn't look at the version. My bad.

Issue with your example still remains. The canvas still does not resize correctly with the container continuously. Canvas stays small(or big) until the container resize animation is finished.

Ideally it would have the correct size at every render. I have forked your example to show the difference. https://stackblitz.com/edit/stackblitz-starters-jtmew9?file=src%2FApp.vue

andretchen0 commented 1 week ago

Issue with your example still remains. The canvas still does not resize correctly with the container continuously. Canvas stays small(or big) until the container resize animation is finished.

I'm not sure I follow. At least, what I understand from above doesn't seem to line up with what I'm seeing.

Latest Chrome on Mac:

https://github.com/user-attachments/assets/d732fe15-04f6-4261-82de-c882c5cf4788

There's an issue with the StackBlitz above – toggleView is used in a handler but not defined anywhere. So maybe you didn't submit the fork that exhibits the issue?

JakobHock commented 1 week ago

Ahh, i'm really sorry about that. Fixed it up. https://stackblitz.com/edit/stackblitz-starters-jtmew9?file=src%2FApp.vue

As you can see when the view is "tres" the resize animation is jumping only when the container has finished. In the three version it is continuously. It "lags" behind the actual size as it wait's on the "render" but this is fine if the rendering is not done manually.

andretchen0 commented 1 week ago

Hi @JakobHock

No problem. Mistakes happen.

Here's what the latest Stackblitz looks like for me:

https://github.com/user-attachments/assets/0e44da5b-5094-448c-b36b-4efe842ac69a

I'm not trying to be obtuse, but I don't notice the reported bug from the top-line issue here. Is that still what's under discussion?

As you can see when the view is "tres" the resize animation is jumping only when the container has finished.

If the bug is visible in the video above, can you download the video and step through it? Then post 2 sequences of still frames of:

the bug in Tres
the correct behavior in Three

JakobHock commented 1 week ago

Hello @andretchen0, thank you for being understanding. The bug isn't happening in the video you posted. Should have just posted a video of how it looks on my end.

This is what it looks like on my browser. Tested with Firefox 131.0.3 and Chrome 130.0.6723.59

https://github.com/user-attachments/assets/6a71a2c9-f7fc-4905-941f-f0870c3bf18f

I thought it would look the same for everyone. Any idea what could cause this discrepancy?

Edit: This happens due to my Monitor. As my monitor has 144hz, the browser render call happens more frequently than on 60 hz. Due to that the debouncedRef never gets triggered on 144Hz as 10ms have not yet passed. 60hz Monitor has 16.667 ms between render calls, 144hz has 6.944444ms between render calls. With a debounced call of 10ms, the size invalidation never gets called before the animation ends as the size changes before the 10ms are over.

I would guess the debounced size invalidation is not really an option, as there are 540 hz monitors.

144hz:

https://github.com/user-attachments/assets/9e7951ac-7499-42c8-9f60-15836ad120bb

60hz:

https://github.com/user-attachments/assets/9a47d16f-fd6a-4068-bf7a-1cde55a9276c

andretchen0 commented 1 week ago

Reopening. Thanks for the clarification.

andretchen0 commented 1 week ago

@alvarosabu

I'm not sure what a good solution is here.

I wasn't part of the original debounce discussion for the most part, but I seem to remember it was introduced due to poor performance on resize.

JakobHock commented 1 week ago

One idea would be to use the useFps composable from vueuse. With this the debounce duration could be dynamically calculated to be under the refresh rate of the browser and thus updating the size before the next render.

I think ideally the invalidation should happen internally before the next render is called, but as the structure of the composables are a bit separated, this achieves the same thing with a bit more performance overhead

Seems to work fine has does not seem to have an impact on performance. Is there a setup to test performance?

Happy to open a PR if this is a good solution from your POV.

andretchen0 commented 1 week ago

Is there a setup to test performance?

I don't believe that one was made. We've only recently started adding dev playgrounds for issues.

If we could make one for this issue, that'd be great.

R3F

It looks like R3F accepts a config object for specifying how resizing should be handled.

https://github.com/pmndrs/react-three-fiber/blob/d54c4bf3635beb4dc68afa08ac8bfdeba3b4dcf4/docs/API/canvas.mdx#L36

I think it would be great to offer something similar. That lets users fix these sorts of issues on their own, based on their use case, if they need to.

`window-size`

Maybe we remove window-size as a flag? R3F doesn't offer one, and that makes sense by this logic:

the size is always the size of its parent container, it's responsive by default. if the relative or absolute parent is 100%/100% it fills the screen, if the parent is fixed, it's fixed.

https://github.com/pmndrs/react-three-fiber/discussions/630

debounce -> throttle?

If we still need the debounce for performance on resize, maybe it's better as a throttle. At least that would fire sometimes.

@JakobHock Just thinking out loud. Don't feel like you have to take all of this on.

@alvarosabu Any thoughts here?

Tresjs / tres