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

Composition: Inheritance management #209

Open
rdeioris opened this issue Nov 4, 2024 · 3 comments
Open

Composition: Inheritance management #209

rdeioris opened this issue Nov 4, 2024 · 3 comments
Labels
bug Something isn't working enhancement New feature or request

Comments

@rdeioris
Copy link
Contributor

rdeioris commented Nov 4, 2024

Hi, the current Inheritance composition arc implementation is mostly correct. to sum up inheritance from spec:

A PrimSpec gets its props and children from another PrimSpec with the specifier "class" (this is not strictly required, but very common as classes do not appear in the final Stage). Properties and children of the inheriting PrimSpec have higher priority.

Note: the class must not be under the inheriting PrimSpec (as specified in https://remedy-entertainment.github.io/USDBook/terminology/inherits.html#inherits)

Prims cannot inherit from an ancestor or descendant, inherit “bases” should be defined as siblings or outside of the target prim’s hierarchy, for example at the root level

Giving the following usda:

#usda 1.0

class Mesh "Base" {
    def Cube "Box" 
    {
        string value = "Test"
    }
}

def "Root" {
    def "Test" ( inherits = </Base>)
    {
        over "Box"
        {
            string value = "Test2"
        }
    }
}

def "Root2" {
    def "Test" ( inherits = </Base/Box>)
    {

    }
}

def "Root3" {
    def "Test" ( inherits = </Base>)
    {
        string value = "Test2"
    }
}

OpenUSD usdcat returns:

#usda 1.0
(
    doc = """Generated from Composed Stage of root layer /mnt/e/Documents/Unreal Projects/GltfUsdPlayground/Assets/classes.usda
"""
)

class Mesh "Base"
{
    def Cube "Box"
    {
        string value = "Test"
    }
}

def "Root"
{
    def Mesh "Test"
    {
        def Cube "Box"
        {
            string value = "Test2"
        }
    }
}

def "Root2"
{
    def Cube "Test"
    {
        string value = "Test"
    }
}

def "Root3"
{
    def Mesh "Test"
    {
        string value = "Test2"

        def Cube "Box"
        {
            string value = "Test"
        }
    }
}

While tusdcat fails as there is a bug in the caching implementation (Root3 will fail as the value is assumed to be in cache, fix will be included in the pull request).

Once the caching is fixed the output is:

#usda 1.0

class Mesh "Base"
{
    def Cube "Box"
    {
        string value = "Test"
    }
}

def "Root2"
{
    def "Test"
    {
        string value = "Test"
    }
}

def "Root"
{
    def "Test"
    {
        def Cube "Box"
        {
            string value = "Test2"
        }
    }
}

def "Root3"
{
    def "Test"
    {
        string value = "Test2"
        def Cube "Box"
        {
            string value = "Test"
        }
    }
}

Almost correct, the only missing piece is the typeName inheritance (fix in pull request).

Finally:

#usda 1.0

def "Root" {
    def "Test" ( inherits = </Root/Test/Base>)
    {
        class Mesh "Base" {
            def Cube "Box" 
            {
                string value = "Test"
            }
        }
    }

}

This is an invalid config, usdcat returns:

In </Root/Test>: Cycle detected:
@XXX@</Root/Test>
CANNOT inherit from:
@XXX@</Root/Test/Base>
 (instantiating stage on stage @XXX@ <0x5648ae149360>)

while tusdcat processes it (swallowing the class block)

#usda 1.0

def "Root"
{
    def "Test"
    {
        def Cube "Box"
        {
            string value = "Test"
        }
    }
}

I think tusdcat should fail too here, thoughts?

@syoyo
Copy link
Collaborator

syoyo commented Nov 4, 2024

Thanks! Let me give some time to double-check the logic of Inheritance and its implementation in TinyUSDZ.

@syoyo
Copy link
Collaborator

syoyo commented Nov 4, 2024

For 'cycle detected' error case, this rule is applied:

Prims cannot inherit from an ancestor or descendant

'Base' Prim is the child of 'Test' Prim, so TinyUSDZ also should fail(and report an error)

I will summerize the logic of Inherit op and put it to wiki. Then do some tests, then merge the PR.

@syoyo
Copy link
Collaborator

syoyo commented Nov 8, 2024

I have described the logic of Inherits to wki: https://github.com/lighttransport/tinyusdz/wiki/Composition-arcs#inherits

PR #210 has been merged.

Prims cannot inherit from an ancestor or descendant, inherit “bases” should be defined as siblings or outside of the target prim’s hierarchy, for example at the root level

After adding Cycle detection check of "inherits" Prim path in another PR, Inheritance management should be complete in TinyUSDZ!

@syoyo syoyo added bug Something isn't working enhancement New feature or request labels Nov 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants