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

Technical Debt: Shader exporters vary wildly in quality, feature support, and stability (USDU-248) #193

Open
hybridherbst opened this issue Jul 4, 2020 · 2 comments

Comments

@hybridherbst
Copy link
Contributor

While going through the code I noticed a couple of places now that need much more testing:

  • PBR attribute exporting is handled very differently between HDRP and Built-In
  • URP support is missing (given that Unity has now officially mentioned "yes, this will be the default" this needs to happen at some point)
  • exporters check for properties, not shader keywords, in most places (in some the keyword checks are in place);
    for example, material.HasProperty("_SpecGlossMap") will return true if this material has ever been set to "Specular", a texture attached, and then set back to "Roughness" workflow. Right way would be to check if actual shader keywords are set for known shaders.
  • there's magic hardcoded values scattered in the code (e.g. specular color is set to 0.4, 0.4, 0.4 if material has no spec color)
  • the workflow around linear textures, sRGB textures, and color spaces (linear vs. gamma) is unclear (and we're seeing color differences between Unity, USD, and AR QuickLook because of this I assume)
  • the shaders for ChannelCombiners etc. are not in Resources, which means they will most likely be missing from builds unless a user explicitly adds them (this will cause certain, but not all, imports/exports to fail in a build)
@jcowles
Copy link
Contributor

jcowles commented Jul 8, 2020

Some of the differences are required (e.g. because HDRP implementation is actually different from built-in), but in general, I totally agree with this -- I actually feel like there is a ton of accrued tech debt across the import and export pipelines and all of it should be revisited with a new holistic design.

I actually think there is less tech debt in shader import/export and much more in the geometry pipelines. UsdSkel support is particularly messy.

@riemaeker
Copy link

Any updates on this? Especially URP support would be very welcome :)

@etienne-unity etienne-unity changed the title Technical Debt: Shader exporters vary wildly in quality, feature support, and stability Technical Debt: Shader exporters vary wildly in quality, feature support, and stability (USDU-248) Aug 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants