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

Survey of type conversions #1895

Open
ld-kerley opened this issue Jun 20, 2024 · 3 comments
Open

Survey of type conversions #1895

ld-kerley opened this issue Jun 20, 2024 · 3 comments

Comments

@ld-kerley
Copy link
Contributor

A few times rencently I wanted to convert a vector2 to a color3 to debug some texture coordinates. I found that there is no convert node to do this conversion. That got me curious as to which type conversions are currently supported in MaterialX, so I decided to do a small survey.

from\to float color3 color4 vector2 vector3 vector 4 integer boolean
float (no-op) convert convert convert convert convert floor/ceil/round ifgreater/eq
color3 extract (no-op) convert MISSING (1) convert MISSING (1) ???? ????
color4 extract convert (no-op) MISSING (1) MISSING (1) convert ???? ????
vector2 extract MISSING (1) MISSING (1) (no-op) convert MISSING (1) ???? ????
vector3 extract convert MISSING (1) convert (no-op) convert ???? ????
vector4 extract MISSING (1) convert MISSING (1) convert (no-op) ???? ????
integer convert MISSING (2) MISSING (2) MISSING (2) MISSING (2) MISSING (2) (no-op) MISSING (3)
boolean convert MISSING (2) MISSING (2) MISSING (2) MISSING (2) MISSING (2) MISSING (3) (no-op)

The column down the side is the "from" type and the top row indicates the "to" type.

Observations:

  • If "from" and "to" are the same, then obviously this is a no-op, or perhaps a <constant> or <dot> node.
  • All of the entries for any of the vector/color types that are tagged as MISSING (1) can be constructed as combinations of multiple convert nodes.
  • Converting from integer or boolean to one of the vector/color types (Missing (2)), requires a <convert> node to convert to a float and then chaining one or more <convert> to get to the destination type.
  • integer and boolean types cannot easily be converted between (Missing (3)).
  • Converting from vector/color types to integer or boolean requires multiple user input, so these seem reasonable to leave as chains of nodes. (<extract> -> <floor/ceil/round>). Unless anyone else has any other suggestions?

Proposals:

  • Create <convert> nodes for each of the vector/color cases (MISSING (1)) - thus avoiding chaining multiple nodes.
  • Create <convert> nodes for each of <integer> and <boolean> to vector/color cases (MISSING (2)) - also avoiding chaining multiple nodes.
  • Create ND_convert_boolean_integer where true=1 and false=0.
  • Create ND_integer_boolean following the conventional C rules of any non-zero integer being true.

Potential concerns:

  • Do we nodes like convert to be "more complete" but at the expense of having a larger node library? I personally think that having gaps in a node like convert leads to confusion with users, especially those new to the system or less technically minded.
@dbsmythe
Copy link
Contributor

The reason that the "MISSING" conversions are missing is because I couldn't think of a single obviously correct operation that would be what the user would expect in the vast majority of situations. If you're converting between a color and a color, the channels have known semantics, so it's easy to come up with a sensible thing to do to add or remove channels. Similarly if you try to convert between a float and either a color or vector: there's a sensible interpretation that's usually what you'd want.
But converting between color and vector with 2, 3 or 4 channels, or between a vector2 and a vector4, is less obvious what would be most frequently desirable: it's easy to come up with multiple sensible operations. I think it's better to require shading networks to extract and combine exactly the channels the way they want, which will be both simple and precise.
Converting from an integer to other types is simply an omission, as it could be treated the same way as converting from a float, but converting from a color or vector or float to an integer is less clear: do you do a floor() or a round()? In that case, I think it's better to convert or extract the input to be a float, and then apply the desired operation to floor or round that to an integer.
Similar for boolean: converting from boolean to other types is probably easy (output a 0 or a 1; is there ever a situation you'd want a different output value for "true"?), but converting from color/vector/float to boolean, what's the dividing line between "false" and "true"? Is "0.5" true or false? How about 0.000000001?

@ld-kerley
Copy link
Contributor Author

@dbsmythe - I'll try and separate out my thoughts in to sections that so that hopefully the uncontentious ones can get easy agreement, and we can debate the harder ones. (roughly in order of contentiousness... :) )

  1. <convert> nodes from integer and boolean should be created to add parity with the from float case. Where we cast the integer to a float, and use true=1.0 and false=0.0 - these values seem reasonable and highly expected. If users want other values to map they could use <ifequal>.

  2. I agree that converting to a boolean or integer from any float based type is ambiguous, and users should be using other more specific nodes to perform those operations.

  3. I think that the most common use of vector2 is to store texture coordinates, and I think its not uncommon when creating materials that procedurally modify the texture coordinates, or even if you're getting geometry you're not sure has good UVs, to want to be able to visualize them as a color3. In that case I think it's pretty standard to want to map them as color3(x, y, 0). Which also follows the existing convention we have in ND_convert_vector2_vector3.

  4. All of the existing convert nodes follow the same pattern of either dropping the higher indexed components, or adding 0 for the first three components and a 1 for the fourth component. Having a consistent ruleset that is easy to comprehend and explain I think also allows for the addition of the following conversions:

4a) Staying within the constraint of respecting similar color/vector semantics ([r,g,b] or [x,y,z]), it seems reasonable to me to add <convert> nodes to convert between vector2 and vector4, following these existing rules. vector2(x,y) -> vector4(x,y,0,1) and vector4(x,y,z,w) -> vector2(x,y).

4b) If we concede the usefulness of vector2 -> color3 and cross the semantic boundary, and we also have a well established convention of adding 0 for the first three channels and 1 for the fourth, or just dropping channels, then can we extend these two ideas together to complete the rest of the missing vector/color conversions?

I understand your perspective around only supplying conversions that are completely unambiguous, and only have one clearly sensible conversion, I wonder if you could share cases where some of the MISSING conversions between color and vector types might have more than one practical use-case?

My major motivation with trying to complete as much of the table above as is possible is to try and lower the cognitive burden when creating these more exotic conversions. Having previously worked at a studio where artists are frequently creating graphs with 100's or 1000's of nodes in, finding ways to simplify and remove as many nodes as possible helps increase clarity of the graph.

It may also perhaps create slightly more efficient shaders. I'm guessing at the end of the day all the shader compilers would optimize away any unnecessary chained conversions resulting from multiple <convert> nodes chained together, but the shader gen would still be creating additional shader code, which still has to be compiled in the first place.

@dbsmythe
Copy link
Contributor

First off, I'm glad this discussion has come up- it's always good to reevaluate past decisions to see if they are still good or if they should be updated.
Going through your list in order:

  1. I think we're on the same page here, and it would be good to create convert variants FROM integer or boolean TO the other float-based types as noted. We could also create a convert from boolean to integer with no controversy.
  2. For converting FROM a float-based type TO integer or boolean, I think we're in agreement here too that it's ambiguous, though that would mean there would still be holes in the overall "convert" operation matrix, unless we take a stand and just pick an operation for that conversion (which one? I'd probably vote for "floor").
  3. You've convinced me that convert from vector2 to color3 should be (x, y, 0), sounds good. Based on the rules for converting a color3 to a color4, it would make sense that convert from vector2 to color4 should be (x, y, 0, 1).
  4. This is where things start to get trickier. If we do fill out the matrix, I would hope that it has the property that converting from type A to type C would always give the same results as converting from A to B and then from B to C, for any types A, B and C. I would have to look at the existing rules and try some permutations, but at first glance, the rule that seems most likely to always work like that would be what I think you're proposing in 4b, namely "From more channels to fewer channels: drop the extra channels; from fewer channels to more channels, use 0 for an added second or third channel, and 1 for an added fourth channel". This logic is at least pretty unambiguous and easy to predict, so maybe that's a good choice for "convert", and if a shading network needs to apply different logic it could use explicit extract/combine/etc.?

What do others think?

jstone-lucasfilm pushed a commit that referenced this issue Jun 27, 2024
Related to #1895

Fills out more of the "missing" convert nodes.

Notably this moves a lot of the implementations to nodegraphs, to reduce the maintenance going forward and to ensure all shader generation targets are aligned.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants