jkuhlmann / cgltf

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

add texture_index to struct cgltf_texture_view #206

Closed EasyIP2023 closed 1 year ago

EasyIP2023 commented 1 year ago

Store json value at "index" versus calculating it.

"pbrMetallicRoughness": {
    "baseColorTexture": {
        "index": 12
    },
    "metallicRoughnessTexture": {
        "index": 1
    }
},

There exists instances where one wants to acquire the texture index for a given material (primitive).

EDIT (may 18, 2023)

There exists instances where one wants to store the json "index" value. This value of course references the index in the "textures" array.

EasyIP2023 commented 1 year ago

Hey, @jkuhlmann realize the github actions just finished on MR. Wanted to rename index to accessor_index. So it makes more since what index is for and aligns more with the other MR's.

zeux commented 1 year ago

My understanding has been that the idiomatic way to get indices from pointers it to simply subtract the address of the relevant array. E.g. for this, you'd get the pointer via material->pbr_metallic_roughness.base_color_texture.texture - data->textures. And we definitely should not be using the name accessor, as it's very misleading when applied to textures - but by and large I feel like the extra indices PRs should not be merged, as they just introduce discrepancies between different parts of the model (some pointers have duplicate indices and some don't), and indices can always be recovered from the existing data.

EasyIP2023 commented 1 year ago

@zeux My understanding is that "index" key, value refers to the index in the accessors array. And of course the accessor gives you the bufferView index, data type (data size), and amount of elements of a particular type stored in a memory segment of a buffer. Adding will allow for faster accessor array accesses. If the application desires it. Yes, there are ways to get that value, but it's in my opinion its better to store it to quickly access it.

EasyIP2023 commented 1 year ago

@zeux Thank you, thought about it more. Yeah, your right. Sorry.

mesh->primitives->attributes->name and meshes->primitives->indices "values" are the index in the accessors array.

"primitives": [                                                                                                                                                                                                   
   {
      "attributes": {                                                                                                                                                                                               
         "TEXCOORD_0": 10,                                                                                                                                                                                           
         "NORMAL": 11,                                                                                                                                                                                               
         "TANGENT": 12,                                                                                                                                                                                              
         "POSITION": 13                                                                                                                                                                                              
      },                                                                                                                                                                                                            
      "indices": 14
   }
]

materials->pbrMetallicRoughness->baseColorTexture->index & materials->normalTexture->index & materials->occlusionTexture->index "values" reference index in the "textures" array.

"materials": [
    {
      "pbrMetallicRoughness": {
          "baseColorTexture": {
        "index": 12
          },
          "metallicRoughnessTexture": {
        "index": 1
      }
      },
   }
]

Correct me if I'm wrong.