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 SubViewport not rendering correctly #62854

Closed

Conversation

Rindbee
Copy link
Contributor

@Rindbee Rindbee commented Jul 9, 2022

Fix #55471.

Previously, the necessary steps in _set_size were not performed when the SubViewport was initialized, which caused rendering issues. There was no problem when changing the parameters (make size, size_2d_override, or size_2d_override_stretch not the default value) which caused _set_size to be called.

@Rindbee Rindbee requested a review from a team as a code owner July 9, 2022 12:49
@YeldhamDev YeldhamDev added this to the 4.0 milestone Jul 9, 2022
clayjohn
clayjohn previously approved these changes Jul 9, 2022
@Rindbee Rindbee requested a review from a team as a code owner July 9, 2022 22:55
@Rindbee
Copy link
Contributor Author

Rindbee commented Jul 9, 2022

When the SubViewport enters the tree, some properties should be set according to its parent.

The parent SubViewportContainer should also be updated when the SubViewport changes as follows:
1. Size changed (or update);
2. Enter/exit the tree.

@Rindbee Rindbee requested a review from clayjohn July 9, 2022 22:57
@clayjohn clayjohn dismissed their stale review July 10, 2022 07:13

Changes since last review

@Rindbee
Copy link
Contributor Author

Rindbee commented Aug 8, 2022

godot/scene/main/viewport.h

Lines 727 to 736 in 836fe9a

enum UpdateMode {
UPDATE_DISABLED,
UPDATE_ONCE, //then goes to disabled
UPDATE_WHEN_VISIBLE, // default
UPDATE_WHEN_PARENT_VISIBLE,
UPDATE_ALWAYS
};
private:
UpdateMode update_mode = UPDATE_WHEN_VISIBLE;

Currently the default update_mode is UPDATE_WHEN_VISIBLE, which doesn't make sense because SubViewport has no concept of visibility. We might need to choose a meaningful update_mode.

Of course, update_mode (including clear_mode) will not take effect, because it is not used in RenderingServer when initialized.

Given the close relationship between SubViewport and SubViewportContainer, the interaction between them is considered comprehensively.

@Rindbee Rindbee force-pushed the fix-SubViewport-wrong-rendering branch from bac9797 to aeb0f7c Compare August 8, 2022 03:58
UPDATE_WHEN_PARENT_VISIBLE,
UPDATE_ALWAYS
};

private:
UpdateMode update_mode = UPDATE_WHEN_VISIBLE;
UpdateMode update_mode = UPDATE_ONCE;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

UPDATE_ALWAYS might be better, but I'm not sure if it's necessary.

…t changes as follows:

1. Size changed;
2. Enter/exit the tree.

When the SubViewport enters the tree, some properties should also be set according to its parent.
@Rindbee Rindbee force-pushed the fix-SubViewport-wrong-rendering branch from aeb0f7c to f0f16c2 Compare August 8, 2022 04:40
@Rindbee Rindbee requested a review from a team as a code owner August 8, 2022 04:40
Rindbee added a commit to Rindbee/godot that referenced this pull request Aug 9, 2022
`SubViewport` has some initialization settings of its own, but these settings are not synchronized to the `RenderingServer` when initialized, which causes some problems.

Split from godotengine#62854.

This is a prerequisite to resolve godotengine#55471, still need to modify `update_mode` manually, or add as a child to `SubViewportContainer` (`update_mode` is automatically set to `UPDATE_ALWAYS`).

Currently, `UPDATE_WHEN_VISIBLE` has no meaning for `SubViewport`. Presumably for **visible**, `RenderingServer` has different rules, viewport needs to have a visible **rect** to be considered **visible**, while `SubViewport` has only `size` and **no** `position`, so it is not considered **visible** even if it is set to be an active viewport (`RS::viewport_set_active`).
Rindbee added a commit to Rindbee/godot that referenced this pull request Aug 10, 2022
Follow-up to godotengine#64138, split from godotengine#62854.

`SubViewport` has some settings applied on initialization, and the side effects of these settings are delayed until it enters the tree.
@Rindbee
Copy link
Contributor Author

Rindbee commented Sep 16, 2022

Split into #64138 and #64152.

@Rindbee Rindbee closed this Sep 16, 2022
@Rindbee Rindbee deleted the fix-SubViewport-wrong-rendering branch September 19, 2022 00:44
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.

Vulkan: SubViewports not functioning unless update mode is set to "Always"
4 participants