Scarfski/filament
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

enforce offsets to be a multiple of 4 bytes

Browse Source 
this affects VerteBuffer, BufferObject and IndexBuffer, this restriction
already existed in the Metal backend, it is now promoted to all
backends.
This commit is contained in:
Mathias Agopian
2025-05-16 11:27:02 -07:00
committed by Mathias Agopian
parent 6481fa2acf
commit 79d6f46f1a
7 changed files with 92 additions and 49 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
filament/backend/include/backend/DriverEnums.h
View File

@@ -648,6 +648,15 @@ enum class BufferObjectBinding : uint8_t {
SHADER_STORAGE
};
constexpr std::string_view to_string(BufferObjectBinding type) noexcept {
switch (type) {
case BufferObjectBinding::VERTEX: return "VERTEX";
case BufferObjectBinding::UNIFORM: return "UNIFORM";
case BufferObjectBinding::SHADER_STORAGE: return "SHADER_STORAGE";
}
return "UNKNOWN";
}
//! Face culling Mode
enum class CullingMode : uint8_t {
NONE, //!< No culling, front and back faces are visible

2
filament/include/filament/BufferObject.h
View File

@@ -130,7 +130,7 @@ public:
*
* @param engine Reference to the filament::Engine associated with this BufferObject.
* @param buffer A BufferDescriptor representing the data used to initialize the BufferObject.
* @param byteOffset Offset in bytes into the BufferObject
* @param byteOffset Offset in bytes into the BufferObject. Must be multiple of 4.
*/
void setBuffer(Engine& engine, BufferDescriptor&& buffer, uint32_t byteOffset = 0);

2
filament/include/filament/IndexBuffer.h
View File

@@ -137,7 +137,7 @@ public:
* @param buffer A BufferDescriptor representing the data used to initialize the IndexBuffer.
* BufferDescriptor points to raw, untyped data that will be interpreted as
* either 16-bit or 32-bits indices based on the Type of this IndexBuffer.
* @param byteOffset Offset in *bytes* into the IndexBuffer
* @param byteOffset Offset in *bytes* into the IndexBuffer. Must be multiple of 4.
*/
void setBuffer(Engine& engine, BufferDescriptor&& buffer, uint32_t byteOffset = 0);

2
filament/include/filament/VertexBuffer.h
View File

@@ -221,7 +221,7 @@ public:
* index \p bufferIndex. BufferDescriptor points to raw, untyped data that will
* be copied as-is into the buffer.
* @param byteOffset Offset in *bytes* into the buffer at index \p bufferIndex of this vertex
* buffer set.
* buffer set. Must be multiple of 4.
*/
void setBufferAt(Engine& engine, uint8_t bufferIndex, BufferDescriptor&& buffer,
uint32_t byteOffset = 0);

14
filament/src/details/BufferObject.cpp
View File

@@ -20,9 +20,19 @@
#include "FilamentAPI-impl.h"
#include <backend/DriverEnums.h>
#include <filament/BufferObject.h>
#include <utils/CString.h>
#include <utils/Panic.h>
#include <utils/StaticString.h>
#include <utility>
#include <stdint.h>
#include <stddef.h>
namespace filament {
struct BufferObject::BuilderDetails {
@@ -78,6 +88,10 @@ void FBufferObject::terminate(FEngine& engine) {
}
void FBufferObject::setBuffer(FEngine& engine, BufferDescriptor&& buffer, uint32_t const byteOffset) {
FILAMENT_CHECK_PRECONDITION((byteOffset & 0x3) == 0)
<< "byteOffset must be a multiple of 4";
engine.getDriverApi().updateBufferObject(mHandle, std::move(buffer), byteOffset);
}

22
filament/src/details/IndexBuffer.cpp
View File

@@ -15,14 +15,22 @@
*/
#include "details/IndexBuffer.h"
#include "details/Engine.h"
#include "FilamentAPI-impl.h"
#include "filament/FilamentAPI.h"
#include <backend/DriverEnums.h>
#include <utils/CString.h>
#include <utils/Panic.h>
#include <utils/StaticString.h>
#include <utility>
#include <stdint.h>
#include <stddef.h>
namespace filament {
struct IndexBuffer::BuilderDetails {
@@ -64,17 +72,19 @@ IndexBuffer* IndexBuffer::Builder::build(Engine& engine) {
FIndexBuffer::FIndexBuffer(FEngine& engine, const Builder& builder)
: mIndexCount(builder->mIndexCount) {
auto& name = builder.getName();
auto name = builder.getName();
const char* const tag = name.empty() ? "(no tag)" : name.c_str_safe();
FILAMENT_CHECK_PRECONDITION(
builder->mIndexType == IndexType::UINT || builder->mIndexType == IndexType::USHORT)
<< "Invalid index type " << static_cast<int>(builder->mIndexType) << ", tag=" << tag;
FEngine::DriverApi& driver = engine.getDriverApi();
mHandle = driver.createIndexBuffer(
(backend::ElementType)builder->mIndexType,
backend::ElementType(builder->mIndexType),
uint32_t(builder->mIndexCount),
backend::BufferUsage::STATIC);
if (auto name = builder.getName(); !name.empty()) {
if (!name.empty()) {
driver.setDebugTag(mHandle.getId(), std::move(name));
}
}
@@ -85,6 +95,10 @@ void FIndexBuffer::terminate(FEngine& engine) {
}
void FIndexBuffer::setBuffer(FEngine& engine, BufferDescriptor&& buffer, uint32_t const byteOffset) {
FILAMENT_CHECK_PRECONDITION((byteOffset & 0x3) == 0)
<< "byteOffset must be a multiple of 4";
engine.getDriverApi().updateIndexBuffer(mHandle, std::move(buffer), byteOffset);
}

90
filament/src/details/VertexBuffer.cpp
View File

@@ -96,7 +96,7 @@ VertexBuffer::Builder& VertexBuffer::Builder::attribute(VertexAttribute const at
size_t const attributeSize = Driver::getElementTypeSize(attributeType);
if (byteStride == 0) {
byteStride = (uint8_t)attributeSize;
byteStride = uint8_t(attributeSize);
}
if (size_t(attribute) < MAX_VERTEX_ATTRIBUTE_COUNT &&
@@ -181,16 +181,16 @@ VertexBuffer* VertexBuffer::Builder::build(Engine& engine) {
// also checks that we don't use an invalid type with integer attributes
if (attributes[j].flags & Attribute::FLAG_INTEGER_TARGET) {
using ET = ElementType;
constexpr uint32_t const invalidIntegerTypes =
(1 << (int)ET::FLOAT) |
(1 << (int)ET::FLOAT2) |
(1 << (int)ET::FLOAT3) |
(1 << (int)ET::FLOAT4) |
(1 << (int)ET::HALF) |
(1 << (int)ET::HALF2) |
(1 << (int)ET::HALF3) |
(1 << (int)ET::HALF4);
FILAMENT_CHECK_PRECONDITION(!(invalidIntegerTypes & (1 << (int)attributes[j].type)))
constexpr uint32_t invalidIntegerTypes =
(1 << int(ET::FLOAT)) |
(1 << int(ET::FLOAT2)) |
(1 << int(ET::FLOAT3)) |
(1 << int(ET::FLOAT4)) |
(1 << int(ET::HALF)) |
(1 << int(ET::HALF2)) |
(1 << int(ET::HALF3)) |
(1 << int(ET::HALF4));
FILAMENT_CHECK_PRECONDITION(!(invalidIntegerTypes & (1 << int(attributes[j].type))))
<< "invalid integer vertex attribute type " << int(attributes[j].type);
}
});
@@ -270,7 +270,7 @@ FVertexBuffer::FVertexBuffer(FEngine& engine, const Builder& builder)
mHandle = driver.createVertexBuffer(mVertexCount, mVertexBufferInfoHandle);
if (auto name = builder.getName(); !name.empty()) {
driver.setDebugTag(mHandle.getId(), name);
driver.setDebugTag(mHandle.getId(), std::move(name));
}
// calculate buffer sizes
@@ -297,10 +297,10 @@ FVertexBuffer::FVertexBuffer(FEngine& engine, const Builder& builder)
if (i != Attribute::BUFFER_UNUSED) {
assert_invariant(bufferSizes[i] > 0);
if (!mBufferObjects[i]) {
BufferObjectHandle bo = driver.createBufferObject(bufferSizes[i],
BufferObjectHandle const bo = driver.createBufferObject(bufferSizes[i],
BufferObjectBinding::VERTEX, BufferUsage::STATIC);
if (auto name = builder.getName(); !name.empty()) {
driver.setDebugTag(bo.getId(), name);
driver.setDebugTag(bo.getId(), std::move(name));
}
driver.setVertexBufferObject(mHandle, i, bo);
mBufferObjects[i] = bo;
@@ -311,7 +311,7 @@ FVertexBuffer::FVertexBuffer(FEngine& engine, const Builder& builder)
// in advanced skinning mode, we manage the BONE_INDICES and BONE_WEIGHTS arrays ourselves,
// so we have to set the corresponding buffer objects.
if (mAdvancedSkinningEnabled) {
for (auto index : { BONE_INDICES, BONE_WEIGHTS }) {
for (auto const index : { BONE_INDICES, BONE_WEIGHTS }) {
size_t const i = mAttributes[index].buffer;
assert_invariant(i != Attribute::BUFFER_UNUSED);
assert_invariant(bufferSizes[i] > 0);
@@ -319,7 +319,7 @@ FVertexBuffer::FVertexBuffer(FEngine& engine, const Builder& builder)
BufferObjectHandle const bo = driver.createBufferObject(bufferSizes[i],
BufferObjectBinding::VERTEX, BufferUsage::STATIC);
if (auto name = builder.getName(); !name.empty()) {
driver.setDebugTag(bo.getId(), name);
driver.setDebugTag(bo.getId(), std::move(name));
}
driver.setVertexBufferObject(mHandle, i, bo);
mBufferObjects[i] = bo;
@@ -332,7 +332,7 @@ FVertexBuffer::FVertexBuffer(FEngine& engine, const Builder& builder)
void FVertexBuffer::terminate(FEngine& engine) {
FEngine::DriverApi& driver = engine.getDriverApi();
if (!mBufferObjectsEnabled) {
for (BufferObjectHandle bo : mBufferObjects) {
for (BufferObjectHandle const& bo : mBufferObjects) {
driver.destroyBufferObject(bo);
}
}
@@ -346,32 +346,38 @@ size_t FVertexBuffer::getVertexCount() const noexcept {
void FVertexBuffer::setBufferAt(FEngine& engine, uint8_t const bufferIndex,
backend::BufferDescriptor&& buffer, uint32_t const byteOffset) {
FILAMENT_CHECK_PRECONDITION(!mBufferObjectsEnabled) << "Please use setBufferObjectAt()";
if (bufferIndex < mBufferCount) {
assert_invariant(mBufferObjects[bufferIndex]);
engine.getDriverApi().updateBufferObject(mBufferObjects[bufferIndex],
std::move(buffer), byteOffset);
} else {
FILAMENT_CHECK_PRECONDITION(bufferIndex < mBufferCount)
<< "bufferIndex must be < bufferCount";
}
FILAMENT_CHECK_PRECONDITION(!mBufferObjectsEnabled)
<< "buffer objects enabled, use setBufferObjectAt() instead";
FILAMENT_CHECK_PRECONDITION(bufferIndex < mBufferCount)
<< "bufferIndex must be < bufferCount";
FILAMENT_CHECK_PRECONDITION((byteOffset & 0x3) == 0)
<< "byteOffset must be a multiple of 4";
engine.getDriverApi().updateBufferObject(mBufferObjects[bufferIndex],
std::move(buffer), byteOffset);
}
void FVertexBuffer::setBufferObjectAt(FEngine& engine, uint8_t const bufferIndex,
FBufferObject const * bufferObject) {
FILAMENT_CHECK_PRECONDITION(mBufferObjectsEnabled) << "Please use setBufferAt()";
FILAMENT_CHECK_PRECONDITION(mBufferObjectsEnabled)
<< "buffer objects disabled, use setBufferAt() instead";
FILAMENT_CHECK_PRECONDITION(bufferObject->getBindingType() == BufferObject::BindingType::VERTEX)
<< "Binding type must be VERTEX.";
if (bufferIndex < mBufferCount) {
auto hwBufferObject = bufferObject->getHwHandle();
engine.getDriverApi().setVertexBufferObject(mHandle, bufferIndex, hwBufferObject);
// store handle to recreate VertexBuffer in the case extra bone indices and weights definition
// used only in buffer object mode
mBufferObjects[bufferIndex] = hwBufferObject;
} else {
FILAMENT_CHECK_PRECONDITION(bufferIndex < mBufferCount)
<< "bufferIndex must be < bufferCount";
}
<< "bufferObject binding type must be VERTEX but is "
<< to_string(bufferObject->getBindingType());
FILAMENT_CHECK_PRECONDITION(bufferIndex < mBufferCount)
<< "bufferIndex must be < bufferCount";
auto const hwBufferObject = bufferObject->getHwHandle();
engine.getDriverApi().setVertexBufferObject(mHandle, bufferIndex, hwBufferObject);
// store handle to recreate VertexBuffer in the case extra bone indices and weights definition
// used only in buffer object mode
mBufferObjects[bufferIndex] = hwBufferObject;
}
void FVertexBuffer::updateBoneIndicesAndWeights(FEngine& engine,
@@ -381,15 +387,15 @@ void FVertexBuffer::updateBoneIndicesAndWeights(FEngine& engine,
auto jointsData = skinJoints.release();
uint8_t const indicesIndex = mAttributes[BONE_INDICES].buffer;
engine.getDriverApi().updateBufferObject(mBufferObjects[indicesIndex],
{jointsData, mVertexCount * 8,
[](void* buffer, size_t, void*) { delete[] static_cast<uint16_t*>(buffer); }},
{ jointsData, mVertexCount * 8,
[](void* buffer, size_t, void*) { delete[] static_cast<uint16_t*>(buffer); } },
0);
auto weightsData = skinWeights.release();
uint8_t const weightsIndex = mAttributes[BONE_WEIGHTS].buffer;
engine.getDriverApi().updateBufferObject(mBufferObjects[weightsIndex],
{weightsData, mVertexCount * 16,
[](void* buffer, size_t, void*) { delete[] static_cast<float*>(buffer); }},
{ weightsData, mVertexCount * 16,
[](void* buffer, size_t, void*) { delete[] static_cast<float*>(buffer); } },
0);
}