Skip to content

Commit

Permalink
Fix materialData array alignment issues.
Browse files Browse the repository at this point in the history
  • Loading branch information
Flone-dnb committed Sep 20, 2024
1 parent 50162a2 commit 93f7250
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 14 deletions.
9 changes: 7 additions & 2 deletions res/engine/shaders/include/data/MaterialData.glsl
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,13 @@ struct MaterialData {
/** Defines how much specular light will be reflected (value in range [0.0F; 1.0F]). */
float roughness;

/** Padding for alignment simplicity. */
vec3 pad;
/** Explicit padding for clarity. */
/** `vec3` does not work here because GLSL requires 16 bytes of alignment of `vec3`s but we have 4 byte alignment here. */
float pad1;
// silence doc checker
float pad2;
// silence doc checker
float pad3;
};

/** Material data for all meshes, use indices from root/push constants to index into this array. */
Expand Down
14 changes: 7 additions & 7 deletions src/engine_lib/private/material/Material.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -421,14 +421,14 @@ namespace ne {
// Set flag before adding binding data.
bIsShaderResourcesAllocated = true;

// Set material constant buffer.
// Set material data binding.
setShaderCpuWriteResourceBindingData(
sMaterialShaderConstantBufferName,
sMaterialShaderBufferName,
sizeof(MaterialShaderConstants),
[this]() -> void* { return onStartUpdatingShaderMeshConstants(); },
[this]() { onFinishedUpdatingShaderMeshConstants(); });

// Set diffuse texture (if path is set).
// Set diffuse texture binding (if path is set).
if (!sDiffuseTexturePathRelativeRes.empty()) {
setShaderTextureResourceBindingData(
sMaterialShaderDiffuseTextureName, sDiffuseTexturePathRelativeRes);
Expand Down Expand Up @@ -641,7 +641,7 @@ namespace ne {
mtxShaderMaterialDataConstants.second.diffuseColor.y = diffuseColor.y;
mtxShaderMaterialDataConstants.second.diffuseColor.z = diffuseColor.z;

markShaderCpuWriteResourceAsNeedsUpdate(sMaterialShaderConstantBufferName);
markShaderCpuWriteResourceAsNeedsUpdate(sMaterialShaderBufferName);
}

void Material::setSpecularColor(const glm::vec3& specularColor) {
Expand All @@ -655,7 +655,7 @@ namespace ne {
mtxShaderMaterialDataConstants.second.specularColor.y = specularColor.y;
mtxShaderMaterialDataConstants.second.specularColor.z = specularColor.z;

markShaderCpuWriteResourceAsNeedsUpdate(sMaterialShaderConstantBufferName);
markShaderCpuWriteResourceAsNeedsUpdate(sMaterialShaderBufferName);
}

void Material::setRoughness(float roughness) {
Expand All @@ -667,7 +667,7 @@ namespace ne {
// Save new opacity for shaders.
mtxShaderMaterialDataConstants.second.roughness = this->roughness;

markShaderCpuWriteResourceAsNeedsUpdate(sMaterialShaderConstantBufferName);
markShaderCpuWriteResourceAsNeedsUpdate(sMaterialShaderBufferName);
}

void Material::setOpacity(float opacity) {
Expand All @@ -679,7 +679,7 @@ namespace ne {
// Save new opacity for shaders.
mtxShaderMaterialDataConstants.second.diffuseColor.w = this->opacity;

markShaderCpuWriteResourceAsNeedsUpdate(sMaterialShaderConstantBufferName);
markShaderCpuWriteResourceAsNeedsUpdate(sMaterialShaderBufferName);
}

std::string Material::getVertexShaderName() const { return sVertexShaderName; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ namespace ne {

// Create a new buffer.
auto result = pResourceManager->createResourceWithCpuWriteAccess(
std::format("\"{}\" GPU array", sHandledShaderResourceName),
std::format("\"{}\" CPU-write dynamic array", sHandledShaderResourceName),
iElementSizeInBytes,
iCapacity,
true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,9 @@ namespace ne {
return Error("shader resource array manager is `nullptr`");
}

// Reserve a space for this shader resource's data per frame resource.
// Reserve a space for this shader resource's data per frame resource
// (one slot per frame in-flight because we operate on CPU-write resources
// so we have to keep N copies of the data in case it's changing to avoid stopping the GPU).
for (unsigned int i = 0; i < FrameResourceManager::getFrameResourceCount(); i++) {
auto result = pShaderResourceArrayManager->reserveSlotsInArray(pShaderResource.get());
if (std::holds_alternative<Error>(result)) [[unlikely]] {
Expand Down
6 changes: 3 additions & 3 deletions src/engine_lib/public/material/Material.h
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ namespace ne RNAMESPACE() {
/** Defines how much specular light will be reflected. */
alignas(iVkScalarAlignment) float roughness = 0.0F;

/** Padding for alignment simplicity. */
/** Explicit padding for clarity. */
float pad[3];
};

Expand Down Expand Up @@ -665,8 +665,8 @@ namespace ne RNAMESPACE() {
/** Whether @ref allocateShaderResources was called or not. */
bool bIsShaderResourcesAllocated = false;

/** Name of the constant buffer used to store material data in shaders. */
static inline const auto sMaterialShaderConstantBufferName = "materialData";
/** Name of the buffer used to store material data in shaders. */
static inline const auto sMaterialShaderBufferName = "materialData";

/** Name of the resource used to store diffuse texture in shaders. */
static inline const auto sMaterialShaderDiffuseTextureName = "diffuseTexture";
Expand Down

0 comments on commit 93f7250

Please sign in to comment.