-
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
EXCEPTION REQUEST: Adds the ability to generate UVs automatically during mesh import #15675
EXCEPTION REQUEST: Adds the ability to generate UVs automatically during mesh import #15675
Conversation
…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>
In case it helps, if this is NOT merged, the import pipeline for assets specifically missing UVs (which are fairly common in the world of robotics/simulation) will come in as featureless unshaded gray blobs until the user trying to import them actually pulls them into a CAD package and generates UVs. This is particularly problematic since most of those users are not CAD experts and the files usually lack the original CAD source. As for any risk introduced, this change introduces an option that is not enabled or added to any scenes by default, and even if added (by hand, by a human), it will not overwrite existing content unless a user explicitly checks a box on each and every instance they want it to happen on, individually. |
I am approving the exception as an unrelated SIG representative from SIG-Core as the need for mesh UV in the robotics models would really help with visually distinguishing the content. |
To perform a manual test: Right click the FBX Asset in asset browser -> "Edit Settings", "Add Modifier" -> UVs --> Save. See how it now works just fine in the viewport and no longer spews warnings. Note that only things you explicitly add this modifier to are affected in any way, and even adding the modifier will not affect anything that already has UVs. |
Missed that this was an exception request but I previously reviewed the changes. |
Approving this exception from SIG Testing, reviewed the changes and this looks like an impactful change that we should take. |
As chair of SIG-Content I approve this request, this improves quality when no UVs are present, it improves workflows & quality when it relates to bringing in assets into O3DE |
Spoke with Nicholas and talked about the UX use cases. No concerns at this point about UX pipeline SIG-UI-UX approved. |
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.
I've read through the change and for the most part it looks really clean. I'll also approve as an unrelated sig representative, however I would really like to see a deeper review done by the asset team, as well as a plan for testing once merged, since this is going in without any associated unit tests and after the new feature cutoff.
As a SIG Co-chair, I approve this exception request. |
@vincent6767 is the co-chair for sig-release. This means this ticket has met the necessary approvals to merge. |
Has this been tested post-merge @lawsonamzn ? |
Yes, it works the same as it worked before merge. |
Note to reviewers, this PR is already in 'development branch' but this is a PR to get it into the Stabilization branch as an Exception.
I will be adding the exception checklist below. Please see the original PR for a full brief including pictures of the difference it makes.
ORIGINAL PR from Development
#15530 - solves #12795 issue.
Without this PR merged, the robotics stuff in o3de-extras will seem incomplete for the release as all models will appear as featureless gray shadeless objects.
The new modifier is implemented in the same UX as the existing modifiers.
https://user-images.githubusercontent.com/70027408/229643430-e28c85ae-4e56-4be2-872c-619650ca2220.png
This PR is a cherry-pick of that squash submission.
Original PR text
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
Exception Checklist
(Must occur at some point before SIG-Release approval) Code has been approved for merging by maintainers