PixarAnimationStudios / OpenUSD

Universal Scene Description
http://www.openusd.org
Other
6.11k stars 1.21k forks source link

UsdShade.Output.ConnectToSource python signature change #1811

Closed dolivares-spinvfx closed 2 years ago

dolivares-spinvfx commented 2 years ago

Description of Issue

We've noticed a regression in USD 21+ related to UsdShade.Output.ConnectToSource:

# Error: ArgumentError: file <maya console> line 8: Python argument types in
    Output.ConnectToSource(Output, Shader, str)
did not match C++ signature:
    ConnectToSource(pxrInternal_v0_21__pxrReserved__::UsdShadeOutput {lvalue}, pxrInternal_v0_21__pxrReserved__::UsdShadeOutput sourceOutput)
    ConnectToSource(pxrInternal_v0_21__pxrReserved__::UsdShadeOutput {lvalue}, pxrInternal_v0_21__pxrReserved__::UsdShadeInput sourceInput)
    ConnectToSource(pxrInternal_v0_21__pxrReserved__::UsdShadeOutput {lvalue}, pxrInternal_v0_21__pxrReserved__::SdfPath sourcePath)
    ConnectToSource(pxrInternal_v0_21__pxrReserved__::UsdShadeOutput {lvalue}, pxrInternal_v0_21__pxrReserved__::UsdShadeConnectableAPI source, pxrInternal_v0_21__pxrReserved__::TfToken sourceName, pxrInternal_v0_21__pxrReserved__::UsdShadeAttributeType sourceType=pxr.UsdShade.AttributeType.Output, pxrInternal_v0_21__pxrReserved__::SdfValueTypeName typeName=<pxr.Sdf.ValueTypeName object at 0x7f06aa1159f0>)
    ConnectToSource(pxrInternal_v0_21__pxrReserved__::UsdShadeOutput {lvalue}, pxrInternal_v0_21__pxrReserved__::UsdShadeConnectionSourceInfo source, pxrInternal_v0_21__pxrReserved__::UsdShadeConnectionModification mod=pxr.UsdShade.ConnectionModification.Replace) # 

Steps to Reproduce

from pxr import Usd, UsdShade
stage = Usd.Stage.CreateInMemory()
material = UsdShade.Material.Define(stage, '/material')
shader = UsdShade.Shader.Define(stage, '/material/shader')
shader.CreateIdAttr('UsdPreviewSurface')
material.CreateSurfaceOutput().ConnectToSource(shader, 'surface')

This give the above error. Doing the following seems to work: material.CreateSurfaceOutput().ConnectToSource(shader.ConnectableAPI(), 'surface')

I found a related closed issue: https://github.com/PixarAnimationStudios/USD/issues/547

System Information (OS, Hardware)

CentOS Linux release 7.9.2009 (Core)

Package Versions

21.05 21.08 (issue not present in 20.08)

asluk commented 2 years ago

I believe this is expected, per the USD 21.05 release notes - https://github.com/PixarAnimationStudios/USD/blob/release/CHANGELOG.md#usd-3

"Removed UsdShadeConnectableAPI implicit constructors from UsdShadeShader and UsdShadeNodeGraph to enforce independence of connectability concept from shader graphs. Callers are required to convert these explicitly now, which can be done via the ConnectableAPI() method on Shader and NodeGraph."

spiffmon commented 2 years ago

Right. Due to the Issue #547 that you referenced, @dolivares-spinvfx , we needed to remove implicit conversions from Shader, Material, NodeGraph, but there were still implicit conversions in ConnectableAPI . But with the generalization (for UsdLux) of ConnectableAPI to be applicable to schemas outside of UsdShade, it was no longer possible for ConnectableAPI to know about all of the connectable schemas for which it needs to declare conversions, so we thought it was more consistent to have it assume any schemas.

So your workaround is actually the prescribed workflow. Sorry for the inconvenience!

dolivares-spinvfx commented 2 years ago

Thanks for the clarification. I suppose the documentation needs to be updated then, as it does not describe this new workflow:

https://graphics.pixar.com/usd/release/tut_simple_shading.html#add-a-usdpreviewsurface

spiffmon commented 2 years ago

Oh thank you for pointing that out! We are slowly trying to get code snippets in documentation out into standalone files that get included, so that they can be exercised in tests, but it's pretty slow going. Will get that one, at least, updated!

dolivares-spinvfx commented 2 years ago

Thanks!