PixarAnimationStudios / OpenUSD-proposals

Share and collaborate on proposals for the advancement of USD
106 stars 27 forks source link

Autodesk: Update the LineStyle proposal from suggestions #60

Open erikaharrison-adsk opened 7 months ago

erikaharrison-adsk commented 7 months ago

Description of Proposal

It is preferred to use a separate schema for the DashDotLines, because the dash dot patterns can only be applied for the lines. If we directly add style property to the BasisCurves, the style property will be only valid when the type is linear. This constraint is not convenient. We also add a separate rprim for the new primitive. In this proposal, I add a separate section talking about the extents of the DashDotLines, and a section talking about how to handle screen space pattern. At the end of the proposal, two examples are provided.

Link to Rendered Proposal

Supporting Materials

Previous PRs:

Contributing

PierreWang commented 7 months ago

The bigger question in my mind is still if this schema is needed - I totally understand the utility of it, but it's also yet another schema to support everywhere, in addition to the underlying existing curves...

As @nvmkuruc pointed, the dash-dot patterned lines will interpret the width as screen-space width. This is different from the definition of a common Curve. To avoid schema slicing, it is better to use a separate typed schema.

PierreWang commented 5 months ago

In the new change, we create a new schema DashDotPattern. The pattern can be set via an array of float2. So we don't need to use a texture to provide the pattern. And we don't require that the DashDotLines primitive must binds a material. Instead, if the DashDotLines primitive has a pattern, it should bind a DashDotPattern.

PierreWang commented 5 months ago

Hi @spiffmon , I have updated the proposal. The properties of DashDotPattern is now in DashDotPatternAPI. I don't know if my sample is right:

def DashDotLines "StyledPolyline" (
    inherits = [</Pattern>]
){
    uniform bool screenspacePattern = true
    uniform token startCapType = "triangle"
    uniform token endCapType = "triangle"
    uniform float patternScale = 11
    int[] curveVertexCounts = [3, 4]
    point3f[] points = [(0, 0, 0), (10, 10, 0), (10, 20, 0), (0, 30, 0), (-10, 40, 0), (-10, 50, 0), (0, 60, 0)]
    float[] widths = [10] (interpolation = "constant")
    color3f[] primvars:displayColor = [(0, 0, 1)]
}
def "Pattern" (
    prepend apiSchemas = ["DashDotPatternAPI"]
)
{
    uniform float patternPeriod = 10
    uniform float2[] pattern    = [(0, 5), (2.5, 0)]
}

In practice, I found that I could not read the patternPeriod and pattern property from the "StyledPolyline" prim in the adapter of DashDotLines. I think as I used inheritance, the properties should be available in the prim.

spiffmon commented 5 months ago

Yes, inherits should make the API schema be present on </StyledPolyline>, and you should be able to confirm that in usdview or any python shell. I don't really know what might be the problem in your Hydra adapter, though I know that in Hydra 2 the general pattern would be to have a SI specifically for DashDotPatternAPI... others here might be able to help more, if you can provide a bit more context on the code?

PierreWang commented 5 months ago

Yes, inherits should make the API schema be present on </StyledPolyline>, and you should be able to confirm that in usdview or any python shell. I don't really know what might be the problem in your Hydra adapter, though I know that in Hydra 2 the general pattern would be to have a SI specifically for DashDotPatternAPI... others here might be able to help more, if you can provide a bit more context on the code?

Thank you. I made a further check today and found a bug in my code. There is no problem now.

PierreWang commented 4 months ago

The implementation of this proposal is here: https://github.com/PixarAnimationStudios/OpenUSD/pull/3151