-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
[usd] Fix metadata authoring bug. #529
Conversation
Filed as internal issue #161890. |
Previously, authoring metadata from python, such as in CustomData, USD would allow raw python lists, but would not convert them to the proper VtArray type. This would cause layers to be unable to export <-> import roundtrip, as well as unable to serialize to crate.
5d4dd24
to
4aa1cf6
Compare
Rebased and clean with dev. |
@spiffmon Sunya and I discussed this one offline; do you see anything wrong with this approach? I'm not as familiar with stuff in Vt so I wasn't sure. |
Hi @superfunc,
Nothing wrong with the approach, however:
1. Nothing prevents heterogeneous python lists; you are only checking the
type of the first VtValue element, and then assuming all the rest are the
same, and doing UncheckedGet's, which will error (once upon a time
terminate the program!). So you should ensure you have a homogenous vector
prior to calling _ToVtArray
2. Can you add support for array of SdfAssetPath's?
3. Does SdfSpec::SetInfo have the same problem ?
…--spiff
On Tue, Sep 25, 2018 at 12:14 PM superfunc ***@***.***> wrote:
@spiffmon <https://github.com/spiffmon> Sunya and I discussed this one
offline; do you see anything wrong with this approach? I'm not as familiar
with stuff in Vt so I wasn't sure.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#529 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AF7qaEMcpOOvXkWVXau68IB5CDRkbIbSks5ueoCagaJpZM4UnGXE>
.
|
1,2: Very good points, missed that. Checking on each element seems expensive, I'll see if there is a boost python way to check if a list is homogeneous. |
@spiffmon Yeah, looks like the problem lives in Sdf, as well: >>> p.SetInfoDictionaryValue('customData', 'int_array', ["foo",2,3,4])
>>> print l.ExportToString()
#sdf 1.4.32
def "p" (
customData = {
int_array = [foo, 2, 3, 4]
}
)
{
} I'll get a separate PR together that fixes that one as well (will clean this one up later this week). |
Actually, I wonder if the problem lives lower, in VtValues casting functions that this relies on. I'll need to dig a bit more to be sure. |
See #529 (Internal change: 1913374) (Internal change: 1913447)
Looks like @gitamohr fixed this in his referenced commit, closing out. |
…lve/dev sync origin/dev to adsk/dev
Description of Change(s)
Previously, authoring metadata from python, such as in CustomData, USD would allow python lists, but would not convert them to the proper VtArray type. This would cause layers to be unreadable, unserializable etc.
For example,
Produces the following broken file (notice the missing typename):
Following my change, we produce a working file:
My change also disallows unknown list types, so bogus authoring can't get in anymore(e.g. if someone tried to author a list of Foo()s).
Fixes Issue(s)