Metadata

[ratmice]

Sorry I haven't been particularly prompt in getting to implementing the MetaData reader/writer. Wanted to post this, just to show some progress, and in case I took some wrong turn in what i've implemented so far...

This is unfinished/untested in that I need to implement the PtexTexture::getMetaData reader aspects before I can really test it. Otherwise this should eventually Fix #10

[ratmice]

So I tried to implement (not very elegant but low level reading of metadata), this doesn't support conversion back into the MetaDataType, it just returns an &[u8]. It seems likely we can make some nicer mechanism.

But I figured none the less a low level reader seemed like a good stopping point, due to string encodings and avoiding introduction of new types for the reader to fan out to, because the AsUint8Ptr style really only seems to work for writing.

So, I think this might require some more work, but is probably complete enough to look at.

[davvid]

Thanks, this is looking pretty good to me. I'm not sure why github complains about the conflict because when I merge locally (which is what I already did) it merged cleanly.

Is there anything I should wait on before merging, or is this more or less good to go?

I've also been meaning to come back to #6 soon ~ let me know if you're still looking into that one or if there's some areas that could use some help.

[ratmice]

I think this one is more or less good to go, unless you want me to do some history rewriting to get rid of the interim error in the ptexwriter_write_meta_data missing argument, and squash the rustfmt commit.

I'll have to look at the conflict tomorrow, unless you get to it first (it is fine to push to my branch).

Regarding #6 i'll have to look at it a bit, I had redone it to use TryFrom, but I personally think we're pretty close to showing that the panics are unreachable. The main thing I think left is 755efd4aaf5bec1a63bbe9b28c0dc83dba05d4cd only tests that we can't write out of range enum variants. But in the case of a corrupted file we could potentially still read one, I need to look at the ptex reader code to see whether that could be a problem making the panic reachable.

[ratmice]

Looking at your commits to update this, I definitely agree that the value null reference checks aren't needed (passing them should be UB), I had added them while debugging what turned out to an ABI mismatch between the rust and C++ code caused by a bad declaration on the rust side -- which was causing all the arguments to be passed in the wrong locations.

Once I figured that out it should have removed those since passing null references there is already undefined behavior.