-
-
Notifications
You must be signed in to change notification settings - Fork 21.4k
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
Ensure r_aabb
is always used when creating surfaces through the RenderingServer
#83840
Conversation
r_aabb
is always used when creating surfaces through the RenderingServer
Would it make sense to do |
e39f73b
to
3c17f55
Compare
I added one. In theory, there should be no case where r_aabb isn't written to. I tested the build from CI on my windows setup and it looks like it does work to avoid the bug. The question is still open as to whether there is another, better fix as well. @kevinkuo52 It would be interesting to see if just initializing r_aabb to AABB() is enough to fix the bug on your system. If so, then my theory is probably correct and this PR will properly fix the bug. |
just initializing r_aabb to AABB() didnt fix the issue on my machine (also tried setting AABB aabb = AABB()). huh this is interesting thought ur theory of "r_aabb is only guaranteed to be written out in the 2D " made a lot of sense. but just setting r_aabb to AABB() didnt fix it, also tried to set it in different places to make sure (like before towards begining of this function, were AABB aabb is currently initizliezed, in the block of using_normals_tangents before the continue) but still didnt fix it. it was only fixed with this change of operating directly on r_aabb. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confirmed working, so let's go.
If any MSVC compiler engineer wants to debug this further, let us know :)
Thanks! |
Tentative fix for #82890
@kevinkuo52 Is still investigating and may find a better fix. But this change is needed in either case as it is possible for compressed meshes to be created without an AABB and that is a potential big issue.
Link to download Windows Editory Binary for testing (taken from Github Actions)
Bugsquad edit: