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.

Sign up for GitHub

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

Jump to bottom

mesh indexing failing with small scale values #44799

Merged
merged 1 commit into from
Jan 26, 2021
Merged

mesh indexing failing with small scale values #44799

merged 1 commit into from
Jan 26, 2021

Conversation

RevoluPowered
Copy link
Contributor

@RevoluPowered RevoluPowered commented Dec 29, 2020 •
edited
Loading

Fixes:

  • Regression in fbx with new indexer #44655 fbx files not working in latest release
  • previous behavior was culling scale values of 0.01 and det == 0.00004. which were valid in FBX land, det was not == 0, but in the cases of actual det==0 scenario which can be reproduced with node_3d_editor_plugin you can see it actually has a det == 0.
  • node_3d_editor_plugin was creating gizmos with transforms with det == 0, this is safe to do now due to this change, I have also fixed this for the gizmos but the grid still needs this to be done for it. This relies on absolute zero values to change visibility so I fixed this.
  • fixed fbx material mappings going to metallic property

I debugged this by putting checks on det == zero.
I checked bistro fbx and it imports fine, no dropped geometry anymore even with 0.01 scaling and det == 0.00004.

debug build
image

release_debug build (cpu time halves :D)
image

without this PR

image

@RevoluPowered RevoluPowered requested a review from reduz December 29, 2020 21:09
@Calinou Calinou added this to the 4.0 milestone Dec 29, 2020
@RevoluPowered RevoluPowered force-pushed the fbx-fix-zero-scaling branch 2 times, most recently from 498554a to 5018824 Compare December 30, 2020 10:26
@RevoluPowered RevoluPowered linked an issue Dec 31, 2020 that may be closed by this pull request
Regression in fbx with new indexer #44655
Closed
@AndreaCatania AndreaCatania mentioned this pull request Jan 21, 2021
Closed
@AndreaCatania
Copy link
Contributor

This PR fixes this issue: #45067

@akien-mga
Copy link
Member

Needs a rebase @RevoluPowered.

@reduz
Copy link
Member

reduz commented Jan 21, 2021

Current BVH does not really allow flat AABBs so this fix will do, but I want to replace by one that does soon so this fix will be irrelevant by then. I suggest lets just merge for now so things work.

@RevoluPowered
Fix zero scaling and material mappings being mapped to wrong fields
86c7faa 
- fixes scale values of 0.0013 (det == 0.00004) not rendering, they should render even at small values, but not at zero like the editor grid plugin supplies zero exactly.
- fixes node_3d_editor_plugin visibility bug when scale is zero
- fix culling with small scaling values - which are still valid to be rendered like 0.00004

note: grid is still not fixed, it has det == 0 issues but this fixes one of them.
@RevoluPowered
Copy link
Contributor Author

Rebase is completed.

@akien-mga akien-mga merged commit 1f5669d into godotengine:master Jan 26, 2021
@akien-mga
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akien-mga akien-mga akien-mga left review comments

@AndreaCatania AndreaCatania AndreaCatania approved these changes

@reduz reduz Awaiting requested review from reduz

Assignees
No one assigned
Labels
bug topic:import topic:rendering
Projects
None yet
Milestone
4.0
Development

Successfully merging this pull request may close these issues.

Regression in fbx with new indexer
5 participants
@RevoluPowered @AndreaCatania @akien-mga @reduz @Calinou