-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Adds the ability to generate UVs automatically during mesh import #15530
Adds the ability to generate UVs automatically during mesh import #15530
Conversation
Consider turning off whitespace compare, as one of the files was ancient tab format and needed converting. |
Gems/SceneProcessing/Code/Source/Generation/Components/UVsGenerator/UVsGenerateComponent.cpp
Outdated
Show resolved
Hide resolved
Gems/SceneProcessing/Code/Source/Generation/Components/UVsGenerator/UVsPreExportComponent.h
Outdated
Show resolved
Hide resolved
...g/Code/Source/Generation/Components/UVsGenerator/UVsGenerators/SphereMappingUVsGenerator.cpp
Outdated
Show resolved
Hide resolved
This adds a new modifier to the available list of modifiers you can add to a mesh during its import in its import settings. The new modifier lets you select how to generate UVs for a mesh. Currently, only 1 generator is supported (Spherical Positional projection) but all the hard work here is done to support additional unwrappers in the future by following the pattern. Signed-off-by: Nicholas Lawson <70027408+lawsonamzn@users.noreply.github.com>
Signed-off-by: Nicholas Lawson <70027408+lawsonamzn@users.noreply.github.com>
…hub.com> Fixes from code review. Signed-off-by: Nicholas Lawson <70027408+lawsonamzn@users.noreply.github.com>
56cb390
to
3b30b28
Compare
Force pushed to get it to latest dev and also refactored it so that the components are hidden in CPP files. The headers now only declare enough forwards to spawn the component in the module and the classes they use to communicate with each other, such as the Context classes. |
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.
Very cool. Like you said, this should also make it easy to add other texture coordinate generators. Not sure how the spherical wrapping would look on something flat but it is clear how to add other methods that use position or normals to project onto the nearest plane.
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.
It would be really helpful to have some automated tests around this new rule.
Gems/SceneProcessing/Code/Source/Generation/Components/UVsGenerator/UVsGenerateComponent.cpp
Outdated
Show resolved
Hide resolved
Gems/SceneProcessing/Code/Source/Generation/Components/UVsGenerator/UVsGenerateComponent.cpp
Outdated
Show resolved
Hide resolved
I was following the pattern for tangent gen. Are there patterns to follow for tests of these generator components? I'll do a search and see if I can find them. If none exist, I'll see if its easy to add. OTOH, I don't have a huge amount of bandwidth to spend on this at the moment, else I'd have done something a lot more elaborate than just sphere map :) Maybe I can leverage those existing pipeline tests and include an asset that has no UVs or something... |
I think the Python tests that process FBX files and compare the output to a last known good are probably enough for here: https://github.com/o3de/o3de/tree/development/AutomatedTesting/Gem/PythonTests/assetpipeline/scene_tests Functionally these tests end up comparing the output to a last known good. I'll add some additional details on the tests and the strategy around them in the other pull request. |
I'm actually not sure how useful these fbx_tests are. They don't run automatically, and largely seem like they cause a debug export of the scene data and cause it to be compared with expected scene data? That seems like I'd be just setting maintainers up for a huge headache anytime anyone changed anything about scenes. I'm going to see if there's some other way to test that this basic functionality works. (STarting with "whats the most likely failure points for this for future maintainers + what would be the worst possible kinds of failures to occur"). I'm thinking "UV rule present, but no UV data is emitted" + "UV data is already present, 'do not overwrite' is set, and it overwrites anyway" or the like. |
FWIW, I tried to fix and update those tests, (they were failing), only to find out that they're essentially useless.
|
I tried adding tests, but the framework is not useful right now. Signed-off-by: Nicholas Lawson <70027408+lawsonamzn@users.noreply.github.com>
…de#15530) * Adds the ability to generate UVs automatically during mesh import This adds a new modifier to the available list of modifiers you can add to a mesh during its import in its import settings. The new modifier lets you select how to generate UVs for a mesh. Currently, only 1 generator is supported (Spherical Positional projection) but all the hard work here is done to support additional unwrappers in the future by following the pattern. This ability is crucial for workflows to do with importing robotic meshes as they often lack UVs, and the path to otherwise get something reasonable to render is painful Signed-off-by: Nicholas Lawson <70027408+lawsonamzn@users.noreply.github.com>
…5530) (#15675) * Adds the ability to generate UVs automatically during mesh import This adds a new modifier to the available list of modifiers you can add to a mesh during its import in its import settings. The new modifier lets you select how to generate UVs for a mesh. Currently, only 1 generator is supported (Spherical Positional projection) but all the hard work here is done to support additional unwrappers in the future by following the pattern. This ability is crucial for workflows to do with importing robotic meshes as they often lack UVs, and the path to otherwise get something reasonable to render is painful Signed-off-by: Nicholas Lawson <70027408+lawsonamzn@users.noreply.github.com>
What does this PR do?
This adds a new modifier to the available list of modifiers you can add to a mesh during its import in its import settings.
Pictures:
Before:
Add a modifier in the import settings
And done
The new modifier lets you select how to generate UVs for a mesh. Currently, only 1 generator is supported (Spherical Positional projection) but all the hard work here is done to support additional UV-unwrappers in the future by following the pattern (ie, adding to the case statement and adding to the enum).
#12795 is addressed partially by this.
The old behavior was that meshes that had no UV data had to have UVs added to them in order to render with the current set of PBR shaders. The only recourse was to go into something like blender and add UVs and re-export. However, in many situations like for example robotics or simulation, much of the input data do not have UVs and are meant for simple color or shape representation, or procedural texturing.
Spherical unwrapping isn't going to be as good as one of the more advanced unwrappers, but for getting some shading on meshes that really don't care about their material, its a start.
This adds a new modifier to the available modifiers for when you ingest meshes, just like the already existing "generate tangents" rule, you can now add a "generate UVs" rule. It follows that pattern, so doesn't really invent anything new.
If no modifier is present (all existing data), no changes are made.
If the modifier is present, the default is to use spherical UVs. The default can be changed in the regset file.
The change to the scenesettings file is as simple as adding
to the list of rules applicable to the model. With optional "replaceExisting"
How was this PR tested?
Testing it manually using meshes that have no UV data and meshes that previously existed. The defaults are configured in such a way to have no effect whatsoever to existing data. Someone has to go and actually add these rules to the import settings for individual meshes to get the benefit. (Basically following in the footsteps of the tangent generation system).