CE-6909: Serlio: Arnold Renderer Support #25

jonathan-meier · 2019-09-25T11:27:23Z

Reasonably complete initial support for Arnold materials with PRT.

Note: this PR depends on PR #24.

src/serlio/prtMaterial/ArnoldMaterialNode.cpp

src/serlio/prtMaterial/MaterialInfo.h

src/serlio/prtMaterial/MaterialInfo.cpp

src/serlio/util/MELScriptBuilder.cpp

src/serlio/prtMaterial/ArnoldMaterialNode.cpp

mistafunk

good work, just found some minor things

Please avoid the "TODO" in the code and write new issues instead
Q: what is the expected behavior if you create arnold materials on a model which already has stingray materials? (arnold just renders pink in my experiment)

jonathan-meier · 2019-10-02T10:20:57Z

Thanks for the review! I addressed all the comments and also fixed some more issues I found:

the bump, normal, roughness and metallic maps should be read as raw textures
the normal map should be explicitly converted from RGB to XYZ using an aiNormalMap shader
since in glTF specularity is entirely controlled via the roughness, we need to set the specular color to white on the aiStandardSurface shader to get matching rendering results

I removed all the TODOs and created individual issues for them.

Currently, you can only create either Stingray materials or Arnold materials for a particular model. Mixing the two (i.e. creating Arnold materials when there already are Stingray materials or vice-versa) is undefined behavior and likely not to work. In the worst case it might even mess up the existing materials.

mistafunk · 2019-10-10T09:29:22Z

looks good & ready to merge.

if you still have time there are two things in ArnoldMaterialNode::buildMaterialShaderScript you could maybe spend 1-2 hours on:

using R"" string literals to avoid having to escape all the double quotes in the mel builder
find a way to deduplicate all the very similar map setup blocks

mistafunk · 2019-10-10T09:55:24Z

Functional Issue:

quad 10x10
apply medieval rpk
activate "High LOD" attribute
create arnold materials
render
activate "roof tiles" attribute
-> unexpectedly missing/wrong materials

…ng double vectors

…is a failure

…018 API MPlugArray only provides STL compliant iterators starting from Maya 2019 API. The wrapper adds this functionality for the Maya 2018 API and provides helper functions to target both APIs in a uniform manner.

…w data

…Info

…ormal map instead of displacement

…del attributes

jonathan-meier · 2019-10-10T16:10:53Z

Fixed the functional issue, which was due to us reconnecting to already connected in-plugs. The connections were not made, since we didn't force them and therefore the fix is to just force them.
Deduplicated the map setup blocks as suggested.

mistafunk · 2019-10-11T09:09:42Z

Found another issue while testing with the FAVELA example:

Stingray:

Arnold (missing materials and warnings):

// Warning: line 24: 'serlioGeneratedArnoldMaterialSh0.outColor' is already connected to 'serlioGeneratedArnoldMaterialSg0.surfaceShader'. // // Warning: line 28: 'serlioGeneratedArnoldMaterialSg0_dirt_multiply.outColor' is already connected to 'serlioGeneratedArnoldMaterialSh0.baseColor'. // // Warning: line 31: 'serlioGeneratedArnoldMaterialSg0_color_map_blend.outColor' is already connected to 'serlioGeneratedArnoldMaterialSg0_dirt_multiply.input1'. // // Warning: line 43: 'serlioGeneratedArnoldMaterialSg0_color_map.outColor' is already connected to 'serlioGeneratedArnoldMaterialSg0_color_map_trafo.passthrough'. // // Warning: line 44: 'serlioGeneratedArnoldMaterialSg0_color_map_trafo.outColor' is already connected to 'serlioGeneratedArnoldMaterialSg0_color_map_blend.input2'. // // Warning: line 47: 'serlioGeneratedArnoldMaterialSg0_bump_value.outNormal' is already connected to 'serlioGeneratedArnoldMaterialSh0.normalCamera'. // // Warning: line 53: 'serlioGeneratedArnoldMaterialSg0_specular_map_blend.outColor' is already connected to 'serlioGeneratedArnoldMaterialSh0.specularColor'. // // Warning: line 65: 'serlioGeneratedArnoldMaterialSg0_specular_map.outColor' is already connected to 'serlioGeneratedArnoldMaterialSg0_specular_map_trafo.passthrough'. // // Warning: line 66: 'serlioGeneratedArnoldMaterialSg0_specular_map_trafo.outColor' is already connected to 'serlioGeneratedArnoldMaterialSg0_specular_map_blend.input2'. // // Warning: line 69: 'serlioGeneratedArnoldMaterialSg0_opacity_map_blend.outColorR' is already connected to 'serlioGeneratedArnoldMaterialSh0.opacityR'. // // Warning: line 70: 'serlioGeneratedArnoldMaterialSg0_opacity_map_blend.outColorR' is already connected to 'serlioGeneratedArnoldMaterialSh0.opacityG'. // // Warning: line 71: 'serlioGeneratedArnoldMaterialSg0_opacity_map_blend.outColorR' is already connected to 'serlioGeneratedArnoldMaterialSh0.opacityB'. // // Warning: line 86: 'serlioGeneratedArnoldMaterialSg0_normal_map.outColor' is already connected to 'serlioGeneratedArnoldMaterialSg0_normal_map_trafo.passthrough'. // // Warning: line 90: 'serlioGeneratedArnoldMaterialSg0_normal_map_trafo.outColor' is already connected to 'serlioGeneratedArnoldMaterialSg0_normal_map_convert.input'. // // Warning: line 91: 'serlioGeneratedArnoldMaterialSg0_normal_map_convert.outValue' is already connected to 'serlioGeneratedArnoldMaterialSg0_bump_value.normalCamera'. // // Warning: line 95: 'serlioGeneratedArnoldMaterialSg0_emissive_map_blend.outColor' is already connected to 'serlioGeneratedArnoldMaterialSh0.emissionColor'. // // Warning: line 100: 'serlioGeneratedArnoldMaterialSg0_roughness_map_blend.outColorR' is already connected to 'serlioGeneratedArnoldMaterialSh0.specularRoughness'. // // Warning: line 105: 'serlioGeneratedArnoldMaterialSg0_metallic_map_blend.outColorR' is already connected to 'serlioGeneratedArnoldMaterialSh0.metalness'. //

mistafunk

Please investigate and fix the material assignment bug described above.

Here is a new maya scene without materials assigned, simply create arnold materials and the bug should be visible:
favela_3_multple_lots_no_materials.mb.zip

…rnold material These parts were originally copied from PRTMaterialNode.

…l node types

…ers having the same name The logic for making our shader names unique relies on the compute method of the arnold material node seeing all the other existing nodes in the scene. However, since the compute method of several nodes can be executed in parallel, two or more arnold material nodes may compute the same 'unique' name for their shaders, thereby breaking uniqueness. To fix this issue, we explicitely set the scheduling type of the arnold material node as globally sequential node. Moreover, the assembled MEL script is executed asynchronously. We specifically execute the creation of the shader node synchronously such that the compute method of following arnold material node updates is guaranteed to see these nodes and consider them for finding unique node names. However, this is fragile and may be a source of errors, since the method used is not thread safe.

jonathan-meier · 2019-10-11T15:25:40Z

Fixed with a workaround and opened two follow-up issues (#40 and #41) to address the issue more thoroughly, which would be out of the scope for this PR/ticket.

mistafunk · 2019-10-11T16:35:32Z

favela test with multiple buildings works fine now.
there are still warnings but these will be covered by the newly created issues...

jonathan-meier requested a review from mistafunk September 25, 2019 11:27