From 972f56d45a4c1c2933e2ab74384f94bafd1d51ba Mon Sep 17 00:00:00 2001 From: baldurk Date: Fri, 21 Oct 2022 16:21:34 +0100 Subject: [PATCH] Display type-incompatible bindings overlapping in GL in pipeline view * This is possible if two sampler uniforms point to the same slot but they are not compatible sampler types. --- .../PipelineState/GLPipelineStateViewer.cpp | 79 ++++++++++++------- .../PipelineState/GLPipelineStateViewer.h | 2 +- renderdoc/api/replay/gl_pipestate.h | 10 +++ renderdoc/driver/gl/gl_replay.cpp | 21 ++++- renderdoc/replay/renderdoc_serialise.inl | 3 +- 5 files changed, 81 insertions(+), 34 deletions(-) diff --git a/qrenderdoc/Windows/PipelineState/GLPipelineStateViewer.cpp b/qrenderdoc/Windows/PipelineState/GLPipelineStateViewer.cpp index cda37b807..93a4f1c45 100644 --- a/qrenderdoc/Windows/PipelineState/GLPipelineStateViewer.cpp +++ b/qrenderdoc/Windows/PipelineState/GLPipelineStateViewer.cpp @@ -535,34 +535,43 @@ void GLPipelineStateViewer::setEmptyRow(RDTreeWidgetItem *node) void GLPipelineStateViewer::setViewDetails(RDTreeWidgetItem *node, TextureDescription *tex, uint32_t firstMip, uint32_t numMips, uint32_t firstSlice, - uint32_t numSlices, const rdcstr &completeStatus) + uint32_t numSlices, const GLPipe::Texture *texBinding) { QString text; - if(!completeStatus.isEmpty()) - text += tr("The texture is incomplete:\n%1\n\n").arg(completeStatus); - - if((tex->mips > 1 && firstMip > 0) || numMips < tex->mips) + if(texBinding) { - if(numMips == 1) - text += tr("The texture has %1 mips, the view covers mip %2.").arg(tex->mips).arg(firstMip); - else - text += tr("The texture has %1 mips, the view covers mips %2-%3.") - .arg(tex->mips) - .arg(firstMip) - .arg(firstMip + numMips - 1); + if(!texBinding->completeStatus.isEmpty()) + text += tr("The texture is incomplete:\n%1\n\n").arg(texBinding->completeStatus); + + if(!texBinding->typeConflict.isEmpty()) + text += tr("Multiple conflicting bindings:\n%1\n\n").arg(texBinding->typeConflict); } - if((tex->arraysize > 1 && firstSlice > 0) || numSlices < tex->arraysize) + if(tex) { - if(numSlices == 1) - text += - tr("The texture has %1 slices, the view covers slice %2.").arg(tex->arraysize).arg(firstSlice); - else - text += tr("The texture has %1 slices, the view covers slices %2-%3.") - .arg(tex->arraysize) - .arg(firstSlice) - .arg(firstSlice + numSlices - 1); + if((tex->mips > 1 && firstMip > 0) || numMips < tex->mips) + { + if(numMips == 1) + text += tr("The texture has %1 mips, the view covers mip %2.").arg(tex->mips).arg(firstMip); + else + text += tr("The texture has %1 mips, the view covers mips %2-%3.") + .arg(tex->mips) + .arg(firstMip) + .arg(firstMip + numMips - 1); + } + + if((tex->arraysize > 1 && firstSlice > 0) || numSlices < tex->arraysize) + { + if(numSlices == 1) + text += + tr("The texture has %1 slices, the view covers slice %2.").arg(tex->arraysize).arg(firstSlice); + else + text += tr("The texture has %1 slices, the view covers slices %2-%3.") + .arg(tex->arraysize) + .arg(firstSlice) + .arg(firstSlice + numSlices - 1); + } } text = text.trimmed(); @@ -797,7 +806,9 @@ void GLPipelineStateViewer::setShaderState(const GLPipe::Shader &stage, RDLabel { QString slotname = QString::number(i); - if(shaderInput && !shaderInput->name.empty()) + if(!r.typeConflict.empty()) + slotname += tr(": "); + else if(shaderInput && !shaderInput->name.empty()) slotname += lit(": ") + shaderInput->name; uint32_t w = 1, h = 1, d = 1; @@ -843,18 +854,30 @@ void GLPipelineStateViewer::setShaderState(const GLPipe::Shader &stage, RDLabel } } - RDTreeWidgetItem *node = - new RDTreeWidgetItem({slotname, r.resourceId, typeName, w, h, d, a, format, QString()}); + RDTreeWidgetItem *node = NULL; - node->setTag(QVariant::fromValue(r.resourceId)); + if(!r.typeConflict.empty()) + { + node = new RDTreeWidgetItem({slotname, tr("Conflicting bindings"), lit("-"), lit("-"), + lit("-"), lit("-"), lit("-"), lit("-"), QString()}); - if(tex) - setViewDetails(node, tex, r.firstMip, r.numMips, 0, ~0U, r.completeStatus); + setViewDetails(node, NULL, 0, 0, 0, ~0U, &r); + } + else + { + node = + new RDTreeWidgetItem({slotname, r.resourceId, typeName, w, h, d, a, format, QString()}); + + node->setTag(QVariant::fromValue(r.resourceId)); + + if(tex) + setViewDetails(node, tex, r.firstMip, r.numMips, 0, ~0U, &r); + } if(!filledSlot) setEmptyRow(node); - if(!r.completeStatus.isEmpty()) + if(!r.completeStatus.isEmpty() || !r.typeConflict.isEmpty()) setEmptyRow(node); if(!usedSlot) diff --git a/qrenderdoc/Windows/PipelineState/GLPipelineStateViewer.h b/qrenderdoc/Windows/PipelineState/GLPipelineStateViewer.h index 651e0bdb0..eaa466504 100644 --- a/qrenderdoc/Windows/PipelineState/GLPipelineStateViewer.h +++ b/qrenderdoc/Windows/PipelineState/GLPipelineStateViewer.h @@ -110,7 +110,7 @@ private: void setViewDetails(RDTreeWidgetItem *node, TextureDescription *tex, uint32_t firstMip, uint32_t numMips, uint32_t firstSlice, uint32_t numSlices, - const rdcstr &completeStatus = rdcstr()); + const GLPipe::Texture *texBinding = NULL); void exportHTML(QXmlStreamWriter &xml, const GLPipe::VertexInput &vtx); void exportHTML(QXmlStreamWriter &xml, const GLPipe::Shader &sh); diff --git a/renderdoc/api/replay/gl_pipestate.h b/renderdoc/api/replay/gl_pipestate.h index 4a7c21a77..d413ac101 100644 --- a/renderdoc/api/replay/gl_pipestate.h +++ b/renderdoc/api/replay/gl_pipestate.h @@ -317,6 +317,16 @@ struct Texture complete. Otherwise it contains an explanation of why the texture is believed to be incomplete. )"); rdcstr completeStatus; + + DOCUMENT(R"(The details of any type conflict on this binding. This can happen if +multiple uniforms are pointing to the same binding but with different types. In this case it is +impossible to disambiguate which binding was used. + + +If this string is empty, no conflict is present. Otherwise it contains the bindings which are +in conflict and their types. +)"); + rdcstr typeConflict; }; DOCUMENT("Describes the sampler properties of a texture."); diff --git a/renderdoc/driver/gl/gl_replay.cpp b/renderdoc/driver/gl/gl_replay.cpp index efceb1174..622e364fb 100644 --- a/renderdoc/driver/gl/gl_replay.cpp +++ b/renderdoc/driver/gl/gl_replay.cpp @@ -1222,6 +1222,8 @@ void GLReplay::SavePipelineState(uint32_t eventId) GLenum binding = eGL_NONE; GLenum target = eGL_NONE; TextureType resType = TextureType::Unknown; + rdcstr firstBindName; + rdcstr typeConflict; for(size_t s = 0; s < ARRAY_COUNT(refls); s++) { @@ -1255,11 +1257,11 @@ void GLReplay::SavePipelineState(uint32_t eventId) if(target != eGL_NONE) t = TextureBinding(target); - resType = res.resType; - if(binding == eGL_NONE) { binding = t; + firstBindName = res.name; + resType = res.resType; } else if(binding == t) { @@ -1268,8 +1270,17 @@ void GLReplay::SavePipelineState(uint32_t eventId) } else if(binding != t) { - RDCWARN("Two uniforms pointing to texture unit %d with types %s and %s", unit, - ToStr(binding).c_str(), ToStr(t).c_str()); + RDCERR("Two uniforms pointing to texture unit %d with types %s and %s", unit, + ToStr(binding).c_str(), ToStr(t).c_str()); + + if(typeConflict.empty()) + { + typeConflict = StringFormat::Fmt("First binding found '%s' is %s", + firstBindName.c_str(), ToStr(resType).c_str()); + } + + typeConflict += + StringFormat::Fmt(", '%s' is %s", res.name.c_str(), ToStr(res.resType).c_str()); } } } @@ -1286,6 +1297,8 @@ void GLReplay::SavePipelineState(uint32_t eventId) else drv.glGetIntegerv(binding, (GLint *)&tex); + pipe.textures[unit].typeConflict = typeConflict; + if(tex == 0) { pipe.textures[unit].resourceId = ResourceId(); diff --git a/renderdoc/replay/renderdoc_serialise.inl b/renderdoc/replay/renderdoc_serialise.inl index 30146c1fe..b7087aa0b 100644 --- a/renderdoc/replay/renderdoc_serialise.inl +++ b/renderdoc/replay/renderdoc_serialise.inl @@ -1695,8 +1695,9 @@ void DoSerialise(SerialiserType &ser, GLPipe::Texture &el) SERIALISE_MEMBER(swizzle); SERIALISE_MEMBER(depthReadChannel); SERIALISE_MEMBER(completeStatus); + SERIALISE_MEMBER(typeConflict); - SIZE_CHECK(56); + SIZE_CHECK(80); } template