Sdf permitting unnamed variants creates inconsistencies between Sdf and Usd

[nvmkuruc]

Description of Issue

Sdf (seemingly erroneously) permits unnamed variants, but Usd does not. This creates two situations.

• layer.GetObjectAtPath('/prim{variantSet=}') should always return a variant set spec but it erroneously returns the unnamed variant spec when specified.
• USD silently fails to apply any edits specified in a variant set with an unnamed variant even when a valid variant is selected.

Sdf should always return the variant set spec and should warn or error if it considers the variant set invalid due to an unnamed variant.

Steps to Reproduce

1. Consider the following snippet of usda.

#usda 1.0

def "prim" (append variantSets = "color"
            variants = { string color = "red"}
           ){
   variantSet "color" = {
       "yellow" {
          int i = 0
       }
       "red" {
          int i = 7
       }
   }
}

2. Run the following python snippet

>>> from pxr import Sdf
>>> layer = Sdf.Layer.FindOrOpen('test.usda')
>>> type(layer.GetObjectAtPath('/prim{color=}'))
<class 'pxr.Sdf.VariantSetSpec'>

/prim{color=} refers to the variant set spec.

3. Replace "yellow" with the empty string.

#usda 1.0

def "prim" (append variantSets = "color"
                  variants = {string color = "red"}){
   variantSet "color" = {
       "" {
          int i = 0
       }
       "red" {
          int i = 7
       }
   }
}

4. Run the following python snippet. It now returns a VariantSetSpec.

>>> from pxr import Sdf
>>> layer = Sdf.Layer.FindOrOpen('test.usda')
>>> type(layer.GetObjectAtPath('/prim{color=}'))
<class 'pxr.Sdf.VariantSpec'>

5. Run usdcat on the layer. You should get this output, where the variant set containing the unnamed variant has been completely removed without warning or error.

#usda 1.0

def "prim" (
    variants = {
        string color = "red"
    }
    append variantSets = "color"
)
{
}

System Information (OS, Hardware)

Linux

Package Versions

Build Flags

[jesschimein]

Filed as internal issue #USD-9486

[nvmkuruc]

We suspect the issue is the VariantName grammar in pathParser.h which is used to validate variant names should be using PEGTL_NS::plus instead of PEGTL_NS::star to reject empty variant names.

[spiffmon]

Thanks, @nvmkuruc - we think the problem is actually the usda parser: it should reject your asset with the unnamed variant in it - it's only once an unnamed variant gets created that things go to pot (so we should reject in the creation of VariantSpec's as well). pathParser needs to accept empty variant names so that we can construct paths that identify the VariantSet.