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

Improve aura rendering for tokens with multiple auras #17288

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

In3luki
Copy link
Collaborator

@In3luki In3luki commented Nov 11, 2024

As a bonus rerendering the same aura is now always cached even for single aura tokens.

Aura highlights are now always drawn from largest to smallest to ensure larger auras not painting over a smaller aura's squares. That could be reversed from small to large with skipping alredy highlighted squares but I'm not sure that the minor performance gain would warrant that.

I think I've got all reset cases covered expect when the scene changes the polygons of tokens in the old scene are not reset. They are reset when the scene changes back however, so it might not be necessary to do that.

@stwlam
Copy link
Collaborator

stwlam commented Nov 11, 2024

This is a lot of manual cache management for the relatively rare case of multiple auras; I'm not sure if a performance gain is worth the added maintenance. Is there no way to integrate it into canvas rendering workflows?

@In3luki In3luki force-pushed the multiple-aura-test branch 3 times, most recently from 28f901d to 3349922 Compare November 12, 2024 11:42
@In3luki
Copy link
Collaborator Author

In3luki commented Nov 12, 2024

  • Removed the polygon caching and reverted all changes tied to that
  • The grouped polygons are now only created for tokens with more than one aura
  • Moved the override polygons to the optional AuraRenderer#polygons property
  • Removed the additional polygons argument from the getAreaSquares function

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.

2 participants