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

Optimization: use EventHandle in Components for scene and layers #7138

Merged
merged 6 commits into from
Dec 3, 2024

Conversation

Maksims
Copy link
Collaborator

@Maksims Maksims commented Nov 25, 2024

Depends on: #7137

This PR uses EventHandle.off instead of EventHandler.off to improve performance when disabling renderable components. As a scene and layers callaback arrays can get large in big projects.
In large scenes, disabling renderable components became ~4-8 times faster based on our tests. In one specific scene, disabling half of a scene used to take ~3550ms now it takes ~600ms (~5.9 times improvement).

I confirm I have read the contributing guidelines and signed the Contributor License Agreement.

@willeastcott
Copy link
Contributor

So this is for merging into main_v. I guess that's fine - we just cherry-pick afterwards to main. But we normally seem to go the other way....

Copy link
Contributor

@willeastcott willeastcott left a comment

Choose a reason for hiding this comment

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

LGTM. If @mvaligursky also approves, we can merge.

@Maksims
Copy link
Collaborator Author

Maksims commented Nov 29, 2024

So this is for merging into main_v. I guess that's fine - we just cherry-pick afterwards to main. But we normally seem to go the other way....

Editor uses engine v1, and most (except one) of our projects are v1, so we work with it in the first place.

@mvaligursky mvaligursky merged commit 461bcb4 into playcanvas:main_v1 Dec 3, 2024
8 checks passed
mvaligursky pushed a commit that referenced this pull request Dec 3, 2024
* implement more optimized EventHandle.off

* use EventHandle for scene and layers events for improved performance

* more EventHandle.off in critical places

* asset-reference event handlers

* remove uncecessary off's

---------

Co-authored-by: Will Eastcott <[email protected]>
@mvaligursky
Copy link
Contributor

cherry picked to main as well

@Maksims Maksims deleted the event-handle-components branch December 16, 2024 11:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Relating to load times or frame rate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants