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

Improve auto lod by comparing every attribute #73734

Closed
wants to merge 1 commit into from

Conversation

fire
Copy link
Member

@fire fire commented Feb 22, 2023

Fixes for some of the issues in #57416

  • a struct to a float indexing change.
  • always use 8 bones / 8 weights.
  • Add a ui for changing lod indices

@fire fire changed the title Improve auto lod by comparing every attribute. Improve auto lod by comparing every attribute and adding a minimum of 256 triangles Feb 22, 2023
@akien-mga akien-mga added this to the 4.x milestone Feb 22, 2023
@fire
Copy link
Member Author

fire commented Feb 22, 2023

@akien-mga do you think a pr that only increases the minimum to 256 triangle and or exposes the setting would be acceptable for 4.0?

@akien-mga
Copy link
Member

I don't know, that depends on the scope of the actual change and whether it's tested to fix reported critical issues.

@clayjohn
Copy link
Member

@akien-mga do you think a pr that only increases the minimum to 256 triangle and or exposes the setting would be acceptable for 4.0?

IMO this is definitely 4.1 material. LOD changes are support difficult for us to test as the surface area for testing is huge.

@jcostello
Copy link
Contributor

@akien-mga do you think a pr that only increases the minimum to 256 triangle and or exposes the setting would be acceptable for 4.0?

IMO this is definitely 4.1 material. LOD changes are support difficult for us to test as the surface area for testing is huge.

Can this break projects between 4.0 and 4.1?

@fire fire force-pushed the better-autolod branch 4 times, most recently from 353538d to 271874a Compare February 22, 2023 22:25
@fire
Copy link
Member Author

fire commented Feb 23, 2023

@clayjohn the only alternative that I can think of is set autolod to off and mark as experimental?

@realkotob
Copy link
Contributor

realkotob commented Feb 23, 2023

This fixes critical issues which is why I think it should go into 4.0, otherwise practically every user importing animated models will have broken results.

The other alternative as mentioned is to have autolod be off by default and marked as experiment, but in that case then this should also be merged anyway.

Testing this is not as complicated as it seems, just need to open a sizeable project (like the TPS demo) and reset the generated autolods and see if the game's assets look ok.

@fire
Copy link
Member Author

fire commented Feb 23, 2023

Out of band conversation. We agreed to have no change.

@fire fire force-pushed the better-autolod branch 3 times, most recently from c530160 to 5ea6aa5 Compare February 23, 2023 19:11
@fire fire changed the title Improve auto lod by comparing every attribute and adding a minimum of 256 triangles Improve auto lod by comparing every attribute Feb 23, 2023
Expose minimum_triangle_count and max mesh error property.
@clayjohn
Copy link
Member

clayjohn commented Apr 7, 2023

I tested this on the TPS demo a few weeks ago and found that this resulted in almost no LODs being generated. Additionally the base LOD had way more vertices. The net effect was that the scene consistently had way more geometry than needed, even if LOD bias was set to always use the worst quality

@fire
Copy link
Member Author

fire commented Apr 7, 2023

Can you provide some test case?

@clayjohn
Copy link
Member

clayjohn commented Apr 7, 2023

Can you provide some test case?

All I did is re-import the main meshes in the 4.0 branch of the TPS-demo https://github.com/godotengine/tps-demo. You can reproduce my finding with any one of the meshes in the project (i.e. just reimport the robot if you want to save time)

@fire
Copy link
Member Author

fire commented Apr 20, 2023

@clayjohn

<param index="3" name="minimum_index_count" type="int" />
<param index="4" name="maximum_mesh_error" type="float" />

Thoughts on extracting this pr to add hard overrides to the lod import with minimum_index_count and maximum_mesh_error.

@clayjohn
Copy link
Member

@clayjohn

<param index="3" name="minimum_index_count" type="int" />
<param index="4" name="maximum_mesh_error" type="float" />

Thoughts on extracting this pr to add hard overrides to the lod import with minimum_index_count and maximum_mesh_error.

Sounds like it could be helpful. But we definitely need to investigate a bit more. I have a feeling we should be able to produce better meshes with the default settings.

@fire
Copy link
Member Author

fire commented Jun 18, 2023

I tried a few of the test cases and after the AABB test fixes they're not failing anymore.

Can reopen if there's a need.

@fire fire closed this Jun 18, 2023
@fire fire deleted the better-autolod branch August 25, 2023 06:12
@AThousandShips AThousandShips removed this from the 4.x milestone Aug 25, 2023
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.

6 participants