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

[ME] Optimised generation of normal maps #231

Merged
merged 3 commits into from Mar 1, 2024

Conversation

takahiro0327
Copy link
Collaborator

The normal map texture conversion performed in the material editor has been optimised.
The two main changes are.

  • Conversion results are cached so that the same texture is not converted more than once.
  • Converted textures are not converted again.

Please merge if this looks good.

The scene loads a little faster.
Before ave 34.2[s]:

Scene loading completed: 34.7[s]
Scene loading completed: 33.7[s]
Scene loading completed: 33.8[s]
Scene loading completed: 34.4[s]
Scene loading completed: 34.4[s]

After ave 33.22[s] -0.98:

Scene loading completed: 32.6[s]
Scene loading completed: 33.1[s]
Scene loading completed: 33.8[s]
Scene loading completed: 33.3[s]
Scene loading completed: 33.3[s]

Peak memory usage during loading has been reduced from 10 GB to 5.6 GB.
image
Transition between memory usage and number of textures during scene loading.

before-after.ods
Code used for measurement.zip

@takahiro0327 takahiro0327 marked this pull request as draft February 29, 2024 15:10
@takahiro0327
Copy link
Collaborator Author

takahiro0327 commented Feb 29, 2024

Fixed builds not passing in API.MaterialEditor.

@takahiro0327 takahiro0327 marked this pull request as ready for review February 29, 2024 15:46
Copy link
Contributor

@ManlyMarco ManlyMarco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems fine. ConvertNormalMap is no longer virtual so if anything else tries to override it then that will be broken, but I don't think it was ever used outside of the ME repository.

@takahiro0327
Copy link
Collaborator Author

takahiro0327 commented Mar 1, 2024

As far as I checked the C# code in Github, there were no external plugins that call or define ConvertNormalMap.
So I hope it is ok.
https://github.com/search?q=ConvertNormalMap++language%3AC%23&type=code&l=C%23

As for the matter of deleting virtual, if it causes any problems, it will probably be a compile-time error. I believe the impact is negligible.
I believe that there is no funky code that is overriding at IL level.
No, that would be a runtime error.
I could add virtual for compatibility, but I would like to leave it as is, in the hope that it is not overridden.

public bool ConvertNormalMap(ref Texture tex, string propertyName)
protected virtual Texture ConvertNormalMap( Texture src )

@ManlyMarco
Copy link
Contributor

Seems like nothing is using this part of the API so I'll merge it.

@ManlyMarco ManlyMarco changed the title Optimised generation of normal maps [ME] Optimised generation of normal maps Mar 1, 2024
@ManlyMarco ManlyMarco merged commit cad1193 into IllusionMods:master Mar 1, 2024
@takahiro0327 takahiro0327 deleted the optimize-normalmap branch March 19, 2024 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants