PixarAnimationStudios / OpenUSD-proposals

Share and collaborate on proposals for the advancement of USD
92 stars 25 forks source link

Autodesk: LineStyle Proposal - CurveStyle Removal #47

Closed erikaharrison-adsk closed 1 month ago

erikaharrison-adsk commented 1 month ago

Description of Proposal

Based on feedback, the CurveStyle schema is removed. The BasisCurves with line style will bind a Material instead of a CurveStyle. The properties of the dash dot style could be set via the input of the material surface shader. For example, this is the schema before:

def DashDotStyle "LineMat1"
{
    token outputs:surface.connect = </LineShader.outputs:surface>
    uniform token startCapType = "round"
    uniform token endCapType = "round"
    uniform float patternScale = 5
}

def Shader "LineShader"
{
    uniform token info:id = "DashDotSurface"
    color4f inputs:diffuseColor.connect = </diffuseTexture.outputs:rgba>
    token outputs:surface
}

def Shader "diffuseTexture"
{
    uniform token info:id = "DashDotTexture"
    asset inputs:file = @DashedX2.png@
    float4 outputs:rgba
}

And this is the schema after the change:

def Material "LineMat1"
{
    token outputs:surface.connect = </LineShader.outputs:surface>
}

def Shader "LineShader"
{
    uniform token info:id = "DashDotSurface"
    color4f inputs:diffuseColor.connect = </diffuseTexture.outputs:rgba>
    int startCapType = 0  // 0 is for "round" cap type, 1 is for "square" and 2 is for "triangle"
    int endCapType = 0
    float patternScale = 5
    token outputs:surface
}

def Shader "diffuseTexture"
{
    uniform token info:id = "DashDotTexture"
    asset inputs:file = @DashedX2.png@
    float4 outputs:rgba
}

And we will add a "style" property to the BasisCurves, to indicate that the curve's material is a normal material or a dashdot pattern.

Supporting Materials

Previous PRs:

Contributing

erikaharrison-adsk commented 1 month ago

Note: I'm not quite sure why the dsyu-pixar Update Readme.md commit has come through via this PR.

dsyu-pixar commented 1 month ago

Note: I'm not quite sure why the dsyu-pixar Update Readme.md commit has come through via this PR.

It's not clear to me either, but that commit was already merged -- maybe rebase & squash?

nvmkuruc commented 1 month ago

One thing that I'd recommend considering is the implication on ComputeExtents. ComputeExtents is currently defined in UsdGeomCurves and pads the extent of the points with the maximum value of the underlying widths. It seems like a curve with widths set to 1.0 in a metersPerUnit = 1.0 stage would get its points padded with a meter.

I'm wondering about added complexity the additional style property adds to validation. It seems like there are a lot of properties which need to be coordinated when style is used. There are restrictions to material:binding, widths (and its interpolation), and type (only linear).

As an alternative approach, could a typed schema more targeted towards screen space lines be developed? In our visualization schemas proposal, we capture some of the history of BasisCurves and how the schema was made stronger by disaggregating the hermite basis into its own schema (HermiteCurves). The BasisCurves formulation was constraining the natural separation of point and tangent vectors of hermite curves and few imaging pipelines (if any) accurately rendered the hermite basis.

PierreWang commented 1 month ago

One thing that I'd recommend considering is the implication on ComputeExtents. ComputeExtents is currently defined in UsdGeomCurves and pads the extent of the points with the maximum value of the underlying widths. It seems like a curve with widths set to 1.0 in a metersPerUnit = 1.0 stage would get its points padded with a meter.

I'm wondering about added complexity the additional style property adds to validation. It seems like there are a lot of properties which need to be coordinated when style is used. There are restrictions to material:binding, widths (and its interpolation), and type (only linear).

As an alternative approach, could a typed schema more targeted towards screen space lines be developed? In our visualization schemas proposal, we capture some of the history of BasisCurves and how the schema was made stronger by disaggregating the hermite basis into its own schema (HermiteCurves). The BasisCurves formulation was constraining the natural separation of point and tangent vectors of hermite curves and few imaging pipelines (if any) accurately rendered the hermite basis.

Thank you. Yes it maybe a good idea to have a separate schema such as StyledCurves, and one reason is that it will use a totally different vertex and fragment shader. I know that Nvidia also had a draft design for a separate type of drawables. The vprims design was for nondiegetic drawables , and which was related with the LineStyle and Text design. As you know with this design the StyledCurve will be automatically a separate schema. I am very interested in the design and I hope to have a place to discuss it.

PierreWang commented 1 month ago

One thing that I'd recommend considering is the implication on ComputeExtents. ComputeExtents is currently defined in UsdGeomCurves and pads the extent of the points with the maximum value of the underlying widths. It seems like a curve with widths set to 1.0 in a metersPerUnit = 1.0 stage would get its points padded with a meter.

I'm wondering about added complexity the additional style property adds to validation. It seems like there are a lot of properties which need to be coordinated when style is used. There are restrictions to material:binding, widths (and its interpolation), and type (only linear).

As an alternative approach, could a typed schema more targeted towards screen space lines be developed? In our visualization schemas proposal, we capture some of the history of BasisCurves and how the schema was made stronger by disaggregating the hermite basis into its own schema (HermiteCurves). The BasisCurves formulation was constraining the natural separation of point and tangent vectors of hermite curves and few imaging pipelines (if any) accurately rendered the hermite basis.

Next I will create a separate curve schema for the styled polyline/line segments. The name may be "DashDotLines". And I will add a section about the ComputeExtents. I will update the proposal soon.

meshula commented 1 month ago

@dsyu-pixar I did a rebase/merge to address the issue noted above.

@PierreWang Merging based on your note that more work on extents is incoming.