CesiumGS / CesiumGS/cesium-native

Accessors and the handling of padding bytes

Open
#677 2 comments 0 reactions 0 assignees View on GitHub
bug
Dominant language
C++
Stars
623
Forks
277
PR merge metrics
No merged PRs in 30d

Description

The [`Accessor::computeBytesPerVertex` method](https://github.com/CesiumGS/cesium-native/blob/405e8519a4af91307805897fa4813103dec56b8a/CesiumGltf/src/Accessor.cpp#L59) computes the size of one element, but... it is not really clear what that means.

For example: For `MAT2` with `BYTE` component types, this method will return `4 * 1 = 4`. But the result should probably be `8`, according to [the glTF specification, section 3.6.2.4, 'Data Alignment'](https://registry.khronos.org/glTF/specs/2.0/glTF-2.0.html#data-alignment), which says

> Accessors of matrix type have data stored in column-major order; start of each column **MUST** be aligned to 4-byte boundaries. Specifically, when `ROWS * SIZE_OF_COMPONENT` (where `ROWS` is the number of rows of the matrix) is not a multiple of 4, then (`ROWS * SIZE_OF_COMPONENT) % 4` padding bytes **MUST** be inserted at the end of each column.

So that "size" that is computed there has to handle the case of padding bytes that are added for matrices. *Not* taking this into account may cause hard-to-track out-of-bounds-crashes for certain input data sets.

---

Moreover, there doesn't seem to be a sensible/convenient way to access such data. The `AccessorView` does offer some functions for that, but assumes contiguous data. For example, consider a glTF asset with the following data:

![Cesium Padding](https://github.com/CesiumGS/cesium-native/assets/5597569/df26a205-593f-4972-b84d-a29e4a64ba62)

Reading that with `CesiumGltf` and trying to access it with an `AccessorView` results in this output:

```
at 0 buffer has 1
at 1 buffer has 2
at 2 buffer has 0
at 3 buffer has 0
at 4 buffer has 3
at 5 buffer has 4
at 6 buffer has 0
at 7 buffer has 0
at 8 buffer has 11
at 9 buffer has 22
at 10 buffer has 0
at 11 buffer has 0
at 12 buffer has 33
at 13 buffer has 44
at 14 buffer has 0
at 15 buffer has 0
element0 i8mat2x2((1, 2), (0, 0))
element1 i8mat2x2((3, 4), (0, 0))
```
The buffer contains the right elements, but the elements (2x2 matrices with int8) include the padding bytes of the columns.
While it would be technically possible to "read that data into another data type" (say, 4x4 matrices with int8), and then manually extract the upper 2x2 part, I don't think that this is the intended behavior here.

---

For testing:

The embedded glTF:
```
{
"asset": {
"version": "2.0"
},
"buffers" : [ {
"uri" : "data:application/gltf-buffer;base64,AQIAAAMEAAALFgAAISwAAA==",
"byteLength" : 16
} ],
"bufferViews" : [ {
"buffer" : 0,
"byteOffset" : 0,
"byteLength" : 16,
"target" : 34962
} ],
"accessors" : [ {
"bufferView" : 0,
"byteOffset" : 0,
"componentType" : 5120,
"count" : 2,
"type" : "MAT2",
"max" : [ 11, 22, 33, 44 ],
"min" : [ 1, 2, 3, 4 ]
} ]
}
```

The code for reading it:
```
#include
#include
#include

#include
#include
#include

#include
#include

std::vector readFile(const std::string &fileName) {
std::ifstream file(fileName, std::ios::binary | std::ios::ate);
if (!file) {
return std::vector();
}
std::streamsize size = file.tellg();
file.seekg(0, std::ios::beg);
std::vector buffer(static_cast(size));
file.read(reinterpret_cast(buffer.data()), size);
return buffer;
}

int main() {

// Read the model
const std::string fileName = "C:/padding_mat2_byte.gltf";
std::vector data = readFile(fileName);
const std::byte *dataBegin =
reinterpret_cast(data.data());
CesiumGltfReader::GltfReader gltfReader;
CesiumGltfReader::GltfReaderResult gltfReaderResult =
gltfReader.readGltf(gsl::span(dataBegin, data.size()));
bool success = gltfReaderResult.model.has_value();
std::cout << "Success: " << success << std::endl;
const CesiumGltf::Model gltf = gltfReaderResult.model.value();

// Print the raw data of the buffer
std::vector rawData = gltf.buffers[0].cesium.data;
for (int i=0; i<16; i++) {
std::cout << "at " << i << " buffer has " << (int32_t)rawData[i] << std::endl;
}

// Read two 2x2 matrices with 'byte' components from the accessor:
typedef glm::mat<2, 2, glm::int8, glm::defaultp> i8mat2;
const CesiumGltf::AccessorView accessorView(gltf, 0);
const i8mat2 element0 = accessorView[0];
const i8mat2 element1 = accessorView[1];

std::cout << "element0 " << glm::to_string(element0) << std::endl;
std::cout << "element1 " << glm::to_string(element1) << std::endl;
}
```

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.