From 6408799b5d876492ed71137914aa6eb3b046c2e8 Mon Sep 17 00:00:00 2001 From: baldurk Date: Thu, 17 Oct 2024 11:48:08 +0100 Subject: [PATCH] Allow specifying scalar pointers in buffer formatter --- docs/how/how_buffer_format.rst | 7 ++ qrenderdoc/Code/BufferFormatter.cpp | 165 ++++++++++++++++++++++++++-- qrenderdoc/Code/QRDUtils.cpp | 16 ++- 3 files changed, 177 insertions(+), 11 deletions(-) diff --git a/docs/how/how_buffer_format.rst b/docs/how/how_buffer_format.rst index a1659ea5a..e4cbec73a 100644 --- a/docs/how/how_buffer_format.rst +++ b/docs/how/how_buffer_format.rst @@ -126,6 +126,13 @@ On APIs where GPU pointers can reside within memory, such as Vulkan, pointers ca MyStructName *pointer; +It is also possible to declare pointers to basic types without a struct, which will be treated as tightly packed. + +.. code:: c++ + + float *pointer; + int2 *otherPointer; + Packing and layout rules ------------------------ diff --git a/qrenderdoc/Code/BufferFormatter.cpp b/qrenderdoc/Code/BufferFormatter.cpp index 4216895ee..b28488ef4 100644 --- a/qrenderdoc/Code/BufferFormatter.cpp +++ b/qrenderdoc/Code/BufferFormatter.cpp @@ -551,7 +551,11 @@ ParsedFormat BufferFormatter::ParseFormatString(const QString &formatString, uin ")" // end of the type group "(?[1-9])?" // might be a vector "(?x[1-9])?" // or a matrix - "(?\\s+[A-Za-z@_][A-Za-z0-9@_]*)?" // get identifier name + "(" // pointer or space + "(?\\s*\\*\\s*)|" // pointer asterisk + "\\s+" // or just some whitespace + ")" // end pointer or space + "(?[A-Za-z@_][A-Za-z0-9@_]*)?" // get identifier name "(?\\s*\\[[0-9]*\\])?" // optional array dimension "(\\s*:\\s*" // optional specifier after : "(" // bitfield or semantic @@ -1202,13 +1206,14 @@ ParsedFormat BufferFormatter::ParseFormatString(const QString &formatString, uin QRegularExpressionMatch structMatch = structUseRegex.match(decl); bool isPadding = false; + bool isPointer = false; if(structMatch.hasMatch() && structelems.contains(structMatch.captured(1))) { StructFormatData &structContext = structelems[structMatch.captured(1)]; QString pointerStars = structMatch.captured(2).trimmed(); - bool isPointer = !pointerStars.isEmpty(); + isPointer = !pointerStars.isEmpty(); if(pointerStars.count() > 1) { @@ -1520,6 +1525,7 @@ ParsedFormat BufferFormatter::ParseFormatString(const QString &formatString, uin QString arrayDim = !match.captured(lit("array")).isEmpty() ? match.captured(lit("array")).trimmed() : lit("[1]"); + isPointer = !match.captured(lit("ptr")).isEmpty(); { bool isArray = !arrayDim.isEmpty(); @@ -1879,6 +1885,12 @@ ParsedFormat BufferFormatter::ParseFormatString(const QString &formatString, uin success = false; break; } + if(isPointer) + { + reportError(tr("Pointers can't be packed into a bitfield.")); + success = false; + break; + } if(el.type.flags & (ShaderVariableFlags::R10G10B10A2 | ShaderVariableFlags::R11G11B10 | ShaderVariableFlags::UNorm | ShaderVariableFlags::SNorm)) { @@ -1910,7 +1922,7 @@ ParsedFormat BufferFormatter::ParseFormatString(const QString &formatString, uin bool(el.type.flags & (ShaderVariableFlags::R10G10B10A2 | ShaderVariableFlags::R11G11B10)); // normally the array stride is the size of an element - const uint32_t elAlignment = packed32bit ? sizeof(uint32_t) : GetAlignment(pack, el); + uint32_t elAlignment = packed32bit ? sizeof(uint32_t) : GetAlignment(pack, el); const uint8_t vecSize = (el.type.rows > 1 && el.type.ColMajor()) ? el.type.rows : el.type.columns; @@ -1939,6 +1951,29 @@ ParsedFormat BufferFormatter::ParseFormatString(const QString &formatString, uin el.type.arrayByteStride *= el.type.columns; } + if(isPointer) + { + ShaderConstant innerConst; + innerConst.type = el.type; + // array is consumed by the pointer, not the inner type + innerConst.type.elements = 1; + innerConst.name = el.name; + ShaderConstantType inner; + inner.name = ""; + inner.members.push_back(innerConst); + + el.type.pointerTypeID = PointerTypeRegistry::GetTypeID(inner); + + el.type.rows = 1; + el.type.columns = 1; + el.type.baseType = VarType::GPUPointer; + el.type.flags = ShaderVariableFlags::HexDisplay; + el.type.arrayByteStride = elAlignment = 8; + if(!pack.tight_arrays) + el.type.arrayByteStride = std::max(16U, el.type.arrayByteStride); + el.type.matrixByteStride = el.type.arrayByteStride; + } + if(el.bitFieldSize > 0) { // we can use the elAlignment since this is a scalar so no vector/arrays, this is just the @@ -2682,7 +2717,9 @@ QString BufferFormatter::DeclareStruct(Packing::Rules pack, ResourceId shader, QString ret; - ret = lit("struct %1\n{\n").arg(MakeIdentifierName(name)); + // don't declare outer struct for scalar-wrapped structs (generated by 'float *foo' type declarations) + if(!name.isEmpty() && members.size() == 1) + ret = lit("struct %1\n{\n").arg(MakeIdentifierName(name)); ret += innerSkippedPrefixString; @@ -2813,8 +2850,10 @@ QString BufferFormatter::DeclareStruct(Packing::Rules pack, ResourceId shader, if(members[i].bitFieldSize > 0) bitfieldSize = QFormatStr(" : %1").arg(members[i].bitFieldSize); - ret += - QFormatStr(" %1 %2%3%4;\n").arg(varTypeName).arg(varName).arg(arraySize).arg(bitfieldSize); + // don't declare outer struct for scalar-wrapped structs (generated by 'float *foo' type declarations) + if(!name.isEmpty() && members.size() == 1) + ret += lit(" "); + ret += QFormatStr("%1 %2%3%4;\n").arg(varTypeName).arg(varName).arg(arraySize).arg(bitfieldSize); } // if we don't have tight arrays, struct byte strides are always 16-byte aligned @@ -2833,7 +2872,9 @@ QString BufferFormatter::DeclareStruct(Packing::Rules pack, ResourceId shader, ret = lit("// Unexpected size of struct %1\n%2").arg(requiredByteStride).arg(ret); } - ret += lit("}\n"); + // don't declare outer struct for scalar-wrapped structs (generated by 'float *foo' type declarations) + if(!name.isEmpty() && members.size() == 1) + ret += lit("}\n"); return declarations + ret; } @@ -3598,7 +3639,13 @@ QString TypeString(const ShaderVariable &v, const ShaderConstant &c) } if(v.type == VarType::GPUPointer) - return PointerTypeRegistry::GetTypeDescriptor(v.GetPointer()).name + "*"; + { + const ShaderConstantType &typeDescriptor = PointerTypeRegistry::GetTypeDescriptor(v.GetPointer()); + // for scalar-wrapped structs (generated by 'float *foo' type declarations) use the inner typename + if(typeDescriptor.name.empty() && typeDescriptor.members.size() == 1) + return typeDescriptor.members[0].type.name + "*"; + return typeDescriptor.name + "*"; + } QString typeStr = ToQStr(v.type); @@ -5104,6 +5151,108 @@ int count; CHECK(ptrType.members[1].name == "b"); CHECK((ptrType.members[1].type == float_type)); + def = R"( +int *a; +float *b; +row_major float3x3 *c; +column_major float3x3 *d; +[[rgb]] xbyte3 *e; +ulong2 *arr[3]; +)"; + + parsed = BufferFormatter::ParseFormatString(def, 0, true); + + CHECK(parsed.errors.isEmpty()); + REQUIRE(parsed.fixed.type.members.size() == 6); + for(int i = 0; i < parsed.fixed.type.members.size(); i++) + { + CHECK(parsed.fixed.type.members[i].type.baseType == VarType::GPUPointer); + CHECK(parsed.fixed.type.members[i].type.arrayByteStride == 8); + if(parsed.fixed.type.members[i].name != "arr") + CHECK(parsed.fixed.type.members[i].type.elements == 1); + } + CHECK(parsed.fixed.type.members[5].type.elements == 3); + + { + ShaderConstantType innerType; + + int i = 0; + + innerType = + PointerTypeRegistry::GetTypeDescriptor(parsed.fixed.type.members[i].type.pointerTypeID); + REQUIRE(innerType.members.size() == 1); + CHECK(innerType.name.isEmpty()); + CHECK(innerType.members[0].name == parsed.fixed.type.members[i].name); + innerType = innerType.members[0].type; + CHECK(innerType.baseType == VarType::SInt); + CHECK(innerType.rows == 1); + CHECK(innerType.columns == 1); + + i++; + + innerType = + PointerTypeRegistry::GetTypeDescriptor(parsed.fixed.type.members[i].type.pointerTypeID); + REQUIRE(innerType.members.size() == 1); + CHECK(innerType.name.isEmpty()); + CHECK(innerType.members[0].name == parsed.fixed.type.members[i].name); + innerType = innerType.members[0].type; + CHECK(innerType.baseType == VarType::Float); + CHECK(innerType.rows == 1); + CHECK(innerType.columns == 1); + + i++; + + innerType = + PointerTypeRegistry::GetTypeDescriptor(parsed.fixed.type.members[i].type.pointerTypeID); + REQUIRE(innerType.members.size() == 1); + CHECK(innerType.name.isEmpty()); + CHECK(innerType.members[0].name == parsed.fixed.type.members[i].name); + innerType = innerType.members[0].type; + CHECK(innerType.baseType == VarType::Float); + CHECK(innerType.rows == 3); + CHECK(innerType.columns == 3); + CHECK(innerType.flags & ShaderVariableFlags::RowMajorMatrix); + + i++; + + innerType = + PointerTypeRegistry::GetTypeDescriptor(parsed.fixed.type.members[i].type.pointerTypeID); + REQUIRE(innerType.members.size() == 1); + CHECK(innerType.name.isEmpty()); + CHECK(innerType.members[0].name == parsed.fixed.type.members[i].name); + innerType = innerType.members[0].type; + CHECK(innerType.baseType == VarType::Float); + CHECK(innerType.rows == 3); + CHECK(innerType.columns == 3); + CHECK(innerType.flags == ShaderVariableFlags::NoFlags); + + i++; + + innerType = + PointerTypeRegistry::GetTypeDescriptor(parsed.fixed.type.members[i].type.pointerTypeID); + REQUIRE(innerType.members.size() == 1); + CHECK(innerType.name.isEmpty()); + CHECK(innerType.members[0].name == parsed.fixed.type.members[i].name); + innerType = innerType.members[0].type; + CHECK(innerType.baseType == VarType::UByte); + CHECK(innerType.rows == 1); + CHECK(innerType.columns == 3); + CHECK(innerType.flags & ShaderVariableFlags::HexDisplay); + CHECK(innerType.flags & ShaderVariableFlags::RGBDisplay); + + i++; + + innerType = + PointerTypeRegistry::GetTypeDescriptor(parsed.fixed.type.members[i].type.pointerTypeID); + REQUIRE(innerType.members.size() == 1); + CHECK(innerType.name.isEmpty()); + CHECK(innerType.members[0].name == parsed.fixed.type.members[i].name); + innerType = innerType.members[0].type; + CHECK(innerType.baseType == VarType::ULong); + CHECK(innerType.rows == 1); + CHECK(innerType.columns == 2); + } + def = R"( struct inner { diff --git a/qrenderdoc/Code/QRDUtils.cpp b/qrenderdoc/Code/QRDUtils.cpp index f31367d87..f380d3085 100644 --- a/qrenderdoc/Code/QRDUtils.cpp +++ b/qrenderdoc/Code/QRDUtils.cpp @@ -1098,9 +1098,19 @@ bool RichResourceTextMouseEvent(const QWidget *owner, const QVariant &var, QRect QString formatter; if(!ptrType.members.isEmpty()) - formatter = BufferFormatter::DeclareStruct( - BufferFormatter::EstimatePackingRules(ResourceId(), ptrType.members), ResourceId(), - ptrType.name, ptrType.members, ptrType.arrayByteStride); + { + Packing::Rules pack = + BufferFormatter::EstimatePackingRules(ResourceId(), ptrType.members); + + // for scalar-wrapped structs (generated by 'float *foo' type declarations) use scalar + // packing by default as that's probably what people will want most often and if we + // guess we're going to guess very conservatively. + if(ptrType.name.isEmpty() && ptrType.members.size() == 1) + pack = Packing::Scalar; + + formatter = BufferFormatter::DeclareStruct(pack, ResourceId(), ptrType.name, + ptrType.members, ptrType.arrayByteStride); + } IBufferViewer *view = ctx.ViewBuffer(ptr->offset, ~0ULL, ptr->base, formatter);