Skip to content

Fix lights position when setting camera rotation to something different from 0 #7173

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

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

Conversation

FSDevelop
Copy link

This PR (delete as applicable)

  • Fix lights position when setting camera rotation to something different from 0

Describe the changes below:

Issue described here:
#7172

@FSDevelop FSDevelop changed the title fix lights position when setting camera rotation Fix lights position when setting camera rotation to something different from 0 Jun 22, 2025
@FSDevelop FSDevelop force-pushed the bugfix/lights_rotation_issue branch from 5a9ba3e to 8577e38 Compare June 22, 2025 08:43
@FSDevelop FSDevelop changed the base branch from master to v4.0.0 June 22, 2025 10:51
@FSDevelop FSDevelop changed the base branch from v4.0.0 to master June 22, 2025 10:52
Copy link

@cavolarl cavolarl left a comment

Choose a reason for hiding this comment

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

#7172

Would it be possible to do it within the transformPoint logic instead?

@FSDevelop
Copy link
Author

I'll try @cavolarl

@FSDevelop
Copy link
Author

FSDevelop commented Jul 3, 2025

#7172

Would it be possible to do it within the transformPoint logic instead?
@cavolarl
I think the problem is that transformPoint is an operation that only does basic math transformations and doesn't know about scroll factors or camera data. The if/esle block I propose is mathematically necessary because rotated cameras need scroll applied before rotation (in world coordinates) while non rotated cameras can apply scroll after transformation (in screen coordinates with zoom). I can do it but it would break other systems that rely on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants