webgpu: Clean up spec constants and fix validation for unused attributes (#9933)
- WebGPUProgram.cpp: Remove legacy replaceSpecConstants string injection since WebGPU handles specialization constants natively. - WebGPUVertexBufferInfo.cpp: Pad unused vertex attributes safely into slot 0 using 4-byte formats to satisfy WebGPU stride validation. - CodeGenerator.cpp: Remove legacy fallback generating 'const' structs for specialization constants. - ShaderGenerator.cpp: Unset unused skinning/morphing attributes to prevent WebGPU validation crashes. - dawn/tnt/CMakeLists.txt: Fix JNI linker error by dropping missing Tint AST reader libraries and adding tint_utils_system. This commit fixes combination use of ubershader and skinning.
This commit is contained in:
Powei Feng
GitHub
@@ -43,101 +43,6 @@ namespace filament::backend {
|
||||
|
||||
namespace {
|
||||
|
||||
/**
|
||||
* Given the source code of a [WGSL] WebGPU shader in text (string view to it) and the constants
|
||||
* to be overwritten now (at runtime), returns updated [WGSL] shader code text, where the constants
|
||||
* have been replaced.
|
||||
* @param shaderLabel Something to call this shader for troubleshooting, error messaging, etc.
|
||||
* @param shaderSource The original WGSL WebGPU shader code as text to be processed. This is a
|
||||
* view to that text and this function does not change it; it is immutable.
|
||||
* Instead, the processed version of this source is returned by the function.
|
||||
* @param specConstants The constants to replace in the shader code, indexed by constant id.
|
||||
* @return Processed version of the WGSL WebGPU shader code provided, where the constants have
|
||||
* been replaced with the values provided by the specConstants parameter.
|
||||
*/
|
||||
[[nodiscard]] std::string replaceSpecConstants(std::string_view shaderLabel,
|
||||
std::string_view shaderSource,
|
||||
utils::FixedCapacityVector<Program::SpecializationConstant> const& specConstants) {
|
||||
// this function is not expected to be called at all if no spec constants are to be replaced
|
||||
assert_invariant(!specConstants.empty());
|
||||
static constexpr std::string_view specConstantPrefix = "FILAMENT_SPEC_CONST_";
|
||||
static constexpr size_t specConstantPrefixSize = specConstantPrefix.size();
|
||||
const char* const sourceData = shaderSource.data();
|
||||
std::stringstream processedShaderSource{};
|
||||
size_t pos = 0;
|
||||
while (pos < shaderSource.size()) {
|
||||
const size_t posOfNextSpecConstant = shaderSource.find(specConstantPrefix, pos);
|
||||
if (posOfNextSpecConstant == std::string::npos) {
|
||||
// no more spec constants, so just stream the rest of the source code string
|
||||
processedShaderSource << std::string_view(sourceData + pos, shaderSource.size() - pos);
|
||||
break;
|
||||
}
|
||||
const size_t posOfId = posOfNextSpecConstant + specConstantPrefixSize;
|
||||
const size_t posAfterId = shaderSource.find('_', posOfId);
|
||||
FILAMENT_CHECK_POSTCONDITION(posAfterId != std::string::npos)
|
||||
<< "malformed " << shaderLabel << ". Found spec constant prefix '"
|
||||
<< specConstantPrefix << "' without an id or '_' after it.";
|
||||
const std::string_view idStr =
|
||||
std::string_view(sourceData + posOfId, posAfterId - posOfId);
|
||||
const size_t posEndOfStatement = shaderSource.find(';', posAfterId);
|
||||
FILAMENT_CHECK_POSTCONDITION(posEndOfStatement != std::string::npos)
|
||||
<< "malformed " << shaderLabel << ". Found spec constant assignment with id "
|
||||
<< idStr << " without a terminating ';' character?";
|
||||
// this is a view into part of the statement, from after the id to the ';'
|
||||
const std::string_view statementSegment =
|
||||
std::string_view(sourceData + posAfterId, posEndOfStatement - posAfterId);
|
||||
size_t posOfEqual = statementSegment.find('=');
|
||||
if (posOfEqual == std::string::npos) {
|
||||
// Not an assignment, so we don't need to replace it.
|
||||
processedShaderSource << std::string_view(sourceData + pos,
|
||||
posEndOfStatement + 1 - pos);
|
||||
pos = posEndOfStatement + 1;
|
||||
continue;
|
||||
}
|
||||
posOfEqual += posAfterId; // position in original source overall, not just the segment
|
||||
int constantId = 0;
|
||||
char* endPtr;
|
||||
errno = 0; // Clear errno before the call
|
||||
|
||||
long tempConstantId = std::strtol(idStr.data(), &endPtr, 10);
|
||||
|
||||
// Check for conversion errors
|
||||
if (endPtr == idStr.data() || *endPtr != '\0' || errno == ERANGE) {
|
||||
// Parsing failed, or conversion was out of range for 'long'
|
||||
// or the string contained non-numeric characters after the number.
|
||||
PANIC_POSTCONDITION("Invalid spec constant id '%s' in %s (not a valid integer or out "
|
||||
"of range for 'long'?)",
|
||||
idStr.data(), shaderLabel.data());
|
||||
} else {
|
||||
// Check if the parsed long value fits into an int
|
||||
if (tempConstantId > std::numeric_limits<int>::max() ||
|
||||
tempConstantId < std::numeric_limits<int>::min()) {
|
||||
PANIC_POSTCONDITION(
|
||||
"Invalid spec constant id '%s' in %s (value out of range for 'int')",
|
||||
idStr.data(), shaderLabel.data());
|
||||
} else {
|
||||
constantId = static_cast<int>(tempConstantId);
|
||||
}
|
||||
}
|
||||
const std::variant<int32_t, float, bool> newValue = specConstants[constantId];
|
||||
// stream up to the equal sign...
|
||||
processedShaderSource << std::string_view(sourceData + pos, posOfEqual + 1 - pos);
|
||||
// stream the new value...
|
||||
if (auto* v = std::get_if<int32_t>(&newValue)) {
|
||||
processedShaderSource << " " << *v << "i";
|
||||
} else if (auto* f = std::get_if<float>(&newValue)) {
|
||||
processedShaderSource << " " << *f << "f";
|
||||
} else if (auto* b = std::get_if<bool>(&newValue)) {
|
||||
processedShaderSource << " " << ((*b) ? "true" : "false");
|
||||
}
|
||||
// end the statement...
|
||||
processedShaderSource << ";";
|
||||
// and skip to after the end of the statement in the original source and continue...
|
||||
pos = posEndOfStatement + 1;
|
||||
}
|
||||
return processedShaderSource.str();
|
||||
}
|
||||
|
||||
/**
|
||||
* Creates a WebGPU shader module for a given "program" "stage", accounting for override constants.
|
||||
* Effectively, this function is responsible for preprocessing the shader source and compiling it.
|
||||
@@ -145,14 +50,12 @@ namespace {
|
||||
* a shader module
|
||||
* @param program The "program" to compile/create the shader, which includes the shader source
|
||||
* @param stage The stage (e.g. vertex, fragment, etc.) to create the shader module
|
||||
* @param specConstants Override constants to apply when creating/compiling the shader module.
|
||||
* @return the proper WebGPU shader module compiled/created from the input parameters. This might
|
||||
* wrap a null handle if the shader is not present (if the shader source is empty), such as
|
||||
* a missing fragment or compute shader.
|
||||
*/
|
||||
[[nodiscard]] wgpu::ShaderModule createShaderModule(wgpu::Device const& device,
|
||||
Program const& program, const ShaderStage stage,
|
||||
utils::FixedCapacityVector<Program::SpecializationConstant> const& specConstants) {
|
||||
Program const& program, const ShaderStage stage) {
|
||||
const char* const programName = program.getName().c_str_safe();
|
||||
std::array<utils::FixedCapacityVector<uint8_t>, Program::SHADER_TYPE_COUNT> const&
|
||||
shaderSource = program.getShadersSource();
|
||||
@@ -164,13 +67,9 @@ namespace {
|
||||
std::stringstream labelStream;
|
||||
labelStream << programName << " " << filamentShaderStageToString(stage) << " shader";
|
||||
const auto label = labelStream.str();
|
||||
const std::string processedShaderSource =
|
||||
specConstants.empty()
|
||||
? reinterpret_cast<const char*>(sourceBytes.data())
|
||||
: replaceSpecConstants(label, reinterpret_cast<const char*>(sourceBytes.data()),
|
||||
specConstants);
|
||||
|
||||
wgpu::ShaderSourceWGSL wgslDescriptor{};
|
||||
wgslDescriptor.code = wgpu::StringView(processedShaderSource);
|
||||
wgslDescriptor.code = wgpu::StringView(reinterpret_cast<const char*>(sourceBytes.data()));
|
||||
const wgpu::ShaderModuleDescriptor descriptor{
|
||||
.nextInChain = &wgslDescriptor,
|
||||
.label = label.data()
|
||||
@@ -251,14 +150,11 @@ namespace {
|
||||
}// namespace
|
||||
|
||||
WebGPUProgram::WebGPUProgram(wgpu::Device const& device, Program const& program)
|
||||
: HwProgram{ program.getName() } {
|
||||
utils::FixedCapacityVector<Program::SpecializationConstant> const& specConstants =
|
||||
program.getSpecializationConstants();
|
||||
: HwProgram{ program.getName() } {
|
||||
// TODO: Consider creating/compiling these shaders in parallel.
|
||||
vertexShaderModule = createShaderModule(device, program, ShaderStage::VERTEX, specConstants);
|
||||
fragmentShaderModule =
|
||||
createShaderModule(device, program, ShaderStage::FRAGMENT, specConstants);
|
||||
computeShaderModule = createShaderModule(device, program, ShaderStage::COMPUTE, specConstants);
|
||||
vertexShaderModule = createShaderModule(device, program, ShaderStage::VERTEX);
|
||||
fragmentShaderModule = createShaderModule(device, program, ShaderStage::FRAGMENT);
|
||||
computeShaderModule = createShaderModule(device, program, ShaderStage::COMPUTE);
|
||||
}
|
||||
|
||||
}// namespace filament::backend
|
||||
|
||||
@@ -225,42 +225,73 @@ struct AttributeInfo final {
|
||||
* associated with a vertex buffer, and the slot each one belongs
|
||||
*/
|
||||
void createBufferLayoutsBindingSlotsAndAttributeInfos(AttributeArray const& attributes,
|
||||
const uint8_t attributeCount, const uint32_t deviceMaxVertexBuffers,
|
||||
const uint32_t deviceMaxVertexBuffers,
|
||||
std::vector<WebGPUVertexBufferInfo::WebGPUSlotBindingInfo>& outWebGPUSlotBindingInfos,
|
||||
std::vector<wgpu::VertexBufferLayout>& outVertexBufferLayouts,
|
||||
std::array<AttributeInfo, MAX_VERTEX_ATTRIBUTE_COUNT>& outAttributeInfos) {
|
||||
std::array<AttributeInfo, MAX_VERTEX_ATTRIBUTE_COUNT>& outAttributeInfos,
|
||||
uint8_t& outActualAttributeCount) {
|
||||
uint8_t currentWebGPUSlotIndex = 0;
|
||||
uint8_t currentAttributeIndex = 0;
|
||||
outVertexBufferLayouts.reserve(MAX_VERTEX_BUFFER_COUNT);
|
||||
outWebGPUSlotBindingInfos.reserve(outVertexBufferLayouts.capacity());
|
||||
for (uint32_t attributeIndex = 0; attributeIndex < attributes.size(); ++attributeIndex) {
|
||||
const auto& attribute = attributes[attributeIndex];
|
||||
if (attribute.buffer == Attribute::BUFFER_UNUSED) {
|
||||
// We ignore "unused" attributes, ones not associated with a buffer. If a shader
|
||||
// references such an attribute we have a bug one way or another. Either the API/CPU
|
||||
// will produce an error (best case scenario), where an attribute is referenced in the
|
||||
// shader but not provided by the backend API (CPU-side), or the shader would be getting
|
||||
// junk/undefined data from the GPU, since we do not have a valid buffer of data to
|
||||
// provide to the shader/GPU.
|
||||
continue;
|
||||
|
||||
// Find a valid attribute to use as a dummy for unused slots (typically POSITION at index 0)
|
||||
Attribute dummyAttribute{};
|
||||
bool hasDummy = false;
|
||||
for (uint32_t i = 0; i < attributes.size(); ++i) {
|
||||
if (attributes[i].buffer != Attribute::BUFFER_UNUSED) {
|
||||
dummyAttribute = attributes[i];
|
||||
hasDummy = true;
|
||||
break;
|
||||
}
|
||||
const bool isInteger = attribute.flags & Attribute::FLAG_INTEGER_TARGET;
|
||||
const bool isNormalized = attribute.flags & Attribute::FLAG_NORMALIZED;
|
||||
const wgpu::VertexFormat vertexFormat =
|
||||
getVertexFormat(attribute.type, isNormalized, isInteger);
|
||||
}
|
||||
|
||||
/*
|
||||
* WebGPU Strict Validation Workaround:
|
||||
*
|
||||
* 1. Completeness: WebGPU requires ALL shader-declared attributes to exist in the Pipeline's
|
||||
* VertexState, even if unused by a specific mesh variant (e.g., BONE_WEIGHTS).
|
||||
* 2. Stride Boundary: Offset + Format_Size <= arrayStride.
|
||||
*
|
||||
* If we omit an unused attribute, WebGPU crashes (violates 1).
|
||||
* If we pad it as Float32x4 (16 bytes) into Slot 0 (e.g., Position, stride 12), WebGPU crashes
|
||||
* (violates 2):
|
||||
*
|
||||
* [ Slot 0 Stride: 12 bytes ]
|
||||
* |-------POSITION--------|
|
||||
* |------ BONE_WEIGHTS (Float32x4 = 16b) -----| <-- OVERFLOWS STRIDE!
|
||||
*
|
||||
* FIX: We alias unused attributes into Slot 0 as a 4-byte format (Float32 or Uint32).
|
||||
*
|
||||
* [ Slot 0 Stride: 12 bytes ]
|
||||
* |-------POSITION--------|
|
||||
* |-BONE-| <-- (Float32 = 4b) Safely fits inside the 12-byte stride!
|
||||
*
|
||||
* WebGPU safely promotes the 4-byte Float32 into a WGSL vec4<f32> as (x, 0.0, 0.0, 1.0).
|
||||
*/
|
||||
for (uint32_t attributeIndex = 0; attributeIndex < attributes.size(); ++attributeIndex) {
|
||||
auto attribute = attributes[attributeIndex];
|
||||
wgpu::VertexFormat vertexFormat;
|
||||
|
||||
if (attribute.buffer == Attribute::BUFFER_UNUSED) {
|
||||
if (!hasDummy) {
|
||||
continue;
|
||||
}
|
||||
// HACK: Re-use the dummy buffer for disabled attributes to satisfy WebGPU validation.
|
||||
// Filament's shaders expect vec4 or uvec4. We provide a dummy format.
|
||||
const bool isInteger = attribute.flags & Attribute::FLAG_INTEGER_TARGET;
|
||||
vertexFormat = isInteger ? wgpu::VertexFormat::Uint8x4 : wgpu::VertexFormat::Unorm8x4;
|
||||
attribute = dummyAttribute;
|
||||
} else {
|
||||
const bool isInteger = attribute.flags & Attribute::FLAG_INTEGER_TARGET;
|
||||
const bool isNormalized = attribute.flags & Attribute::FLAG_NORMALIZED;
|
||||
vertexFormat = getVertexFormat(attribute.type, isNormalized, isInteger);
|
||||
}
|
||||
|
||||
uint8_t existingSlot = INVALID_SLOT_INDEX;
|
||||
for (uint32_t slot = 0; slot < currentWebGPUSlotIndex; slot++) {
|
||||
WebGPUVertexBufferInfo::WebGPUSlotBindingInfo const& info =
|
||||
outWebGPUSlotBindingInfos[slot];
|
||||
// We consider attributes to be in the same slot/layout only if they belong to the
|
||||
// same buffer and are interleaved; they cannot belong to separate partitions in the
|
||||
// same buffer, for example.
|
||||
// For the attributes to be interleaved, the stride must match (among other things).
|
||||
// The attribute offset being less than the slot's/layout's buffer offset indicates
|
||||
// that it is in a separate partition before this slot/layout, thus not part of it.
|
||||
// The difference from the attribute's offset and this slot's/layout's buffer offset
|
||||
// being greater than the stride indicates it is in a separate partition after this
|
||||
// slot/layout, thus not part of it.
|
||||
if (info.sourceBufferIndex == attribute.buffer && info.stride == attribute.stride &&
|
||||
attribute.offset >= info.bufferOffset &&
|
||||
((attribute.offset - info.bufferOffset) < attribute.stride)) {
|
||||
@@ -269,26 +300,18 @@ void createBufferLayoutsBindingSlotsAndAttributeInfos(AttributeArray const& attr
|
||||
}
|
||||
}
|
||||
if (existingSlot == INVALID_SLOT_INDEX) {
|
||||
// New combination, use a new WebGPU slot
|
||||
FILAMENT_CHECK_PRECONDITION(currentWebGPUSlotIndex < MAX_VERTEX_BUFFER_COUNT &&
|
||||
currentWebGPUSlotIndex < deviceMaxVertexBuffers)
|
||||
<< "Number of vertex buffer layouts must not exceed MAX_VERTEX_BUFFER_COUNT ("
|
||||
<< MAX_VERTEX_BUFFER_COUNT << ") or the device limit ("
|
||||
<< deviceMaxVertexBuffers << ")";
|
||||
existingSlot = currentWebGPUSlotIndex++;
|
||||
outWebGPUSlotBindingInfos.push_back({
|
||||
.sourceBufferIndex = attribute.buffer,
|
||||
outWebGPUSlotBindingInfos.push_back({ .sourceBufferIndex = attribute.buffer,
|
||||
.bufferOffset = attribute.offset,
|
||||
.stride = attribute.stride });
|
||||
// we need to have a vertex buffer layout for each slot
|
||||
outVertexBufferLayouts.push_back({
|
||||
.stepMode = wgpu::VertexStepMode::Vertex,
|
||||
.arrayStride = attribute.stride
|
||||
// we do not know attributeCount or attributes at this time. We get those
|
||||
// in a subsequent pass over the attributeInfos we collect in this loop.
|
||||
});
|
||||
outVertexBufferLayouts.push_back(
|
||||
{ .stepMode = wgpu::VertexStepMode::Vertex, .arrayStride = attribute.stride });
|
||||
}
|
||||
// we don't use a designated initializer here because certain compilers can't handle it
|
||||
outAttributeInfos[currentAttributeIndex++] = AttributeInfo(
|
||||
existingSlot,
|
||||
{
|
||||
@@ -297,9 +320,8 @@ void createBufferLayoutsBindingSlotsAndAttributeInfos(AttributeArray const& attr
|
||||
attribute.offset - outWebGPUSlotBindingInfos[existingSlot].bufferOffset,
|
||||
.shaderLocation = attributeIndex });
|
||||
}
|
||||
FILAMENT_CHECK_POSTCONDITION(currentAttributeIndex == attributeCount)
|
||||
<< "Using " << currentAttributeIndex << " attributes, but " << attributeCount
|
||||
<< " where indicated.";
|
||||
|
||||
outActualAttributeCount = currentAttributeIndex;
|
||||
outVertexBufferLayouts.shrink_to_fit();
|
||||
outWebGPUSlotBindingInfos.shrink_to_fit();
|
||||
}
|
||||
@@ -316,21 +338,21 @@ WebGPUVertexBufferInfo::WebGPUVertexBufferInfo(const uint8_t bufferCount,
|
||||
<< ") exceeds Filament's MAX_VERTEX_ATTRIBUTE_COUNT limit ("
|
||||
<< MAX_VERTEX_ATTRIBUTE_COUNT << ") and/or the device's limit ("
|
||||
<< deviceLimits.maxVertexAttributes << ")";
|
||||
mVertexAttributes.reserve(attributeCount);
|
||||
mVertexAttributes.reserve(MAX_VERTEX_ATTRIBUTE_COUNT); // Reserve max as we might add dummies
|
||||
if (attributeCount == 0) {
|
||||
mVertexBufferLayouts.resize(0);
|
||||
mWebGPUSlotBindingInfos.resize(0);
|
||||
return; // should not be possible, but being defensive. nothing to do otherwise
|
||||
}
|
||||
std::array<AttributeInfo, MAX_VERTEX_ATTRIBUTE_COUNT> attributeInfos{};
|
||||
createBufferLayoutsBindingSlotsAndAttributeInfos(attributes, attributeCount,
|
||||
deviceLimits.maxVertexBuffers, mWebGPUSlotBindingInfos, mVertexBufferLayouts,
|
||||
attributeInfos);
|
||||
uint8_t actualAttributeCount = 0;
|
||||
createBufferLayoutsBindingSlotsAndAttributeInfos(attributes, deviceLimits.maxVertexBuffers,
|
||||
mWebGPUSlotBindingInfos, mVertexBufferLayouts, attributeInfos, actualAttributeCount);
|
||||
// sort attribute infos by increasing slot (by increasing offset within each slot).
|
||||
// We do this to ensure that attributes for the same slot/layout are contiguous in
|
||||
// the vector, so the vertex buffer layout associated with these contiguous attributes
|
||||
// can directly reference them in the mVertexAttributes vector below.
|
||||
std::sort(attributeInfos.data(), attributeInfos.data() + attributeCount,
|
||||
std::sort(attributeInfos.data(), attributeInfos.data() + actualAttributeCount,
|
||||
[](AttributeInfo const& first, AttributeInfo const& second) {
|
||||
if (first.slot < second.slot) {
|
||||
return true;
|
||||
@@ -344,7 +366,7 @@ WebGPUVertexBufferInfo::WebGPUVertexBufferInfo(const uint8_t bufferCount,
|
||||
});
|
||||
// populate mVertexAttributes and update mVertexBufferLayouts to reference the correct
|
||||
// attributes in it (which will be contiguous in memory as ensured by the sorting above)...
|
||||
for (uint32_t attributeIndex = 0; attributeIndex < attributeCount; ++attributeIndex) {
|
||||
for (uint32_t attributeIndex = 0; attributeIndex < actualAttributeCount; ++attributeIndex) {
|
||||
AttributeInfo const& info = attributeInfos[attributeIndex];
|
||||
mVertexAttributes.push_back(info.attribute);
|
||||
if (mVertexBufferLayouts[info.slot].attributeCount == 0) {
|
||||
|
||||
@@ -881,14 +881,6 @@ utils::io::sstream& CodeGenerator::generateSpecializationConstant(utils::io::sst
|
||||
|
||||
static const char* types[] = { "int", "float", "bool" };
|
||||
|
||||
// Spec constants aren't fully supported in Tint,
|
||||
// workaround until https://issues.chromium.org/issues/42250586 is resolved
|
||||
if (mTargetApi == TargetApi::WEBGPU) {
|
||||
std::string const variableName = "FILAMENT_SPEC_CONST_" + std::to_string(id) + "_" + name;
|
||||
out << " const " << types[value.index()] << " " << variableName << " = " << constantString << ";\n";
|
||||
out << types[value.index()] << " " << name << " = " << variableName << ";\n";
|
||||
return out;
|
||||
}
|
||||
if (mTargetLanguage == MaterialBuilderBase::TargetLanguage::SPIRV) {
|
||||
out << "layout (constant_id = " << id << ") const "
|
||||
<< types[value.index()] << " " << name << " = " << constantString << ";\n";
|
||||
|
||||
3
third_party/dawn/tnt/CMakeLists.txt
vendored
3
third_party/dawn/tnt/CMakeLists.txt
vendored
@@ -52,8 +52,6 @@ if (ANDROID AND FILAMENT_BUILD_FILAMAT)
|
||||
tint_lang_spirv_intrinsic
|
||||
tint_lang_spirv_ir
|
||||
tint_lang_spirv_reader
|
||||
tint_lang_spirv_reader_ast_lower
|
||||
tint_lang_spirv_reader_ast_parser
|
||||
tint_lang_spirv_reader_common
|
||||
tint_lang_spirv_reader_lower
|
||||
tint_lang_spirv_reader_parser
|
||||
@@ -77,6 +75,7 @@ if (ANDROID AND FILAMENT_BUILD_FILAMAT)
|
||||
tint_utils_rtti
|
||||
tint_utils_strconv
|
||||
tint_utils_symbol
|
||||
tint_utils_system
|
||||
tint_utils_text
|
||||
tint_utils_text_generator
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user