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

Render issue with continuous resize animation #848

Open
5 tasks done
JakobHock opened this issue Oct 3, 2024 · 10 comments
Open
5 tasks done

Render issue with continuous resize animation #848

JakobHock opened this issue Oct 3, 2024 · 10 comments
Labels
bug Something isn't working p3-minor-bug An edge case that only affects very specific usage (priority)

Comments

@JakobHock
Copy link

JakobHock commented Oct 3, 2024

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

@alvarosabu alvarosabu added the pending-triage Ticket is pending to be prioritised label Oct 12, 2024
@andretchen0 andretchen0 reopened this Oct 17, 2024
@andretchen0
Copy link
Contributor

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

@andretchen0 andretchen0 closed this as not planned Won't fix, can't repro, duplicate, stale Oct 17, 2024
@JakobHock
Copy link
Author

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
Copy link
Contributor

andretchen0 commented Oct 17, 2024

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:

Screen.Recording.2024-10-17.at.18.05.42.mov

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
Copy link
Author

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
Copy link
Contributor

Hi @JakobHock

No problem. Mistakes happen.

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

Screen.Recording.2024-10-18.at.14.42.32.mov

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
Copy link
Author

JakobHock commented Oct 18, 2024

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

2024-10-18.15-03-29.mp4

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:

144hz.mp4

60hz:

60hz.mp4

@andretchen0
Copy link
Contributor

Reopening. Thanks for the clarification.

@andretchen0 andretchen0 reopened this Oct 18, 2024
@andretchen0
Copy link
Contributor

@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.

@andretchen0 andretchen0 added bug Something isn't working p3-minor-bug An edge case that only affects very specific usage (priority) and removed pending-triage Ticket is pending to be prioritised labels Oct 18, 2024
@JakobHock
Copy link
Author

JakobHock commented Oct 18, 2024

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
Copy link
Contributor

andretchen0 commented Oct 18, 2024

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.

pmndrs/react-three-fiber#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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working p3-minor-bug An edge case that only affects very specific usage (priority)
Projects
Status: Done
Development

No branches or pull requests

3 participants