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

Fix Y-sorted root item having modulation applied twice #95669

Merged
merged 1 commit into from
Jan 29, 2025

Conversation

kleonc
Copy link
Member

@kleonc kleonc commented Aug 16, 2024

Fixes #95667.

For the y-sorted subtree's root modulation is obtained once again after gathering y-sorted items, just like e.g. transform. In the similar manner it needs to be cancelled out.
(again fixing my own mistake, from #78134 🙃)

@kleonc kleonc added bug topic:rendering topic:2d cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release labels Aug 16, 2024
@kleonc kleonc added this to the 4.4 milestone Aug 16, 2024
@kleonc kleonc marked this pull request as ready for review August 16, 2024 23:32
@kleonc kleonc requested a review from a team as a code owner August 16, 2024 23:32
Copy link
Member

@clayjohn clayjohn left a comment

Choose a reason for hiding this comment

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

Should be fine to merge after a rebase

@akien-mga
Copy link
Member

@kleonc Would you be able to rebase this soon?

@kleonc kleonc force-pushed the y_sort_fix_root_double_modulate branch from e6a8e9b to 24b502b Compare January 28, 2025 13:22
@kleonc
Copy link
Member Author

kleonc commented Jan 28, 2025

Rebased, and fixed the change as the previous / modulate would cancel out the whole modulation (including the modulation part propagated from the parent non-y-sorted items). Instead, only the single item's (y-sorted subtree root's) modulation is cancelled out now with / ci->modulate.

But before merging: @clayjohn is it fine as is or should we handle potential division by zero? 🤔

@Repiteo Repiteo merged commit ee4acfb into godotengine:master Jan 29, 2025
19 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Jan 29, 2025

Thanks!

@kleonc kleonc deleted the y_sort_fix_root_double_modulate branch January 30, 2025 12:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release topic:rendering topic:2d
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Y-sorted subtree's root gets modulation applied twice
4 participants