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

GetAttr inconsistently returning by copy or by const reference #2617

Closed
ShaddyDC opened this issue Aug 23, 2023 · 4 comments
Closed

GetAttr inconsistently returning by copy or by const reference #2617

ShaddyDC opened this issue Aug 23, 2023 · 4 comments

Comments

@ShaddyDC
Copy link

Description of Issue

I'm working on wrapping some of OpenUSD with Rust, and I ran into the issue that some types return UsdAttribute either by const reference or by value.
Specifically, UsdGeomPrimvar has the signature UsdAttribute const & UsdGeomPrimvar::GetAttr()const, as does UsdShadeInput, but UsdShadeOutput has UsdAttribute UsdShadeOutput::GetAttr() const.
Unless that causes other issues, it would be convenient for me if they could all return a constant reference, or if necessary, by value.
Or is there maybe a reason for this I've missed?

I have not checked where else this may be applied inconsistently.

Steps to Reproduce

  1. NA

System Information (OS, Hardware)

NA

Package Versions

23.08

Build Flags

NA

@spiffmon
Copy link
Member

spiffmon commented Aug 23, 2023 via email

@ShaddyDC
Copy link
Author

Here is a more thorough list then:

> rg 'UsdAttribute( const)? ?& ?GetAttr'
pxr/usd/usdUtils/sparseValueWriter.h
139:    const UsdAttribute & GetAttr() const {

pxr/usd/usdGeom/xformOp.h
331:    UsdAttribute const &GetAttr() const { 

pxr/usd/usdGeom/primvar.h
400:    UsdAttribute const &GetAttr() const { return _attr; }

pxr/usd/usdGeom/constraintTarget.h
111:    UsdAttribute const &GetAttr() const { return _attr; }

pxr/usd/usdShade/input.h
229:    const UsdAttribute &GetAttr() const { return _attr; }

pxr/usd/usd/attributeQuery.h
124:    const UsdAttribute& GetAttribute() const;

pxr/usd/usdSkel/inbetweenShape.h
128:    UsdAttribute const &GetAttr() const { return _attr; }
> rg 'UsdAttribute( const)? ?GetAttr'
pxr/usd/usd/prim.h
1569:    USD_API UsdAttribute GetAttributeAtPath(const SdfPath& path) const;
1709:    UsdAttribute GetAttribute(const TfToken& attrName) const;

pxr/usd/usd/stage.h
813:    UsdAttribute GetAttributeAtPath(const SdfPath &path) const;

pxr/usd/usdShade/output.h
213:    UsdAttribute GetAttr() const { return _attr; }

This makes me think like that instance may be the only GetAttr call then, though I'm not sure what the preffered way for the others would be.

@jesschimein
Copy link
Contributor

Filed as internal issue #USD-8617

@spiffmon
Copy link
Member

stage.h and prim.h must return by value, because the objects are not retained by UsdStage or UsdPrim. SO it does look like output.h is the only site that can and should change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants