search

jkuhlmann / cgltf

:diamond_shape_with_a_dot_inside: Single-file glTF 2.0 loader and writer written in C99
MIT License
1.44k stars 136 forks source link

2 GB+ json parser support #197

Closed pixeljetstream closed 1 year ago

pixeljetstream commented 1 year ago

Hi,

hitting the limits due to jsmn using int and unsigned int rather than ptrdiff_t and size_t . I started making appropriate edits, but they are rather invasive as a lot of the cgltf_parse functions now need to be changed to ptrdiff_t returns and ptrdiff_t i inputs. As well as several size calculations. Just checking if this is fine

jkuhlmann commented 1 year ago

Hi!

I definitely think that's a justified change. It sounds like you need to change jsmn as well? Do you think it would make sense to make a PR for that library first, integrate the results back into cgltf and then modify cgltf?

pixeljetstream commented 1 year ago

There isn't a lot traction in jsmn. Last commit end of 2021. Plenty open MRs. So not sure if worth doing. But I can certainly do another fork at least

zeux commented 1 year ago

FWIW a first step could be to upgrade byte indices to ptrdiff_t/size_t - this is likely to mostly impact jsmn and not a whole lot else. int i inputs to parsing functions refer to token locations in the stream - I don't know what the "normal" ratio is between bytes and tokens but I'd guess that most 2+GB JSON GLTFs have a much smaller number of tokens. A subsequent conversion could allow for >2B tokens as well but that could be done separately to reduce churn.

pixeljetstream commented 1 year ago

That was indeed the minimal work I did on my end to get it working. Just thought it may be a bit unclean to commit to main. But would be less churn as you said, so fine with me.

jkuhlmann commented 1 year ago

Alright, I'm also fine getting this change in without involving jsmn. Feel free to open a PR then.