zeux
commented
1 year ago Note: there's an issue that this change runs into wrt extras inside camera data for malformed inputs - this issue was present before, so I'd like to fix it in a separate PR. Briefly, parsing cameras that have both orthographic and perspective fields set overwrites the data of the other union, and since the camera type can flip back and forth during parsing because multiple perspective/orthographic/etc. sections are permitted, this may result in memory leaks and/or crashes.
I'm tempted to simply remove inner extras (the camera object supports extras and it's unclear why it's useful to specify extras inside the perspective/orthographic sub-structures), but we should probably also make the actual parsing code stricter. (fixed in #186)
jkuhlmann
This change switches cgltf_extras to embed the JSON data the same way we handle unprocessed extensions, by copying it to heap. While this results in a few extra (ha!) heap allocations, in practice the other data such as object names et al dominates the allocation traffic. The benefit here is that it makes using the library simpler - extras can be read from directly without copying, and this means that the original glTF data can be discarded which can improve memory consumption.
The downside, of course, is that there's quite a few places where extras is used. This can be mitigated by #118 in the future, but for now we diligently free extras for every struct that has them, something that is made easier by the fact that most structs that contain extras also contain extensions.
Note that this change leaves start_offset/end_offset in tact for now, which makes it easy to upgrade to the new version, but in the future we can remove these - cgltf_write can work with manually assembled trees so for now it handles both ::data and ::start/end_offset.
I've opted to remove cgltf_extras from cgltf_pbr_metallic_roughness since it's the only material extension struct that has it, and that makes the code more consistent - it seems unlikely that any users use that specific extras field, but if they do we can bring it back upon request.
Fixes #116.