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

Use a ResizeObserver to detect changes in canvas size #1121

Merged

Conversation

tmarti
Copy link
Contributor

@tmarti tmarti commented Aug 14, 2023

Description

Today, in each scene tick, the canvas and window sizes are compared to their previous values to detect if the pixel-size of the canvas needs to be updated. By doing that, calls happen to .clientWidth and .clientHeight properties, and this can trigger DOM layout calculations per-tick, which adds overhead to the rendering process (according to Chrome Profiler).

Instead, this PR pretty much simplifies the mechanism and delegates the knowledge about if the canvas resized to a ResizeObserver, which avoids to kick any extra DOM layout calculation. With this the resize-handling code gets simpler and more performant.

Bonus track

Also, when the Perspective matrix is updated, the class will make sure to mark its Camera as "dirty" so it updates the matrix. This is needed in some cases so the camera gets aware that the x/y axis-ratio has changed, as the aspect ratio is handled on the projection matrix.

Reference

ResizeObserver is supported on all major browsers since around 3 years ago:

@tmarti tmarti changed the title Use a ResizeObserver to detect changes canvas size Use a ResizeObserver to detect changes in canvas size Aug 14, 2023
@ghost
Copy link

ghost commented Aug 14, 2023

👇 Click on the image for a new way to code review

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

@xeolabs xeolabs merged commit d54611d into xeokit:master Aug 14, 2023
2 checks passed
@xeolabs xeolabs added the enhancement New feature or request label Aug 14, 2023
@xeolabs xeolabs added this to the 2.4.0 milestone Aug 14, 2023
xeolabs added a commit that referenced this pull request Aug 31, 2023
@xeolabs
Copy link
Member

xeolabs commented Sep 12, 2023

@tmarti I wonder if this BIMViewer issue is somehow related: xeokit/xeokit-bim-viewer#155

@tmarti
Copy link
Contributor Author

tmarti commented Sep 13, 2023

I think so; for the perspective case this commit in this PR should solve it:

I think the same should be done for the ortho case?

@tmarti tmarti deleted the resize-observer-for-canvas-size-update branch November 16, 2023 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants