MaterialX icon indicating copy to clipboard operation
MaterialX copied to clipboard

Implementation of thin-walled / double-sided materials

Open klucknav opened this issue 4 years ago • 31 comments

It looks like the only implementation for thin_surface nodes is in MDL. Missing both a GLSL and OSL implementation. Unfortunately, without the GLSL implementation, we cannot use this node in either MaterialX View or Usdview with Storm. And the OSL implementation would be needed to use with Usdview with Prman.

klucknav avatar Mar 10 '22 20:03 klucknav

Hi @klucknav, I just want to add some context on this.

We have refrained from working on an implementation of thin_surface so far, because its definition is debatable. When we defined the PBR library it seemed like a natural way to separate closed surfaces with volume (the surface node) from thin-walled surfaces (the thin_surface node). But in hindsight it's not a good approach.

The first problem is that there is no natural way to have a user controlled switch between the two, for example as needed for an uber shader. You could use a surfaceshader mix node to switch between the one or the other, but then you introduce the concept of having a surface that could be 50% closed and 50% thin-walled, which doesn't makes sense.

In addition, we now have the "unlit" version of surface, which then by definition becomes a closed volume unlit surface. So we would then need an "unlit" version of thin_surface as well, which is quite messy.

So we have come to the conclusion that we need a better way to define closed vs. thin-walled surfaces. A proposal we are considering is to move the definition of closed vs thin-walled to the material level. Adding inputs on the surfacematerial constructor node for both frontfacing and backfacing surfaceshaders, as well as a boolean switch to say if it's thin-walled or not.

The thin_surface node would then be removed and replaced with this more robust definition.

niklasharrysson avatar Mar 11 '22 17:03 niklasharrysson

Here is a proposal to discuss for the surfacematerial node to make it support thin-walled / double-sided materials:

  <!-- Surface material -->
  <nodedef name="ND_surfacematerial" node="surfacematerial" nodegroup="material">
    <input name="surface" type="surfaceshader" value="" />
    <input name="backface" type="surfaceshader" value="" />
    <input name="thin_walled" type="boolean" value="false" />
    <input name="displacement" type="displacementshader" value="" />
    <output name="out" type="material" />
  </nodedef>

If thin_walled is enabled the material is thin-walled and double-sided, where surface and backface defines the surfaceshaders on each side respectively. If thin_walled is disabled (the default) the material is a closed surface with an interior volume, where surface defines the single surfaceshader and backface is ignored.

Upgrading documents from the old surfacematerial node is straight forward since the default behavior is the same. The upgrade just need to rename the inputs.

niklasharrysson avatar Mar 15 '22 15:03 niklasharrysson

@niklasharrysson This proposal looks great to me, and I'll forward the suggestion from a parallel thread that this should be a new surfacematerial_doublesided node, rather than a modification of the existing surfacematerial node. This should make it more straightforward for renderers to detect when a node graph references double-sided materials, which they may wish to handle with a separate code path.

jstone-lucasfilm avatar Mar 26 '22 04:03 jstone-lucasfilm

This proposal does seem like a more flexible solution than a special thin_surface shader. From a USD perspective, It, like any change to what a material definition is, implies a corresponding change to the UsdShadeMaterial schema. In USD, sidedness is a property of geometry, not the material, and taking that with the description above of the two newly proposed properties, I wanted to question the necessity of the thin_walled property. Its presence does provide a separation of concerns, but it also makes for extra steps, and a possible "inconsistent state" that is defined away as backface is ignored. To me it seems natural that specifying a backface behavior for double-sided geometry implies thin surface, so I'd ask if it makes sense to have backface be the sole new property? But maybe there are good workflow reasons for wanting the extra boolean guard?

Secondly, although I understand that Lama and MaterialX makes it possible to attach completely different bsdf's to each side of the thin surface, there are renderers that aren't, in complete generality, that flexible yet. So in describing the backface property, we were thinking of providing guidance that (from a UsdShade perspective) it may be safer to use the same kind of bsdf for both sides.

Thanks much for looking into this, @niklasharrysson !

spiffmon avatar Mar 28 '22 16:03 spiffmon

I am pleased to see that clarification and thought around the issues of double-sided materials is under way.

I'd like to explore @spiffmon's comment that in USD, sidedness is a property of geometry.

In general, I hold that statement as a maxim, not just for USD. Hence the "parallel thread" suggestion on introducing a new material node that's explicitly meant for double sided geometry, for those cases where this thin-walled/double-sided notion is absolutely required.

I want to raise the issue that if MaterialX allows geometry properties (sidedness) to be affected in the material node, it means that a renderer has to do extra work to discover if any node in a graph is sided, and if so, the network the node participates in can be polluted with sidenedness, and thus all geometry bound to the network will itself become polluted with sidedness. In the worst case limit, if a single node forces double sided rendering, it might force useless double rendering of everything. The alarm bell is that it has the potential of an artist introducing the One Node that pessimizes all render times, and even if a conformance tool discovers the issue, the One Slow Node might be part of the One Holy Network That Production Can't Change.

That's where the proposal for a new material node comes from. An explicit node would mean that a potentially pessimizing choice must be deliberately made and easily identified, rather than introducing a requirement that renderers must introspect every node to discover its pessimizing potential.

So in conclusion, I don't think that geometry should be modified by a material, and if it must, I think it should be done in a highly visible and controlled manner. Furthermore, propagation rules may be required to specify how far the double-sided requirement reaches.

meshula avatar Mar 28 '22 21:03 meshula

Thanks for the great feedback @spiffmon and @meshula

@spiffmon You are precisely right that the thin_walled input is a bit redundant in this sense. The main reason it was added to this proposal is that there are existing shading models that have such a user-controlled switch. Examples include Autodesk Standard Surface and glTF PBR, which we want to describe in MaterialX. In addition, Nvidia MDL also has such a switch in its material definition.

But I agree we should not introduce redundancy unless there is a very good reason. Looking more closely at what the thin-walled property does in practice for the mentioned use-cases above, it alters the transmissive properties of the material, disabling refraction and disabling volumetric scattering and absorption. I think this could be implemented explicitly in a BSDF graph instead, for the shading models that need this switch. For example, one could mix/switch between different BSDF nodes based on this property. This might actually be a better solution since each shading model may have slightly different opinions on how thin-walled should be handled. So in other words we could probably remove the thin_walled property in this proposal, and let each shading model handle it in their implementation graph.

I also agree with your second point that we should provide guidance that for double-sided materials (however we introduce this) the BSDF's on both sides must be of the same kind. At least the transmissive properties must be the same.

@meshula this is a great point, and I think we all agree that if we need double-sided materials it should be introduced as a separate material node for easier identification.

However I think we could use some more information on how this is handled on geometry level in USD. Are you able to assign different materials to each side when the geometry is tagged as double-sided?

I'm not sure we could rely on having such a material assignment workflows in applications that want to adopt MaterialX outside of USD. But it's definitely something to discuss more.

Thanks.

niklasharrysson avatar Mar 29 '22 11:03 niklasharrysson

There's practice, and there's specifications :) Without addressing USD, here's what MaterialX has to say:

It is common for shading models to differentiate between thick and thin surfaces. We define a ​thick surface​ as an object where the surface represents a closed watertight interface with a solid interior made of the same material. A typical example is a solid glass object. A ​thin surface​ on the other hand is defined as an object which doesn’t have any thickness or volume, such as a tree leaf or a sheet of paper. For a thick surface there is no backside visible if the material is opaque. If a backside is hit by accident in this case, the shader should return black to avoid unnecessary computations and possible light leakage. In the case of a transparent thick surface, a backside hit should be treated as light exiting the closed interface. For a thin surface both the front and back side are visible and it can have different materials on each side. If the material is transparent in this case the thin wall makes the light transmit without refracting, like a glass window or bubble. Two nodes in our material model are used to construct surface shaders for these cases. The node constructs thick surfaces and is the main node to use since most objects around us have thick surfaces. The <thin_surface> node may be used to construct thin surfaces, and here a different BSDF and EDF may be set on each side.

There is lee-way for a pessimistic interpretation here. One may have a different BSDF and EDF set on each side, and also it says "it can have different materials on each side". In order to achieve different materials in the general case, the geometry would have to be implicitly promoted, or the spec could say "it can have different materials on each side, if the geometry is double sided", to close the loophole. :)

meshula avatar Mar 29 '22 17:03 meshula

I like "it can have different materials on each side, if the geometry is double sided" but pedantically would suggest going further, avoiding use of "material" altogether, since Material is a) the thing to which a primitive can bind, b) a container hosting (weak) primvars, c) an encapsulation point. Which is not what we're suggesting, here, so I'd suggest sticking with something like "it can have different a BSDF/EDF for each side, if the geometry is double sided"

spiffmon avatar Mar 29 '22 18:03 spiffmon

I sense we may have been talking about different kinds of sidedness above, so first I want to clarify that the material sidedness we want to describe in MaterialX was never intended to alter or take precedence over geometric sidedness. Geometry set to single-sided, invisible from the backside, would always be invisible even if a two-sided material is assigned to it. So a renderer would not need to parse any shading networks to find out if a surface is visible or not from the backside.

If we make the change and remove the explicit thin_walled parameter from the proposal (which I think is the right move the more I think about it), then I will rewrite the whole section about thin and thick surfaces. The specification would no longer dictate what a thin vs thick surface is, and instead each shading model is free to implement this as needed in a BSDF graph. I will also clarify that a two-sided material is only meaningful on double-sided geometry.

niklasharrysson avatar Mar 30 '22 11:03 niklasharrysson

If there is no thin_walled parameter, how would the renderer decide whether to apply refraction or not, if, for example, a dielectric_bsdf with a ior = 1.5 is used? Would that mean that you will add a thin_dielectric_bsdf?

proog128 avatar Mar 30 '22 11:03 proog128

@proog128 You could flip between two dielectric_bsdf's, using a mixer with the thin_walled parameter as mixing weight, so it's either one with refractions or the other without refractions. This is what I meant by having each shading model decide how to handle it in its BSDF graph.

Something like this would have to be done in the generated code anyway in order to implement the thin-walled property if we had it specified as part of the specification. So we're moving the implementation details from the code generator into the hands of the shading model BSDF graph creator. An advantage is that the graph creator can then also do custom handling and tweaks if needed for the specific shading model.

Please let us know of any concerns with this approach. Thanks!

niklasharrysson avatar Mar 30 '22 12:03 niklasharrysson

At the moment, thin_dielectric_bsdf and similar nodes are not available in the MaterialX PBRSpec. thin_dielectric_bsdf could be emulated, for example via layer(top = dielectric_bsdf(ior = 1.5, "R"), bottom = dielectric_bsdf(ior = 1.0, "T")), but that increases complexity in the BSDF graph and makes shading less efficient and complicates the implementation (if I am thinking about writing a native MaterialX implementation in a renderer). Therefore, I think adding thin versions of all potentially refractive BSDFs is required if there is no thin_walled flag.

One concern I have is that this doesn't map well to MDL and would be difficult to support in the MDL code generator. Somehow the code generator has to guess the value for MDL thin_walled from the BSDFs used in the graph. For instance, if it finds thin_dielectric_bsdf, it would set thin_walled to true. In this particular case it makes the code generator more complicated.

Another problem with removing an explicit thin_walled property (being it a thin_surface node or a thin_walled bool in surfacematerial) is that it creates ambiguity in the material definition. What if a graph contains thin_dielectric_bsdf and dielectric_bsdf, and both are active at the same time? Is it a thin or a volumetric material? Thinking again about the MDL code generator, should it set material.thin_walled to true or false?

Finally, the thin_walled property may serve as a hint whether the material has to be put onto a stack of nested dielectrics or not. Only if thin_walled = false the material has to be pushed onto the stack, because only then there is a volume below the surface. Alternatively, this information could be derived from the BSDF (i.e., dielectric_bsdf has a volume, thin_dielectric_bsdf not), but then we again end up with ambiguity: a graph could be, at the same time, a volume and a thin surface.

proog128 avatar Mar 30 '22 12:03 proog128

@proog128 , I thought it was redundant as part of the material definition. Question: is there a meaningful scenario in which we would name a back_surface bsdf but did not intend the material to be thin?

if not, then the render-prepper or code generator could infer thin_walled not from the presence of certain bsdfs, but by how they are used/combined, as specified by the OM.

spiffmon avatar Mar 30 '22 13:03 spiffmon

@spiffmon No, I don't think there is a meaningful back_surface bsdf for non-thin materials. If I understand your proposal correctly, you would leverage this fact to infer the thin_walled flag from the presence of a back_surface bsdf. Am I correct? If I am not missing anything, this would require the possibility to set a kind of "null_bsdf" node. This null_bsdf signals "has no back-face and is thin". Reason: In the glTF PBR node, for example, we would have to mix back face between same-as-front-face and null based on the node's thinWalled parameter: backface = mix(null_bsdf(), <same-graph-as-frontface>, thinWalled). Another (small) concern with this approach is that if a renderer/code generator wants to know, in case backface is set, whether backface equals frontface (maybe to enable optimizations etc.), then it has to compare two graphs.

proog128 avatar Mar 30 '22 15:03 proog128

@proog128 , my concern and questioning was for the user-level object model that we would expose in UsdShadeMaterial, and those concerns may indeed be different from those of the code generator. On UsdShadeMaterial, there will be a backSurface “relationship” that can target a shader node/prim representing a bsdf, but its default state is “empty”. So, at the UsdShade level it is trivial to know whether you have a back surface or not, no need to compare graphs.

But if you all decide that it makes for more efficient MaterialX codegen to separately encode thin-sidedness, then we can arrange to have HdMtlx reconstitute the bit based on presence of a backSurface network. I just wanted to make sure we wouldn’t be losing generality in removing from the user model what seemed like redundant information.

spiffmon avatar Mar 30 '22 17:03 spiffmon

This is a great discussion. But I think there are two parallell questions being discussed:

A) Should thin-walled be a feature added to this twosided surfacematerial node? B) How should the thin-walled feature be enabled on user-level on this surfacematerial node?

B is only relevant if the answer to A is Yes, so I think we should resolve A before discussing B.

For A my proposal above was that thin-walled could be kept out from the new surfacematerial node.

  • From a data model point of view it's redundant because it's really just a short-cut override for users to disable refraction and absorption on upstream BSDF's. This can be set on the BSDF level directly instead.

  • From an implementation point of view if we do add this override feature it means every MaterialX back-end and renderer will need to build support for it. @proog128 you are right that MDL has the thin_walled switch build into its data model, so it's easy to support there. But I'm afraid it's a lot harder everywhere else. Not only in our code generators but also for renderers that doesn't use code generation and have their own shading systems.

  • Instead it can be implemented as logic directly in the BSDF graph, for the shading models that requires thin-walled, as suggested above. Then it would automatically be supported by all since it's just a normal MaterialX BSDF graph.

  • Note that with this proposal the new twosided surfacematerial node would be trivial to support, since it's just a backfacing switch between two surfaceshaders.

Perhaps I'm missing something about the thin-walled feature that is not expressible the way I'm imagining? But let's at least discuss question A first, before we get to B :)

niklasharrysson avatar Mar 30 '22 22:03 niklasharrysson

@niklasharrysson Solving the questions in this order makes total sense, thanks for structuring the discussion :-).

Instead it can be implemented as logic directly in the BSDF graph, for the shading models that requires thin-walled, as suggested above. Then it would automatically be supported by all since it's just a normal MaterialX BSDF graph.

Do you mean that the flag should become a per-BSDF switch (dielectric_bsdf has a new parameter "thinWalled") or a per-surface switch (surface constructor has a new parameter "thinWalled")? I think my concerns only apply if thinWalled becomes a per-BSDF switch.

proog128 avatar Mar 31 '22 08:03 proog128

I mean the flag could be added only to the nodedef's of shading models that requires this, and then used as needed in the corresponding BSDF nodegraphs.

There is already an example of this in the Autodesk Standard Surface shading model:

The nodedef for standard_surface has a thin_walled flag: https://github.com/AcademySoftwareFoundation/MaterialX/blob/main/libraries/bxdf/standard_surface.mtlx#L93

This flag is used in the corresponding implementation nodegraph. First converted to float: https://github.com/AcademySoftwareFoundation/MaterialX/blob/main/libraries/bxdf/standard_surface.mtlx#L221

..and then used to select a different SSS model if thin-walled is enabled: https://github.com/AcademySoftwareFoundation/MaterialX/blob/main/libraries/bxdf/standard_surface.mtlx#L223

What is missing in this graph currently is to also select between two different models for transmission (with/without refraction, absorption, scattering) depending on this flag, but this is something we should fix to make the standard_surface MaterialX implementation more complete.

I think you should be able to do the same thing with the gltf_pbr shading model. If the thickness input is > 0.0 you can select a different model for transmission in the nodegraph.

niklasharrysson avatar Mar 31 '22 10:03 niklasharrysson

this is something we should fix to make the standard_surface MaterialX implementation more complete.

Which BSDF do you want to use in Standard Surface to disable refraction? This is the part that troubles me. I cannot think of a good way to express this with the current set of BSDFs for glTF PBR. Of course to disable refraction I could set the IOR to 1 (or 0, because of special case), but then I am describing one interface between two volumes with an IOR of 1. However, what I actually want to describe is two interfaces between three volumes: the volume on the one side, the (infinitesimally) thin volume described by the current node graph in the middle, and the volume on the other side.

Maybe I am overcomplicating, maybe I am misunderstanding the relationship between surfaces and volumes in MaterialX. As far as I understand, IORs in MaterialX are relative, i.e., they describe the relative IOR at an interface, as opposed to IORs in MDL, which are absolute, requiring the renderer to track the nested volumes (touching the problem of nested dielectrics). But besides that, I had always understood the difference between surface and thin_surface to be that the former is one interface and the latter is two interfaces. It would be great if the MaterialX PBR node spec would include a few more details on this. I am struggling to understand how to set up the IORs and volume parameters such that nesting of volumes works across all code generation targets. Is the MDL code generation in combination with Iray a good reference? Which code generation/renderer gives the correct solution?

proog128 avatar Mar 31 '22 12:03 proog128

Yes for the transmission part I would use a dielectric_bsdf with ior = 1.0. But for the reflection part we would still have the user-controlled ior.

BSDF thin_walled_dielectric = layer(top = dielectric_bsdf(ior = 1.5, "R"), base = dielectric_bsdf(ior = 1.0, "T"))

So it may be a hack but the whole concept of thin-walled is a hack., and I think this would be good enough. But I could definitely be wrong :)

What I like about this approach is that it uses constructs that are already available in MaterialX, so there is no update at all needed to the specification or to all renderers and back-ends.

If I where to implement thin_walled_dielectric in vanilla OSL code this is the approach I would have to take anyway. But even if thin_walled_dielectric was implemented as an atomic BSDF in C++, would it handle this very differently?

Maybe I am overcomplicating, maybe I am misunderstanding the relationship between surfaces and volumes in MaterialX.

Not at all, the description of volumetrics and things like nested dielectrics is not defined well enough in MaterialX yet. I know we've had conversations about this befor in other threads. That part needs to be worked on. And certainty from a reference implementation point of view. I'm afraid there have been no updates on that since we last spoke about it. However that is a much larger topic, and with thin-walled there is no volume, so my hope was that we don't need that here.

niklasharrysson avatar Mar 31 '22 22:03 niklasharrysson

Yes for the transmission part I would use a dielectric_bsdf with ior = 1.0.

This approach unfortunately disables transmission roughness, at least in the MDL exporter. I didn't try OSL, but I guess it's the same.

But even if thin_walled_dielectric was implemented as an atomic BSDF in C++, would it handle this very differently?

I think there are a couple of improvements that make the thin-walled dielectric BSDF better match an explictly modeled object. Some of them are available in e.g. glTF PBR, iray/MDL or Mitsuba.

In "T" and "RT" mode:

  • If we write a new BSDF in C++ for a path tracer that tracks nested volumes, we are not allowed to update the nested IOR stack in case of thin-walled. Since we don't enter a volume, there is nothing to do.
  • The OSL dielectric_bsdf with ior = 1.0 doesn't have roughness (if I am not mistaken). This is fine, and ideally a thin dielectric BSDF with an IOR of 1 shows the same behavior. But not a thin dielectric BSDF with an IOR of 1.5. When we assume that the orientations of the microfacets on both sides of the thin volume are independent/not correlated, rays are refracted and the transmission becomes blurry. This is what we expect in the thin-walled glTF PBR material. Unfortunately, with base = dielectric_bsdf(ior = 1.0, "T") we would not have the blurriness.

In "RT" mode only:

  • The thin dielectric BSDF could take internal reflections (the bounces between the two surfaces) into account.

So it may be a hack but the whole concept of thin-walled is a hack., and I think this would be good enough. But I could definitely be wrong :)

In this particular case (glTF PBR), I am mostly worried about the wrong impression we give material authors regarding the impact of roughness on thin-walled objects. It might be true that we cannot get this in OSL-based renderers at the moment, but we certainly can do it in MDL.

In general, I think it would be great for a renderer-independent material standard to store as much semantic information as possible, in order to best capture the original intent of the material author. Or at least as much semantic information as can be represented by the targeted shading languages. Code generation could then "downsample" to the best possible representation of the target language, with as little loss as possible.

proog128 avatar Apr 04 '22 14:04 proog128

Thanks for the additional details @proog128, these are some great points.

To my mind they are strong enough arguments that we may consider A to be resolved, with the conclusion that we do need explicit support for thin-walled in the specification. If others agree I think we can move on to look at B, how to define this in the spec.

There is a practical issue with the initial proposal I made above where the surfacematerial node has the thin_walled switch. The problem is that our "ubershader" nodes, where we want to have control over this (standard_surface, gltf_pbr, etc.), are all nodes of type surfaceshader and not type material. So there is no practical way to wire this in their implementation graph that outputs a surfaceshader, when the input to control it is on the material level outside the graph.

One option would be to change the ubershaders to output a material type instead, but then displacement becomes part of the ubershaders as well (displacement is a material property), so it doesn't match their original definition.

Another solution is to separate thin_walled from the front/back assignments, keeping only assignments on material level and having thin_walled specified on surfaceshader level. This gives a better separation of concerns.

Here's a new proposal to discuss with these changes made:

 *** REMOVED NODE ***
 <!--
   Node: <thin_surface>
   Construct a surface shader from scattering and emission distribution functions for non-closed "thin" objects.
 -->

 *** NEW NODE ***
 <!--
   Node: <surfacematerial_twosided>
   Constructs a material with different surfaceshaders on each side of geometry,
   if the geometry is double sided. If only one of the sides are connected this
   surfaceshader is used on both sides.
 -->
 <nodedef name="ND_surfacematerial_twosided" node="surfacematerial_twosided" nodegroup="material">
   <input name="frontsurfaceshader" type="surfaceshader" value="" />
   <input name="backsurfaceshader" type="surfaceshader" value="" />
   <input name="displacementshader" type="displacementshader" value="" />
   <output name="out" type="material" />
 </nodedef>

 *** UPDATED NODE ***
 <!--
   Node: <surface>
   Construct a surface shader from scattering and emission distribution functions.
 -->
 <nodedef name="ND_surface" node="surface" nodegroup="pbr">
   <input name="bsdf" type="BSDF" value="" />
   <input name="edf" type="EDF" value="" />
   <input name="opacity" type="float" value="1.0" />
   <input name="thin_walled" type="boolean" value="false" />
   <output name="out" type="surfaceshader" />
 </nodedef>

There is a possibility to have two surfaceshaders, one thin and one thick, be mixed or assigned on front vs back on a material, which doesn't make sense. But this we could resolve by letting thin take precedence and override the other in such cases.

niklasharrysson avatar Apr 05 '22 21:04 niklasharrysson

To my mind they are strong enough arguments that we may consider A to be resolved, with the conclusion that we do need explicit support for thin-walled in the specification.

That's great, thank you @niklasharrysson.

Here's a new proposal to discuss with these changes made:

