From f8baa0d0dc0a3b9c7f744b38bbcd5188ebfedb54 Mon Sep 17 00:00:00 2001 From: Shawn Hatori Date: Sun, 17 Sep 2023 19:30:58 -0400 Subject: [PATCH 1/3] Unpack indices of arbitrary component types Since the `memcpy` only needs the size of the index data, it can use the component size (when equal to the stride) to determine the total number of bytes to copy. This allows it to work for different component types, including the common `cgltf_component_type_r_16u`. This also adds a reference comment for the function at the top of the file, consistent with `cgltf_accessor_unpack_floats`. --- cgltf.h | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/cgltf.h b/cgltf.h index af24c65..6e7b426 100644 --- a/cgltf.h +++ b/cgltf.h @@ -63,6 +63,11 @@ * By passing null for the output pointer, users can find out how many floats are required in the * output buffer. * + * `cgltf_accessor_unpack_indices` reads in the index data from an accessor. Assumes that + * `cgltf_load_buffers` has already been called. By passing null for the output pointer, users can + * find out how many indices are required in the output buffer. Returns 0 if the accessor is + * sparse. + * * `cgltf_num_components` is a tiny utility that tells you the dimensionality of * a certain accessor type. This can be used before `cgltf_accessor_unpack_floats` to help allocate * the necessary amount of memory. `cgltf_component_size` and `cgltf_calc_size` exist for @@ -2629,7 +2634,7 @@ cgltf_size cgltf_animation_channel_index(const cgltf_animation* animation, const return (cgltf_size)(object - animation->channels); } -cgltf_size cgltf_accessor_unpack_indices(const cgltf_accessor* accessor, cgltf_uint* out, cgltf_size index_count) +cgltf_size cgltf_accessor_unpack_indices_(const cgltf_accessor* accessor, void* out, cgltf_size out_component_size, cgltf_size index_count) { if (out == NULL) { @@ -2637,6 +2642,7 @@ cgltf_size cgltf_accessor_unpack_indices(const cgltf_accessor* accessor, cgltf_u } index_count = accessor->count < index_count ? accessor->count : index_count; + cgltf_size index_component_size = cgltf_component_size(accessor->component_type); if (accessor->is_sparse) { @@ -2646,6 +2652,10 @@ cgltf_size cgltf_accessor_unpack_indices(const cgltf_accessor* accessor, cgltf_u { return 0; } + if (index_component_size > out_component_size) + { + return 0; + } const uint8_t* element = cgltf_buffer_view_data(accessor->buffer_view); if (element == NULL) { @@ -2653,23 +2663,28 @@ cgltf_size cgltf_accessor_unpack_indices(const cgltf_accessor* accessor, cgltf_u } element += accessor->offset; - if (accessor->component_type == cgltf_component_type_r_32u && accessor->stride == sizeof(cgltf_uint)) + if (index_component_size == out_component_size && accessor->stride == out_component_size) { - memcpy(out, element, index_count * sizeof(cgltf_uint)); + memcpy(out, element, index_count * index_component_size); } else { - cgltf_uint* dest = out; - - for (cgltf_size index = 0; index < index_count; index++, dest++, element += accessor->stride) + uint8_t* dest = (uint8_t*)out; + for (cgltf_size index = 0; index < index_count; index++, dest += out_component_size, element += accessor->stride) { - *dest = (cgltf_uint)cgltf_component_read_index(element, accessor->component_type); + cgltf_size index_data = cgltf_component_read_index(element, accessor->component_type); + memcpy(dest, &index_data, index_component_size); } } return index_count; } +cgltf_size cgltf_accessor_unpack_indices(const cgltf_accessor* accessor, cgltf_uint* out, cgltf_size index_count) +{ + return cgltf_accessor_unpack_indices_(accessor, out, sizeof(cgltf_uint), index_count); +} + #define CGLTF_ERROR_JSON -1 #define CGLTF_ERROR_NOMEM -2 #define CGLTF_ERROR_LEGACY -3 From f295b3abfc05ca930b84c293b89e191960165862 Mon Sep 17 00:00:00 2001 From: Shawn Hatori Date: Wed, 25 Oct 2023 15:24:10 -0400 Subject: [PATCH 2/3] use fixed size memcpy, update function signature --- cgltf.h | 61 ++++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 39 insertions(+), 22 deletions(-) diff --git a/cgltf.h b/cgltf.h index 6e7b426..4ffbcf4 100644 --- a/cgltf.h +++ b/cgltf.h @@ -66,11 +66,11 @@ * `cgltf_accessor_unpack_indices` reads in the index data from an accessor. Assumes that * `cgltf_load_buffers` has already been called. By passing null for the output pointer, users can * find out how many indices are required in the output buffer. Returns 0 if the accessor is - * sparse. + * sparse or if the output component size is less than the accessor's component size. * * `cgltf_num_components` is a tiny utility that tells you the dimensionality of * a certain accessor type. This can be used before `cgltf_accessor_unpack_floats` to help allocate - * the necessary amount of memory. `cgltf_component_size` and `cgltf_calc_size` exist for + * the necessary amount of memory. `cgltf_component_size` and `cgltf_calc_size` exist for * similar purposes. * * `cgltf_accessor_read_float` reads a certain element from a non-sparse accessor and converts it to @@ -80,7 +80,7 @@ * * `cgltf_accessor_read_uint` is similar to its floating-point counterpart, but limited to reading * vector types and does not support matrix types. The passed-in element size is the number of uints - * in the output buffer, which should be in the range [1, 4]. Returns false if the passed-in + * in the output buffer, which should be in the range [1, 4]. Returns false if the passed-in * element_size is too small, or if the accessor is sparse. * * `cgltf_accessor_read_index` is similar to its floating-point counterpart, but it returns size_t @@ -855,7 +855,7 @@ cgltf_size cgltf_component_size(cgltf_component_type component_type); cgltf_size cgltf_calc_size(cgltf_type type, cgltf_component_type component_type); cgltf_size cgltf_accessor_unpack_floats(const cgltf_accessor* accessor, cgltf_float* out, cgltf_size float_count); -cgltf_size cgltf_accessor_unpack_indices(const cgltf_accessor* accessor, cgltf_uint* out, cgltf_size index_count); +cgltf_size cgltf_accessor_unpack_indices(const cgltf_accessor* accessor, void* out, cgltf_size out_component_size, cgltf_size index_count); /* this function is deprecated and will be removed in the future; use cgltf_extras::data instead */ cgltf_result cgltf_copy_extras_json(const cgltf_data* data, const cgltf_extras* extras, char* dest, cgltf_size* dest_size); @@ -1050,7 +1050,7 @@ static cgltf_result cgltf_default_file_read(const struct cgltf_memory_options* m fclose(file); return cgltf_result_out_of_memory; } - + cgltf_size read_size = fread(file_data, 1, file_size, file); fclose(file); @@ -1982,7 +1982,7 @@ void cgltf_free(cgltf_data* data) data->memory.free_func(data->memory.user_data, data->materials); - for (cgltf_size i = 0; i < data->images_count; ++i) + for (cgltf_size i = 0; i < data->images_count; ++i) { data->memory.free_func(data->memory.user_data, data->images[i].name); data->memory.free_func(data->memory.user_data, data->images[i].uri); @@ -2634,7 +2634,7 @@ cgltf_size cgltf_animation_channel_index(const cgltf_animation* animation, const return (cgltf_size)(object - animation->channels); } -cgltf_size cgltf_accessor_unpack_indices_(const cgltf_accessor* accessor, void* out, cgltf_size out_component_size, cgltf_size index_count) +cgltf_size cgltf_accessor_unpack_indices(const cgltf_accessor* accessor, void* out, cgltf_size out_component_size, cgltf_size index_count) { if (out == NULL) { @@ -2666,25 +2666,42 @@ cgltf_size cgltf_accessor_unpack_indices_(const cgltf_accessor* accessor, void* if (index_component_size == out_component_size && accessor->stride == out_component_size) { memcpy(out, element, index_count * index_component_size); + return index_count; } - else + + // The component size of the index data is less than the component size of the output array, so index data will be padded. + // NOTE: Assumes little-endian. + uint8_t* dest = (uint8_t*)out; + switch (index_component_size) { - uint8_t* dest = (uint8_t*)out; + case 1: + for (cgltf_size index = 0; index < index_count; index++, dest += out_component_size, element += accessor->stride) + { + cgltf_size index_data = cgltf_component_read_index(element, accessor->component_type); + memcpy(dest, &index_data, 1); + } + break; + case 2: + for (cgltf_size index = 0; index < index_count; index++, dest += out_component_size, element += accessor->stride) + { + cgltf_size index_data = cgltf_component_read_index(element, accessor->component_type); + memcpy(dest, &index_data, 2); + } + break; + case 4: for (cgltf_size index = 0; index < index_count; index++, dest += out_component_size, element += accessor->stride) { cgltf_size index_data = cgltf_component_read_index(element, accessor->component_type); - memcpy(dest, &index_data, index_component_size); + memcpy(dest, &index_data, 4); } + break; + default: + return 0; } return index_count; } -cgltf_size cgltf_accessor_unpack_indices(const cgltf_accessor* accessor, cgltf_uint* out, cgltf_size index_count) -{ - return cgltf_accessor_unpack_indices_(accessor, out, sizeof(cgltf_uint), index_count); -} - #define CGLTF_ERROR_JSON -1 #define CGLTF_ERROR_NOMEM -2 #define CGLTF_ERROR_LEGACY -3 @@ -3816,7 +3833,7 @@ static int cgltf_parse_json_texture_view(cgltf_options* options, jsmntok_t const out_texture_view->texcoord = cgltf_json_to_int(tokens + i, json_chunk); ++i; } - else if (cgltf_json_strcmp(tokens + i, json_chunk, "scale") == 0) + else if (cgltf_json_strcmp(tokens + i, json_chunk, "scale") == 0) { ++i; out_texture_view->scale = cgltf_json_to_float(tokens + i, json_chunk); @@ -3901,11 +3918,11 @@ static int cgltf_parse_json_pbr_metallic_roughness(cgltf_options* options, jsmnt if (cgltf_json_strcmp(tokens+i, json_chunk, "metallicFactor") == 0) { ++i; - out_pbr->metallic_factor = + out_pbr->metallic_factor = cgltf_json_to_float(tokens + i, json_chunk); ++i; } - else if (cgltf_json_strcmp(tokens+i, json_chunk, "roughnessFactor") == 0) + else if (cgltf_json_strcmp(tokens+i, json_chunk, "roughnessFactor") == 0) { ++i; out_pbr->roughness_factor = @@ -4375,11 +4392,11 @@ static int cgltf_parse_json_image(cgltf_options* options, jsmntok_t const* token int size = tokens[i].size; ++i; - for (int j = 0; j < size; ++j) + for (int j = 0; j < size; ++j) { CGLTF_CHECK_KEY(tokens[i]); - if (cgltf_json_strcmp(tokens + i, json_chunk, "uri") == 0) + if (cgltf_json_strcmp(tokens + i, json_chunk, "uri") == 0) { i = cgltf_parse_json_string(options, tokens, i + 1, json_chunk, &out_image->uri); } @@ -4459,7 +4476,7 @@ static int cgltf_parse_json_sampler(cgltf_options* options, jsmntok_t const* tok = cgltf_json_to_int(tokens + i, json_chunk); ++i; } - else if (cgltf_json_strcmp(tokens + i, json_chunk, "wrapT") == 0) + else if (cgltf_json_strcmp(tokens + i, json_chunk, "wrapT") == 0) { ++i; out_sampler->wrap_t @@ -4509,7 +4526,7 @@ static int cgltf_parse_json_texture(cgltf_options* options, jsmntok_t const* tok out_texture->sampler = CGLTF_PTRINDEX(cgltf_sampler, cgltf_json_to_int(tokens + i, json_chunk)); ++i; } - else if (cgltf_json_strcmp(tokens + i, json_chunk, "source") == 0) + else if (cgltf_json_strcmp(tokens + i, json_chunk, "source") == 0) { ++i; out_texture->image = CGLTF_PTRINDEX(cgltf_image, cgltf_json_to_int(tokens + i, json_chunk)); From 1afdc74c3ae0337c040d184e9985a022105ca382 Mon Sep 17 00:00:00 2001 From: Shawn Hatori Date: Sun, 29 Oct 2023 12:46:54 -0400 Subject: [PATCH 3/3] write with respect to output array size --- cgltf.h | 25 +++++++------------------ 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/cgltf.h b/cgltf.h index 4ffbcf4..4124e9b 100644 --- a/cgltf.h +++ b/cgltf.h @@ -2669,34 +2669,23 @@ cgltf_size cgltf_accessor_unpack_indices(const cgltf_accessor* accessor, void* o return index_count; } - // The component size of the index data is less than the component size of the output array, so index data will be padded. - // NOTE: Assumes little-endian. - uint8_t* dest = (uint8_t*)out; - switch (index_component_size) + // The component size of the output array is larger than the component size of the index data, so index data will be padded. + switch (out_component_size) { - case 1: - for (cgltf_size index = 0; index < index_count; index++, dest += out_component_size, element += accessor->stride) - { - cgltf_size index_data = cgltf_component_read_index(element, accessor->component_type); - memcpy(dest, &index_data, 1); - } - break; case 2: - for (cgltf_size index = 0; index < index_count; index++, dest += out_component_size, element += accessor->stride) + for (cgltf_size index = 0; index < index_count; index++, element += accessor->stride) { - cgltf_size index_data = cgltf_component_read_index(element, accessor->component_type); - memcpy(dest, &index_data, 2); + ((uint16_t*)out)[index] = (uint16_t)cgltf_component_read_index(element, accessor->component_type); } break; case 4: - for (cgltf_size index = 0; index < index_count; index++, dest += out_component_size, element += accessor->stride) + for (cgltf_size index = 0; index < index_count; index++, element += accessor->stride) { - cgltf_size index_data = cgltf_component_read_index(element, accessor->component_type); - memcpy(dest, &index_data, 4); + ((uint32_t*)out)[index] = (uint32_t)cgltf_component_read_index(element, accessor->component_type); } break; default: - return 0; + break; } return index_count;