Suggestion: add cgltf_load_buffers_memory, and rewrite cgltf_load_buffers to use it

[ghost]

I just checked out more in-depth in how to load up cgltf from memory / a VFS library (like PhysFS).
It appears to me for loading files with cgltf with all external resources there are two choices:

1. Use cgltf_load_buffers (which requires file access)

2. Duplicate the entire content of cgltf_load_buffers, especifically also the loop recognizing & handling data: URIs, and dealing with the data->buffers detail members which doesn't seem particularly future-proof, and adjust it to load from memory or wherever else

The second approach seems unnecessarily more complicated, especially since I assume most more versatile game engines and frameworks will need this. It also seems counter-intuitive to me that reading from memory isn't the regular case with file reading being a specialization case, rather than the other way around.

Therefore I suggest the following:

1. There could be an implementation alike to this:

   cgltf_result cgltf_load_buffers_memory(
       const cgltf_options* options, cgltf_data* data,
       void (*data_callback)(const char *uri, cgltf_size *buffer_size, const void **buffer_data, void *userdata),
       void *userdata
   )
   

   which does what cgltf_load_buffers does now, except it calls the data_callback to retrieve the file contents instead of disk access. The data_callback shall set *buffer_size = 0 on failure to obtain the data, and shall otherwise point *buffer_data = ... to an existing data buffer. It shall be up to the cgltf user, who supplied data_callback, to dispose of whatever the data_callback allocated during its use after the entire run of cglt_load_buffers_from_memory has returned, and all data was safely copied into the cgltf structures.

2. Once step 1 is implemented, possibly reimplement cgltf_load_buffers on top by just using a callback to read the files supplied to the new cgltf_load_buffers_memory to avoid code duplication

3. (Very optional and nasty to do, I know. But just as an idea:) possibly fix the whole naming scheme with some deprecation plan. It's cgltf_parse and cgltf_parse_file right now, so shouldn't cgltf_load_buffers actually be cgltf_load_buffers_file? With possibly this new cgltf_load_buffers_memory function eventually becoming cgltf_load_buffers as consistently also the memory-based variant (or alternatively, maybe prefix all the memory-based loading functions with something like _memory, e.g. cgltf_parse_memory)

[ghost]

I am also just realizing, if you moved the library away from the notion that data buffers ever need to be set manually (which right now appears to be an unavoidably required thing to do for loading from memory, see above) then cgltf could also apply sparse-to-non-spare conversion automatically in the load_buffers functions.

Because if cgltf doesn't automatically apply non-sparse conversion, isn't it reasonable to assume a lot of users just won't implement it even though the functionality is right there in cgltf? I also just don't see the use in making everyone do it manually when it could be a trivial load time conversion that pretty much everyone would prefer to support anyway. (Or who wouldn't want to support sparse data, intentionally? Maybe I'm missing a use case)

[jkuhlmann]

Did you see the file options in cgltf_options? There are read and release callbacks which should hopefully be sufficient to access data from buffers or a VFS library. They default to being implemented using fopen(), fread(), etc., but can easily be reimplemented so that you don't have to duplicate any functionality.
Does that help?

[ghost]

@jkuhlmann ah nice! Examples would help btw to not miss such things, I just didn't notice that was a possibility.

However that still leaves some cases out in the open, mainly: 1. custom ways of file URI resolution (e.g. my engine has custom guessing that tries multiple ways to resolve relative paths relative to model, relative to game dir, ... to compensate for when the model stores it weirdly or the user moves files around) 2. supporting HTTP/HTTPS and other ways of retrieving resources.

It also seems to me like cgltf_load_buffer_file has a bug. Wouldn't it need to parse and resolve % encodings which are mandatory in GLTF URIs? (spec quote: Reserved characters must be percent-encoded, per RFC 3986, Section 2.2., this also applies to relative paths without file:// protocol prefix) Edit: or cgltf_load_buffer would need to handle this, but neither seem to do it

Ideally, I think cgltf should at the very least offer a custom callback to let me resolve any URI to a full absolute file path (not a relative one that it somehow appends) as an alternative to whatever it does. Because my URI resolver is somewhat elaborate and does support a file:// prefix, % for reserved characters, and more, so I would want to keep using that without needing to reimplement cgltf_load_buffers. I'm personally not interested in HTTP/HTTPS or other remote URI support, so I don't care about that.

[jkuhlmann]

Alright, your points all make sense. I've already got examples on my list. And, yes, it looks like cgltf should resolve reserved URI character encodings.

However, about resolving URIs, doesn't the code pretty much leave the URI intact if you just pass in an empty string for the gltf_path when calling cgltf_load_buffers()?

[ghost]

Hm but now imagine file://abc/def%2520blubb is passed in, now if you add resolving reserved URI encodings but keep the naive expect-relative-paths-only you suddenly get file://abc/def%20blubb which doesn't make sense as either an URI or file path anymore, and would even break an URI-aware custom fread() function I might pass in while also not work for the default system-fread().

Now if I could pass in a custom URI-to-file-path-or-whatever-my-custom-fread-wants callback I could decide on my own what I need to escape or unescape, while the default could still deal with reserved URI character encodings properly and make more paths work for fread() while not ruining my day if I want to plug in a more comprehensive URI processing.

So that is why I think an URI resolution callback to set in the options would really be the best way to solve all of this.