Skip to content

Commit

Permalink
Fix vertex viewer issues
Browse files Browse the repository at this point in the history
  • Loading branch information
nyorain committed Aug 27, 2023
1 parent 32d8a1d commit 97a4188
Show file tree
Hide file tree
Showing 9 changed files with 110 additions and 33 deletions.
15 changes: 13 additions & 2 deletions docs/own/workstack.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,19 @@
by default for vertex input: y-up is y-up (although many models have z-up)
default for vertex output: y-down is y-up (except when viewport is negative,
then y-up is y-up).
- [ ] Make vertices selectable. I.e. via mouse click in debugger
- [ ] Draw selected vertex via point
- [ ] hmm, y-flip isn't that useful atm, should rather be something
like oneMinusY?
- [ ] Make vertices selectable. I.e. via mouse click in table
{done only for vertex input/output for now, not RT}
- [ ] show vertex table for RT displayTriangles
- [ ] add support for vertex selection
- [ ] displayInstances: allow to give each instance its own color.
Somehow display instances? Allow to select them.
- [ ] Draw selected vertex via point {partially done, ugly and has issues}
- [ ] visual: ignore depth for drawn point
- [ ] visual: color it in some obvious color, red/blue
- [ ] visual: make point size variable
- [ ] allow selecting a triangle, highlighting the three points in the table
- [x] Allow to select specific vertex (either input or output) in debugger
- [ ] Allow to choose display style
- [x] solid (single-colored or shaded) vs wireframe
Expand Down
6 changes: 6 additions & 0 deletions src/buffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,12 @@ VKAPI_ATTR VkResult VKAPI_CALL CreateBuffer(
// AFAIK this should always be supported, for all buffers.
nci.usage |= VK_BUFFER_USAGE_TRANSFER_SRC_BIT;

// storage buffer usage needed for indirect vertex copy
if(nci.usage & (VK_BUFFER_USAGE_VERTEX_BUFFER_BIT | VK_BUFFER_USAGE_INDEX_BUFFER_BIT) &&
dev.indirectVertexCopy) {
nci.usage |= VK_BUFFER_USAGE_STORAGE_BUFFER_BIT;
}

// NOTE: needed for our own operations on the buffer. Might be better to
// properly acquire/release it instead though, this might have
// a performance impact.
Expand Down
4 changes: 3 additions & 1 deletion src/commandHook/record.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1295,12 +1295,14 @@ void CommandHookRecord::copyVertexInput(Command& bcmd, RecordInfo& info) {
auto wbuf = dst.writeData();
Metadata md {};
md.copyTypeOrIndexOffset = params.firstIndex; // indexOffset
md.minIndex = 0xFFFFFFFFu;
md.minIndex = params.indexCount > 0 ? 0xFFFFFFFFu : 0u;
md.maxIndex = 0u;
md.indexCount = params.indexCount;
md.dispatchPerInstanceX = ceilDivide(params.instanceCount, 64u);
md.dispatchPerInstanceY = 1u;
md.dispatchPerInstanceZ = 1u;
md.dispatchPerVertexY = 1u;
md.dispatchPerVertexZ = 1u;
md.firstInstance = params.firstInstance;
md.firstVertex = params.vertexOffset;
nytl::write(wbuf, md);
Expand Down
2 changes: 1 addition & 1 deletion src/data/copyVertices.comp
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ void main() {
(info.firstVertex + info.minIndex);

// prevent out-of-bounds reading
if(srcBufOffset + (dtID + 1) * vertexStride > src.data.length()) {
if(srcBufOffset + (dtID + idOffset + 1) * vertexStride > src.data.length()) {
return;
}

Expand Down
6 changes: 3 additions & 3 deletions src/data/processIndices.comp
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,15 @@ void main() {
const uint srcOffset = cmdIndexOffset + invocOff;
const uint dstOffset = invocOff;

const uint maxCount = min(srcOffset + info.indexCount, srcIndices.length());
if(srcOffset >= maxCount) {
const uint indicesEnd = info.copyTypeOrIndexOffset + min(info.indexCount, srcIndices.length());
if(srcOffset >= indicesEnd) {
return;
}

uint ownMax = 0u;
uint ownMin = 0xFFFFFFFFu;

for(uint i = 0u; i < indicesPerInvoc && srcOffset + i < maxCount; ++i) {
for(uint i = 0u; i < indicesPerInvoc && srcOffset + i < indicesEnd; ++i) {
uint index = uint(srcIndices[srcOffset + i]);
ownMin = min(ownMin, index);
ownMax = max(ownMax, index);
Expand Down
5 changes: 5 additions & 0 deletions src/data/vertices.vert
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,10 @@ void main() {
outPos = pos;

gl_Position = pcr.viewProjMtx * vec4(pos, 1.0);

// needed e.g. for pointPipe but also when displaying point data by
// application
// TODO: control via pcr
gl_PointSize = 8.f;
}

4 changes: 4 additions & 0 deletions src/device.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,10 @@ struct Device {
// Whether we are in integration testing mode
bool testing {};

// Whether indirect vertex copy is enabled.
// Will modify usage flags resources are created with
static constexpr auto indirectVertexCopy = true;

std::atomic<bool> doFullSync {};
std::atomic<bool> captureCmdStack {};

Expand Down
94 changes: 70 additions & 24 deletions src/gui/vertexViewer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,23 @@ void VertexViewer::createFrustumPipe() {
dev.dispatch.DestroyShaderModule(dev.handle, vertShader, nullptr);
}

VkPipeline VertexViewer::getOrCreatePipe(VkFormat format, u32 stride,
VkPrimitiveTopology topo, VkPolygonMode polygonMode) {
for(auto& pipe : pipes_) {
if(pipe.format == format &&
pipe.stride == stride &&
pipe.polygon == polygonMode &&
pipe.topology == topo) {
return pipe.pipe;
}
}

// TODO: do async and show ui message meanwhile.
// We currently sometimes get ui hangs from this when first
// selecting the vertex viewer.
return createPipe(format, stride, topo, polygonMode);
}

VkPipeline VertexViewer::createPipe(VkFormat format, u32 stride,
VkPrimitiveTopology topology, VkPolygonMode polygonMode) {
auto& dev = gui_->dev();
Expand Down Expand Up @@ -539,23 +556,8 @@ void VertexViewer::imGuiDraw(const DrawData& data) {
auto polygonMode = wireframe_ ? VK_POLYGON_MODE_LINE : VK_POLYGON_MODE_FILL;

// try to find matching pipeline
VkPipeline foundPipe {};
for(auto& pipe : pipes_) {
if(pipe.format == attrib.format &&
pipe.stride == binding.stride &&
pipe.polygon == polygonMode &&
pipe.topology == data.topology) {
foundPipe = pipe.pipe;
}
}

if(!foundPipe) {
// TODO: do async and show ui message meanwhile.
// We currently sometimes get ui hangs from this when first
// selecting the vertex viewer.
foundPipe = createPipe(attrib.format, binding.stride,
data.topology, polygonMode);
}
auto foundPipe = getOrCreatePipe(attrib.format, binding.stride,
data.topology, polygonMode);

auto displaySize = ImGui::GetIO().DisplaySize;

Expand Down Expand Up @@ -620,14 +622,27 @@ void VertexViewer::imGuiDraw(const DrawData& data) {
// vertex buffer was captured, otherwise indices might be
// out-of-range
dev.dispatch.CmdBindIndexBuffer(cb, data.indexBuffer.buffer,
0, *data.params.indexType);
data.indexBuffer.offset(), *data.params.indexType);
dev.dispatch.CmdDrawIndexed(cb, data.params.drawCount, 1,
data.params.offset, data.params.vertexOffset, data.params.instanceID);
} else {
dev.dispatch.CmdDraw(cb, data.params.drawCount, 1,
data.params.offset, data.params.instanceID);
}

constexpr auto drawSelectedPoint = true;
if(drawSelectedPoint && data.selectedVertex < data.params.drawCount) {
auto pointPipe = getOrCreatePipe(attrib.format, binding.stride,
VK_PRIMITIVE_TOPOLOGY_POINT_LIST, VK_POLYGON_MODE_FILL);
dev.dispatch.CmdBindPipeline(cb, VK_PIPELINE_BIND_POINT_GRAPHICS, pointPipe);

pcData.shade = false;
dev.dispatch.CmdPushConstants(cb, pipeLayout_,
VK_SHADER_STAGE_VERTEX_BIT | VK_SHADER_STAGE_FRAGMENT_BIT,
0, sizeof(pcData), &pcData);
dev.dispatch.CmdDraw(cb, 1, 1, data.selectedVertex, 0u);
}

if(data.drawFrustum) {
struct {
Mat4f matrix;
Expand Down Expand Up @@ -1085,8 +1100,9 @@ bool VertexViewer::displayInput(Draw& draw, const DrawCmdBase& cmd,
u32 index = readIndex(*params.indexType, ib);
imGuiText("{}", params.vertexOffset + index);

if (copyType == CommandHookRecord::copyTypeVertices) {
vertexID = index;
if(copyType == CommandHookRecord::copyTypeVertices) {
dlg_assert(index >= metadata.minIndex);
vertexID = index - metadata.minIndex;
}
}
} else {
Expand Down Expand Up @@ -1202,15 +1218,42 @@ bool VertexViewer::displayInput(Draw& draw, const DrawCmdBase& cmd,
drawData_.vertexBuffers.push_back({buf.buf, 0u, buf.size});
}

// normally won't need this, copied buffers are tight

if(useIndexedDraw) {
drawData_.indexBuffer = state.indexBufCopy.asSpan(sizeof(Metadata));
drawData_.params.offset = 0u;

// for copyTypeVertices, we just copied the raw index buffer.
// But we only copied the portion really needed by the draw call
dlg_assert(metadata.copyTypeOrIndexOffset == CommandHookRecord::copyTypeVertices);
drawData_.params.vertexOffset = -i32(metadata.minIndex);

// debug
// if(drawData_.params.drawCount > 100) {
// drawData_.params.drawCount = 3 * 16;
// }

auto indices = state.indexBufCopy.data();
skip(indices, sizeof(Metadata));
auto indSize = indexSize(*params.indexType);
auto indOff = selectedVertex_ * indSize;
if(indices.size() >= indOff + indSize) {
skip(indices, indOff);
drawData_.selectedVertex = readIndex(*params.indexType, indices) +
drawData_.params.vertexOffset;
} else {
dlg_trace("not enough indices for selected vertex");
drawData_.selectedVertex = 0xFFFFFFFFu;
}
} else {
drawData_.params.indexType = {};
}

// never need this, copied buffers are tight
drawData_.params.offset = 0u;
drawData_.params.vertexOffset = 0u;
drawData_.params.vertexOffset = 0;
drawData_.params.offset = 0u;

drawData_.selectedVertex = selectedVertex_;
}

auto cb = [](const ImDrawList*, const ImDrawCmd* cmd) {
auto* self = static_cast<VertexViewer*>(cmd->UserCallbackData);
Expand Down Expand Up @@ -1521,6 +1564,7 @@ void VertexViewer::displayOutput(Draw& draw, const DrawCmdBase& cmd,
drawData_.drawFrustum = gui_->dev().nonSolidFill;
drawData_.clear = doClear_;
drawData_.mat = nytl::identity<4, float>();
drawData_.selectedVertex = 0xFFFFFFFFu; // TODO

auto cb = [](const ImDrawList*, const ImDrawCmd* cmd) {
auto* self = static_cast<VertexViewer*>(cmd->UserCallbackData);
Expand Down Expand Up @@ -1605,6 +1649,7 @@ void VertexViewer::displayTriangles(Draw& draw, const AccelTriangles& tris, floa
drawData_.drawFrustum = false;
drawData_.clear = doClear_;
drawData_.mat = nytl::identity<4, float>();
drawData_.selectedVertex = 0xFFFFFFFFu; // TODO

auto cb = [](const ImDrawList*, const ImDrawCmd* cmd) {
auto* self = static_cast<VertexViewer*>(cmd->UserCallbackData);
Expand Down Expand Up @@ -1735,6 +1780,7 @@ void VertexViewer::displayInstances(Draw& draw, const AccelInstances& instances,
data.vertexBuffers = {{{tris.buffer.buf, 0u, tris.buffer.size}}};
data.self = this;
data.mat = ini.transform;
data.selectedVertex = 0xFFFFFFFFu; // TODO

auto cb = [](const ImDrawList*, const ImDrawCmd* cmd) {
auto* data = static_cast<DrawData*>(cmd->UserCallbackData);
Expand Down
7 changes: 5 additions & 2 deletions src/gui/vertexViewer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ struct DrawParams {
std::optional<VkIndexType> indexType {}; // nullopt for non-indexed draw
u32 offset {}; // firstVertex or firstIndex
u32 drawCount {}; // vertexCount or indexCount
u32 vertexOffset {}; // only for indexed drawing
i32 vertexOffset {}; // only for indexed drawing

// TODO: correctly implement multi-instance support
u32 instanceID {};
Expand All @@ -46,6 +46,8 @@ struct VertexViewer {

private:
void centerCamOnBounds(const AABB3f& bounds);
VkPipeline getOrCreatePipe(VkFormat format, u32 stride,
VkPrimitiveTopology topo, VkPolygonMode polygonMode);
VkPipeline createPipe(VkFormat format, u32 stride,
VkPrimitiveTopology topo, VkPolygonMode polygonMode);
void createFrustumPipe();
Expand All @@ -62,6 +64,7 @@ struct VertexViewer {

DrawParams params;
vku::BufferSpan indexBuffer; // only for indexed drawing
u32 selectedVertex {0xFFFFFFFFu};

Vec2f offset {};
Vec2f size {};
Expand Down Expand Up @@ -126,7 +129,7 @@ struct VertexViewer {

u32 selectedVertex_ {};

u32 selectedID_ {};
u32 selectedID_ {}; // command id
std::vector<DrawData> drawDatas_;

u32 precision_ {5u};
Expand Down

0 comments on commit 97a4188

Please sign in to comment.