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

Faster twin renderer #14441

Merged
merged 13 commits into from
Oct 20, 2023
Merged

Faster twin renderer #14441

merged 13 commits into from
Oct 20, 2023

Conversation

carolhmj
Copy link
Contributor

@carolhmj carolhmj commented Oct 18, 2023

This PR is a rearchitecture of the HTMLTwinRenderer, aiming to improve its speed.

Some PGs I tested with:
The ones in: https://doc.babylonjs.com/toolsAndResources/accessibility/screenReaders
Stress test: #C9GZTF#43
AddAllControls: #C9GZTF#49
Weapons demo with accessibility: #I6V1ST#240

@bjsplat
Copy link
Collaborator

bjsplat commented Oct 18, 2023

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Oct 18, 2023

@sebavan sebavan requested a review from RaananW October 18, 2023 20:48
Copy link
Member

@RaananW RaananW left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's discuss this offline :-)

Copy link
Contributor

@Popov72 Popov72 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly missing doc!

Generally speaking, there are a lot of checks with "instanceof", which is slow in javascript. If they're not in the hot path, then that's fine, otherwise I think you should try replacing them with something else (when possible), because as I understand it, this PR is intended to improve overall performance?

@sebavan sebavan merged commit f9de1a6 into BabylonJS:master Oct 20, 2023
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants