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 frustum top plane calculation #807

Merged
merged 1 commit into from
Dec 22, 2022
Merged

Fix frustum top plane calculation #807

merged 1 commit into from
Dec 22, 2022

Conversation

gmodarelli
Copy link
Contributor

Hello there, I've spotted a typo in the computation of the frustum top plane.
I've briefly checked the other Mesh Shading samples and there are no bugs there.

Cheers :)

@walbourn walbourn added bug samples Issues related to Samples labels Dec 21, 2022
@stanard stanard merged commit 0aa79ba into microsoft:master Dec 22, 2022
@gmodarelli gmodarelli deleted the frustum-plane-fixes branch December 23, 2022 07:33
@walbourn
Copy link
Member

walbourn commented Jan 3, 2023

Can you provide a description of how to repro the failure?

@gmodarelli
Copy link
Contributor Author

Hey there, sorry I missed this message. Yes, I'll try to add a "Freeze Render" option so you can see how meshes outside of the top plane of the frustum are not culled.

Is it enough if I provide screenshots of this or would you like me to commit the code as well?

@stanard
Copy link
Member

stanard commented Jan 9, 2023

I already merged this change. Coincidentally, we found the same bug in a branch of MiniEngine and already fixed it with the same change. There is symmetry with each of the opposing planes (add/subtract), and by inspection, you can see that there was a typo on one of the planes.

@walbourn
Copy link
Member

walbourn commented Jan 9, 2023

We ported this fix to the ATG samples copy as well and were able to repro the original issue to verify the fix. Looks good.

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

Successfully merging this pull request may close these issues.

3 participants