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

Vulkan Clustered: Fix culling of negatively-scaled objects #67176

Merged
merged 1 commit into from
Oct 11, 2022

Conversation

cooperra
Copy link
Contributor

@cooperra cooperra commented Oct 10, 2022

Negatively scaled objects should be mirrored. This is already implemented, but it breaks when mirrored and non-mirrored instances of the same object are visible together.

It turns out that the code that skips-over repeats in RenderForwardClustered::_render_list_template also skips the code that accounts for the culling mode of mirrored objects.

The solution here is to consider the mirror flag when determining repeats. This might result in more draw commands than necessary since a mirrored object can split a group of non-mirrored instances in two. Maybe we can optimize that later? I don't expect mirrored objects to be common.

This problem doesn't appear in the mobile renderer because the repeat optimization isn't implemented there yet.

The problem still appears in MultiMeshInstance3D in all renderers.

Fixes #62879 and fixes #58546.

Negatively scaled objects should be mirrored. This is already implemented, but it breaks when mirrored and non-mirrored instances of the same object are visible together.

It turns out that the code that skips-over repeats in `RenderForwardClustered::_render_list_template` also skips the code that accounts for the culling mode of mirrored objects.

The solution here is to consider the `mirror` flag when determining repeats. This might result in more draw commands than necessary since a mirrored object can split a group of non-mirrored instances in two.

This problem doesn't appear in the mobile renderer because the repeat optimization isn't implemented there yet.

The problem still appears in MultiMeshInstance3D in *all* renderers.

Fixes godotengine#62879 and godotengine#58546.
@cooperra cooperra requested a review from a team as a code owner October 10, 2022 07:45
@akien-mga akien-mga added this to the 4.0 milestone Oct 10, 2022
@fire fire requested a review from a team October 10, 2022 18:29
@fire
Copy link
Member

fire commented Oct 10, 2022

May fix #54708

@akien-mga akien-mga merged commit 422c398 into godotengine:master Oct 11, 2022
@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