usdShade: Adding NormalMapTexture validator #3359
Conversation
| { | ||
| errors.emplace_back( | ||
| UsdShadeValidationErrorNameTokens->nonCompliantScale, | ||
| UsdValidationErrorType::Warn, |
There was a problem hiding this comment.
@tallytalwar in the compliance checker, this is a warning, however the bias check below is an error. Is that correct? I wanted to change the bias error below to be a warning, let me know what you think
There was a problem hiding this comment.
Referring the original commit for bias and scale adjustments to compliance checker: b718721
When complying with UsdPreviewTexture and UsdUVTexture (that is when nonCompliantScaleValues is false) we need to make sure to report error for bias values. Refer these here: https://github.com/PixarAnimationStudios/OpenUSD/blob/release/pxr/usdImaging/plugin/usdShaders/shaders/shaderDefs.usda#L100-L112
Regarding non-conforming scale values being reported as warnings: There is a reasoning here: https://github.com/PixarAnimationStudios/OpenUSD/blob/dev/pxr/usd/usdUtils/complianceChecker.py#L550-L552 (I will request to keep all these code comments as-is in the cpp so as to save the reasoning). I believe Blender is the application mentioned here (cc. @dgovil is that still the case?)
There was a problem hiding this comment.
made sure to include all comments from the complianceChecker code (I did miss one). Also rebased this branch
| } | ||
|
|
||
| void | ||
| TestUsdShadeNormalMapTextureValidator() |
There was a problem hiding this comment.
@tallytalwar There are quite a few failure conditions in this test and validator, let me know if you think it would be better to break this up in to multiple validators or not
There was a problem hiding this comment.
@tallytalwar We had the issue in the past with the encapsulation validator where me adding external files did not work for you, just calling this out here as this may happen again as I don't know what the difference was here in the past.
|
Filed as internal issue #USD-10296 |
|
/AzurePipelines run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
- added new validatorTokens - added implementation to match previous complianceChecker implementation - added tests to test each failure condition
beersandrew
force-pushed
the
usdShade-normalMapTexture-validator
branch
from
ad1424b
to
454ae8c
Compare
- remove unnecessary check - add comment from complianceChecker.py that was missed
|
/AzurePipelines run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Description of Change(s)
Added an implementation of NormalMapTexture validation rules
Fixes Issue(s)