Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Handling 3dsMax's Simplified Physical Material import. #569

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.

Sign up for GitHub

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

Jump to bottom
Merged
sebastienlagarde merged 4 commits into from
Jun 4, 2020
Merged

Handling 3dsMax's Simplified Physical Material import. #569

sebastienlagarde merged 4 commits into from
Jun 4, 2020

Conversation

@Thomasch-unity3d
Copy link
Contributor

@Thomasch-unity3d Thomasch-unity3d commented May 20, 2020

Guide : https://github.com/Unity-Technologies/Graphics/blob/master/.github/pr-read.png.md

Display Addon : https://userstyles.org/styles/182991/unity-graphics-pr-readme

Purpose of this PR

This PR adds support for 3DsMax's 2021 Simplified Physical Material ( also referred to as PBR (metal/rough) ) from FBX files in the Model Importer.
In order to avoid adding new shaders, this material maps to the existing Autodesk Interactive shader.
AssetPostprocessor versions were not updated to avoid unnecessary re-imports as it's likely that a vast majority of FBX files already imported are not impacted by the change.

Testing status

Manual Tests

Manually tested with assets with several materials with varying settings/maps.
Tested upgrade of SRP version from released package to branch.

Automated Tests

No new test added.

Links

Yamato: (Select your branch) https://yamato.prd.cds.internal.unity3d.com/jobs/902-Graphics

Comments to reviewers

@Thomasch-unity3d Thomasch-unity3d requested a review from a team as a code owner May 20, 2020 15:33
@Thomasch-unity3d Thomasch-unity3d requested a review from eh-unity May 20, 2020 15:33
etienne-unity
etienne-unity approved these changes May 26, 2020
View reviewed changes
Copy link

@etienne-unity etienne-unity left a comment

Choose a reason for hiding this comment

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

All values and textures are applied properly within the restrictions of the conversion to Autodesk Interactive Unity shader.

  • Tested in HDRP and URP with all fields used with both values and textures.

@eh-unity
Copy link
Contributor

eh-unity commented May 29, 2020

This is a bit offtopic, but FYI:

It's a bit unfortunate that the shader uses "_MainTex" and "_Color" instead of "_BaseMap" and "_BaseColor" like the most of the shaders in URP. It would be nice to have consistent naming.

I added shader attributes [MainTexture] and [MainColor] to get rid of hardcoded names and mark any property as the main tex/color. This is used in the scripting API.
Unfortunately the engine side API isn't that great (a building block for script side), but could be used to give some forward compatibility if we ever decide to align/harmonize the naming.
That should be a separate PR though (and there are other similar cases that could be fixed in a batch), so I'll approve this PR.
Also the shader is from graph, so another thing to consider.

eh-unity
eh-unity approved these changes May 29, 2020
View reviewed changes
Copy link
Contributor

@eh-unity eh-unity left a comment

Choose a reason for hiding this comment

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

Looks fine.

@Thomasch-unity3d Thomasch-unity3d force-pushed the 3dsmax-simplified-physical-material-import branch from 608a066 to 2f11307 Compare June 1, 2020 18:02
@sebastienlagarde sebastienlagarde merged commit 35daf9d into master Jun 4, 2020
@sebastienlagarde sebastienlagarde deleted the 3dsmax-simplified-physical-material-import branch June 4, 2020 10:45
sebastienlagarde pushed a commit that referenced this pull request Jun 6, 2020
@Thomasch-unity3d @sebastienlagarde
Handling 3dsMax's Simplified Physical Material import. (#569)
7ba2866
@sebastienlagarde sebastienlagarde mentioned this pull request Jun 6, 2020
Merged
6 tasks
sebastienlagarde added a commit that referenced this pull request Jun 12, 2020
@sebastienlagarde @Thomasch-unity3d
@NicoLeyman @JordanL8 @thomas-zeng
[9.x.x] Misc doc and FBX backport [Need C++ PR] (#751)
865e56d 
* Handling 3dsMax's Simplified Physical Material import. (#569)

* [Skip CI] Virtual Texturing cache settings (#536)

* Hdrp/docs/volumetric lighting format fix (#628)

* Updated volumetric lighting and subsurface scattering docs

* Update Override-Diffusion-Profile.md

* Adds mention of fidelityfx-cas as requested by AMD (#629)

* * Updated XR mirrorview logic to use `TryGetAdditionalCameraDataOrDefault`. (#641)

* Update Light-Component.md (#682)

* Update HDRP-Camera.md (#706)

* Revert "[Skip CI] Virtual Texturing cache settings (#536)"

This reverts commit 9794586.

Co-authored-by: Thomasch-unity3d <30902625+Thomasch-unity3d@users.noreply.github.com>
Co-authored-by: NicoLeyman <49522355+NicoLeyman@users.noreply.github.com>
Co-authored-by: JordanL8 <lewis.jordan@hotmail.co.uk>
Co-authored-by: thomas-zeng <49886741+thomas-zeng@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@etienne-unity etienne-unity etienne-unity approved these changes

@eh-unity eh-unity eh-unity approved these changes

+1 more reviewer

@sebastienlagarde sebastienlagarde sebastienlagarde approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

backported-9.x HDRP universal

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Thomasch-unity3d @eh-unity @sebastienlagarde @etienne-unity
Morty Proxy This is a proxified and sanitized view of the page, visit original site.