The proposal looks good to me, it will definitely work perfectly with the glTF PBR and its thin_walled toggle.

proog128 avatar Apr 09 '22 08:04 proog128

Glad to hear that @proog128.

@spiffmon, @meshula just let us know if there are any concerns from USD and Pixar side.

Thanks for all feedback!

niklasharrysson avatar Apr 09 '22 08:04 niklasharrysson

This new proposal satisfies my hope to make front-shader/back-shader available on a special node rather than an option that can be introduced anywhere in a graph, and it also makes the use of thin-surface explicit in terms of describing the distribution functions. So it addresses the concerns I raised :)

meshula avatar Apr 09 '22 18:04 meshula

Excellent! Thanks @meshula

niklasharrysson avatar Apr 10 '22 09:04 niklasharrysson

I think I follow why it is important for codegen and the underlying math to have an explicit declaration of thin_walled behavior. But I am unsure if it is advantageous to propagate that up into the user-model, vs. inferring it from the presence of a backsurface shader terminal connection. In UsdShade there is only a single Material definition/schema, and following the proposal here to the letter would mean introducing a second TwoSidedMaterial schema, which, if we wanted the specificity of front vs back, would not even share an inheritance relationship with Material, which already defines a simple surface terminal. That's kind of a big deal.

I will discuss with Pixar internal stakeholders, but I'm very curious to hear from Autodesk, Substance, and other toolmakers whether the idea of exposing a different material type to users is clarifying or cluttering. For sure, you could key off of it to do GUI validation on the shaders you connect to it, to ensure they are thin-walled, and so don't get into the inconsistent state Niklas mentioned, but I think you could also enable such validation when the backsurface terminal becomes connected, also?

spiffmon avatar Apr 11 '22 00:04 spiffmon

Thanks @spiffmon for the additional feedback. I think there are two questions here that need further discussion:

  1. Could thin-walled be inferred from the presence of a backsurface connection on material level?
  2. Is there a need for a separate material node for two-sided materials?

For (1), first of all I think it's a good separation of concerns if we keep the distinction between material assignment (i.e. material nodes) and surface appearance (i.e. surfaceshader nodes). Since thin-walled is a surface appearance feature I think it belongs as a switch in the surfaceshader domain.

But there is also the practical aspect that some shading models have thin-walled as a user controlled switch in their parameterization (e.g. standard_surface and gltf_pbr). We can't encapsulate these shading models as atomic surface shader nodes, with a thin-walled input in their parameterization, if thin-walled is solely inferred from how the shader is assigned on the material node downstream.

However, we could enforce that having both front-surface and back-surface assigned would result in the shaders getting thin-walled enabled, no matter what the user has set, to avoid the inconsistent state you mention.

For (2) I agree it doesn't make sense to introduce a second TwoSidedMaterial schema in USD. Having this as a separate node in MaterialX was proposed in response to @meshula's concern that introducing two-sidedness may be problematic for renderers if not done through an easily identifiable new node. I wonder though if that concern was not mainly about geometry sidedness? Now that we have clarified that this is strictly about material sidedness which doesn't affect geometry sidedness, is this still a concern? Or would it be possible to just add backsurface to the existing material node and avoid the secondary two-sided specialization?

niklasharrysson avatar Apr 11 '22 15:04 niklasharrysson

My concern was that due to existing wording, introducing a two-sided material could be interpreted as implicitly "upgrading" geometry to two-sided - in other words it was not explicitly clear whether a single sided mesh must be rendered double sided if a "back" material was introduced. I proposed the separate node to accommodate an "upgrade" by making it blazingly obvious in the graph. If there were to be a need for the "upgrade" to propagate to other mesh nodes that must be rendered jointly, then it would force explicitly "upgrading" modes, rather than introducing a heuristic rule about what must or must not be rendered doublesided.

If the rule is that materials cannot affect geometry, then my concern is alleviated.

meshula avatar Apr 11 '22 17:04 meshula

That's great to hear! To my mind it would make sense then to keep a single surfacematerial node. I will rewrite the section in the specification that deals with this so there is no risk of confusion what two-sided means in this context.

niklasharrysson avatar Apr 12 '22 13:04 niklasharrysson