AcademySoftwareFoundation / MaterialX

MaterialX is an open standard for the exchange of rich material and look-development content across applications and renderers.
http://www.materialx.org/
Apache License 2.0
1.88k stars 357 forks source link

Cloverleaf placement appears incorrect #1841

Open kwokcb opened 6 months ago

kwokcb commented 6 months ago

Issue

If you create a cloverleave node and try and center it it will not be placed where expected. If you use the same placement inputs for circle or hexagon it does.

I'm unsure if there is any upgrade issue as channels is used in the implementation nodegraph or just is pre-existing ?

Test Data

Output on a 2d plane. BTW a center of 1,1 places it in the center for some reason ?

File used:

<?xml version="1.0"?>
<materialx version="1.39" colorspace="lin_rec709">
  <surfacematerial name="surfacematerial_material1" type="material">
    <input name="surfaceshader" type="surfaceshader" nodename="convert_surfaceshader1" />
    <input name="displacementshader" type="displacementshader" value="" />
  </surfacematerial>
  <tiledcloverleafs name="tiledcloverleafs_color4" type="color3">
    <input name="texcoord" type="vector2" value="0,0" />
    <input name="uvtiling" type="vector2" value="1,1" />
    <input name="uvoffset" type="vector2" value="0,0" />
    <input name="size" type="float" value="0.5" />
    <input name="staggered" type="boolean" value="false" />
  </tiledcloverleafs>
  <convert name="convert_surfaceshader1" type="surfaceshader">
    <input name="in" type="color3" nodename="tiledcloverleafs_color4" />
  </convert>
  <surfacematerial name="surfacematerial_material2" type="material">
    <input name="surfaceshader" type="surfaceshader" nodename="convert_surfaceshader2" />
    <input name="displacementshader" type="displacementshader" value="" />
  </surfacematerial>
  <cloverleaf name="cloverleaf_float1" type="float">
    <input name="texcoord" type="vector2" value="0,0" />
    <input name="center" type="vector2" value="0.5,0.5" />
    <input name="radius" type="float" value="0.25" />
  </cloverleaf>
  <convert name="convert_surfaceshader2" type="surfaceshader">
    <input name="in" type="float" nodename="cloverleaf_float1" />
  </convert>
  <circle name="circle_float1" type="float">
    <input name="texcoord" type="vector2" value="0,0" />
    <input name="center" type="vector2" value="0.5,0.5" />
    <input name="radius" type="float" value="0.25" />
  </circle>
  <convert name="convert_surfaceshader3" type="surfaceshader">
    <input name="in" type="float" nodename="circle_float1" />
  </convert>
  <surfacematerial name="surfacematerial_material3" type="material">
    <input name="surfaceshader" type="surfaceshader" nodename="convert_surfaceshader3" />
    <input name="displacementshader" type="displacementshader" value="" />
  </surfacematerial>
  <surfacematerial name="surfacematerial_material4" type="material">
    <input name="surfaceshader" type="surfaceshader" nodename="convert_surfaceshader4" />
    <input name="displacementshader" type="displacementshader" value="" />
  </surfacematerial>
  <convert name="convert_surfaceshader4" type="surfaceshader">
    <input name="in" type="float" nodename="hexagon_float1" />
  </convert>
  <hexagon name="hexagon_float1" type="float">
    <input name="texcoord" type="vector2" value="0,0" />
    <input name="center" type="vector2" value="0.5,0.5" />
    <input name="radius" type="float" value="0.25" />
  </hexagon>
</materialx>
kwokcb commented 6 months ago

Leaving a ping for @ashwinbhat as I believe you and Roberto worked on these ?

dbsmythe commented 6 months ago

I had similar observations about several of the Shape nodes, wondering if they should be updated to be more consistent, predictable and complete:

kwokcb commented 6 months ago

Yes.

but it's a lot of work to offset the line to handle the radius. Also can't get rid of the circles ?

dbsmythe commented 6 months ago

<Line> actually makes sense to me: the point1 and point2 positions are the centers of the endpoints of the line, and radius is the half-thickness of the line, so all pixels that are within "radius" pixels of the (infinitesimally-thin) line between point1 and point2 are "on". The only addition I would suggest is like I mentioned at the TSC meeting, to have a "color3" variant of <line> as well as float, which would have a pair of color inputs to define the "in the line" and "not in the line" colors. And same for all other Shape nodes: have both float-output and a color3-output variants of all of them rather than "some output float, some output color3". Though maybe better choices for the two color inputs would be "colorfg" and "colorbg" rather than color1/color2 so it's more clear which color input drives the color of the pattern and which is the non-pattern background color.

kwokcb commented 6 months ago
zicher3d commented 3 months ago

The circles on the line ends is because we are using a signed distance field formula. If we use rectangles we will have issues connecting lines together. I guess we could have option or another line node for rectangle ends. I haven't thought about that.

zicher3d commented 3 months ago

Regarding circles, hexagon, or cloverleaf position. The initial position is consitent but yes, looks like the offset for cloverleaf needs to be multiplied by 2. That should be fixed. But the tiled version works ok, maybe there it's compensated somehow. if we fix one we need to probably "unfix" the other.