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 stretch transform when resizing SubViewports #67331

Merged

Conversation

Sauermann
Copy link
Contributor

@Sauermann Sauermann commented Oct 13, 2022

resolve #67329
resolve #68871

In SubViewport::set_size the following function is called:

_set_size(p_size, _get_size_2d_override(), Rect2i(), _stretch_transform(), true);

The problem is, that currently _stretch_transform() is based on the previous size of the SubViewport instead of the new p_size.

This PR changes the code, so that stretch transform is now calculated inside of Viewport::_set_size, where the new size and size_2d_override are available.

Updated 2022-11-19: Fix merge conflict
Updated 2023-01-27: Fix merge conflict
Updated 2023-02-01: Fix merge conflict with #66906

@Sauermann Sauermann requested a review from a team as a code owner October 13, 2022 00:24
@Sauermann Sauermann force-pushed the fix-subviewport-no-stretch-update branch 3 times, most recently from 6e068a1 to 3f318b0 Compare October 13, 2022 02:18
@akien-mga akien-mga added this to the 4.0 milestone Oct 13, 2022
@Sauermann Sauermann force-pushed the fix-subviewport-no-stretch-update branch from 3f318b0 to 5d04ea1 Compare October 14, 2022 09:36
@Sauermann Sauermann force-pushed the fix-subviewport-no-stretch-update branch from 5d04ea1 to 8274862 Compare November 19, 2022 12:23
@Sauermann Sauermann force-pushed the fix-subviewport-no-stretch-update branch 2 times, most recently from fdbdfd3 to 7c477f4 Compare February 1, 2023 01:52
Move calculation of stretch transform from outside to inside
of `Viewport::_set_size` function.
@Sauermann Sauermann force-pushed the fix-subviewport-no-stretch-update branch from 7c477f4 to 8182f29 Compare February 1, 2023 07:45
Copy link
Member

@RandomShaper RandomShaper left a comment

Choose a reason for hiding this comment

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

Looks very correct to me.

Copy link
Member

@bruvzg bruvzg left a comment

Choose a reason for hiding this comment

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

Seems reasonable.

@akien-mga akien-mga merged commit 2852d5e into godotengine:master Feb 1, 2023
@akien-mga
Copy link
Member

Thanks!

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