Mesh exploder

[GertyP]

Description

This is just a tentative draft only to see if there's any interest in a little feature that I've found can be very helpful and revealing of problematic and sub-optimal meshes when I've previously implemented it as a little in-engine visualiser.

This just displaces vertex positions as a function of their local position and their vertex ID, which means that multiple vertices in the same position but which are actually distinct vertices (with different vertex IDs) are displaced by different amounts, revealing gaps between triangles that don't share the exact same vertices.

Sometimes those gaps are just where the mesh author expects (i.e. faceted triangle normals and/or texture co-ord breaks/seams will expose these breaks that partially duplicate coincident vertices) but sometimes they can reveal otherwise hidden, unintentionally broken meshes, which then exposes faults in the original source asset data and/or the asset pipeline.

This kind of visualisation has revealed a bunch of problems for me in different engines in the past, like -

• Broken vertex welding code in asset pipelines
• Excessive/unintended source asset vertex attributes getting used in asset pipeline processing
• Unnecessary, never visible (e.g. interior double-sided) geometry
• Even multiple sets of the same geometry overlaid on each other!

Yes, it's technically possible to identify the same things with the existing visualisation and vertex data table, if you're prepared to carefully and patiently examine the data, but it's much easier to just quickly waggle a slider to reveal these things.

Rather than re-implement the same kind of thing in different engines (some of which can be a real hassle), I thought that this kind of thing is quite well suited to renderdoc, so I gave this a go.

A few things I should probably mention -

1. There seems to be no existing QSlider widget anywhere in rdoc, so when I initially added a slider widget, it draw nothing and, not being familiar with Qt, it took me a while to figure out that I seem to have to implement some rudimentary slider bounds and drawing code myself. Is that expected? Is there a simpler way to achieve better slider drawing than the way I've tried it?
2. Because renderdoc's mesh visualisation cannot rely on any other vertex attributes (e.g. we don't always have normals or any attributes other than position available) then choosing a good vertex displacement direction only based on position can result in us exploding the vertices along directions that aren't particularly effective in revealing the disjoint gaps. Perhaps it would be possible to build up an extra vertex attribute set used by renderdoc's mesh visualiser, which fills in a better displacement direction (like an average of all adjoining triangle face normals would often make for better/cleaner mesh explosion), which can then be used by the rdoc mesh visualiser vertex shader, but that feels like something only worth doing as a later refinement (if any of this gets any interest).

I can seen how it's slightly ... unusual, so I won't mind if it's not to your taste but it would be nice to also understand where you think the drawbacks might outweigh the benefits I've tried to describe above. Please let me know what you think.

[baldurk]

Thanks for getting in touch with an early checkin/feedback, that's really helpful when developing any change. I think this would be OK to add - I'd say it's definitely niche in its use, but it's fairly light on implementation complexity and doesn't add a huge extra maintenance burden or interfere for people who aren't using it.

The thing I would say at a high level is that it would make sense to have this as more of a feature you enable when you want it, than having the slider always visible by default. Maybe it should be one of the solid shading modes, if that makes sense? If that was renamed from 'solid shading' to 'visualisation mode' it might be more inclusive of this concept.

That way there isn't a slider present all the time for people who only want its use occasionally or might not understand what it's for as it's not as immediately intuitive as wireframe or vertex highlighting. This also allows you to be a bit more generous with the UI - e.g. I don't see a reset-to-default button which would be useful if you accidentally nudge it and want to set it back to 0, and you might want different exploding algoithms or weightings.

As a more minor implementation note - might it make sense to implement this inside the flat shaded geometry shader? That way you have a per-triangle face normal to work with for the direction. The vertex ID could be passed through easily enough from vertex to geometry stage.

The lack of slider theming in the custom theme is indeed because there's not been a slider before - the custom theme only covers the controls currently in use. It can probably be tweaked a bit but I'll wait until this PR is more final.

[baldurk]

As a note just to avoid miscommunication, while this is marked as draft then unless you specifically request comment or a review I'm not going to do so as you may be pushing intermediate commits while working on this. Also while it's a draft I'll stick to high level feedback and wait for precise feedback until it's more final.

[GertyP]

I think I've been a bit unlucky and been tinkering in an area that's just undergone a large amount of changes and given me a few fiddly conflicts to resolve!

I made changes, following on from your feedback, as separate commits (instead of squashing everything down into the one main commit) in case it helps to look at smaller diffs but it just made rebasing with all the new incoming conflicts harder and more error-prone so, in the end, I've gone and squashed it all down.

I think this now does everything you suggest, along with a couple of other tweaks -

• Changes 'solidShading' and 'solidShadeMode' to 'visualisation' and 'visualisationMode'.
• Adds new 'Exploded' visualisation mode.
• Adds exploder scale widget.
• Hides the 'highlightVerts' widget when using 'Exploded' vis for both real-estate and practical implementation reasons.
• Hide new exploder controls when not in 'Exploded' mode.
• Better visualisation of shared vs disjoint vertices in the explode visualisation by also colouring verts by vertex ID with a simple repeating, continuous gradient.
• Slightly better explode displacement direction, exploding from mesh bounds centre instead of simply from model-space origin.

The visualisation now better shows a mesh with poor vertex sharing -

versus one with good vtx re-use -

I can already see I've broken some tests (seems to be documentation, mostly), which I'll look at fixing but I thought I'd update this so you can give any other thoughts you think might be worthwhile.

On a couple of unrelated points, while resolving conflicts, I spotted what looked like a typo in the recent meshlet changes -

uint32_t *meshletCounts = (uint32_t *)m_MeshRender.MeshletSSBO.Map(
    &dynOffs[1], AlignUp4(numMeshlets + 4) * sizeof(uint32_t));
if(!data)
  return;

which I suspect should be corrected to use if(!meshletCounts) instead, which I've incorporated into this latest squashed change.
I know some people are insistent splitting out changes like this into separate commits but I'm yet to be convinced that the benefit of doing so outweighs the time and error-prone risks of sometimes fiddly git history rewriting. However, let me know and I'll separate that out.

Also, in vk_rendermesh.cpp, there's -

	// point list topologies don't have lighting obvious, just render them as solid
	if(pipe == VK_NULL_HANDLE)
	  pipe = cache.pipes[VKMeshDisplayPipelines::ePipe_SolidDepth];

but the comment references point list topologies, which doesn't seem to relate to the the actual 'if' test. I wondered if it was really intended to test for cfg.position.topology == Topology::PointList but was mistakenly copied from what was an exact equivalent block of code immediately after the switch block.
And since it is actually equivalent to the subsequent block, I removed the duplication, which I've rolled in to the lastest squashed commit.

[baldurk]

Yes I wasn't sure which was going to land first but indeed there was a new solid shade/visualisation mode added recently for mesh shader support.

Regarding the commits - squashing is fine especially with a hefty conflict resolve for this. I'm not too anal about commit organisation. It's certainly something to strive for but logical separation is an ideal not a requirement. The only thing I am strict about is that each commit individually should comply with clang-format and compile (so no commits in PRs which only fix compile errors or formatting). Everything else is negotiable IMO.

I'll do a more detailed review next week as I'm away for the weekend. The meshletCounts fix sounds good, I think the if(!pipe) is a bit indirect - the lit-shaded pipeline will not be created when the topology is point, so it will end up as NULL there and so we fall back to just 'solid'.

[GertyP]

so no commits in PRs which only fix compile errors or formatting

OK. You can see I've immediately broken that with what I hope are fixes to some python and documentation tests 😄 but rest assured, the intention isn't to leave it like that; it's just so you or anyone who may be interested can see what subsequent smaller incidental or incremental changes have been made, but once everyone's happy with everything, I'll squash them into the main feature commit.

[GertyP]

Regarding -

I don't see a reset-to-default button which would be useful if you accidentally nudge it and want to set it back to 0

it automatically snaps the slider back to 0 when released (although there seems to be some other weird widget interaction that I'm not familiar with, which manages to move the slider in a way that doesn't give us a 'on released' event to snap back to zero 🤷 ).

Re. -

might it make sense to implement this inside the flat shaded geometry shader? That way you have a per-triangle face normal to work with for the direction. The vertex ID could be passed through easily enough from vertex to geometry stage.

I might have misunderstood but I'm not sure how that would work. If we defer displacing vtx positions until the GS, where we construct face normals from the tri positions, then exploding/displacing along the tri face normals will always shatter the resultant mesh into individual shards with gaps between all primitives, even with vertices that are shared between prim edges because the GS is emitting a triplet of new displaced verts for each primitive; the useful aspect of the visualiser is to reveal cracks along primitive edges where the underlying vertex IDs are not shared (well, that and helping to reveal additional coincident duplicate primitives). I think the ideal way to reveal that would be to calculate a displacement normal for each vertex ID as an
average of all triangle face normals that share/reference it, but that's beyond the ability of a geometry shader. That kind of thing would require a CPU-side traversal of the entire draw's index list before then baking these calculated displacement normals into a new vertex stream... all of which starts to complicate things quite a bit and I worry may well have unforseen complications/interactions with other systems in rdoc.

[baldurk]

This looks good overall. I've made some smaller implementation notes but nothing fundamental as we've covered before. You're right of course about using the geometry shader, I hadn't thought that all the way through to see that it would break the vertex sharing information even if you did pass through the vertex ID.

[baldurk]

I'm not sure I like this behaviour honestly, as it means if I want to examine a mesh while exploded from different angles it becomes a constant back and forth of dragging the slider, then moving the camera, then dragging it again. It's also just a bit weird for the program to revert something I did - it feels almost like a bug where the value is supposed to be read-only but it can be temporarily modified before snapping back. I'm not sure what your thoughts were or how strongly you thought this behaviour was desirable?

I think a reset-to-default button would be preferable. It could be just an icon'd tool button with a tooltip, although there is a slight snag in that the 'reset' icon is used earlier to reset the camera position to zero. We can probably find an appropriate alternative icon or just go with 'Reset' text.

I did notice the same thing as you - scrolling with the mouse wheel or clicking to the left or right of the handle will move it without it being 'released'.

[GertyP]

OK. I quite like the auto-return behaviour but I'm wondering if there's a nice way to have both the auto-return and the normal slider behaviour, perhaps with a little 2-position radio knob: In one position, it always auto-returns-to-zero when released. In the other position, it stays put when released but you can reset it with a quick flick and return of the knob between positions... Although now I say that, I worry that'll make the U.I. a bit too tight/busy... and I don't look forward to having to wrangle Qt into dealing with and rendering such a widget. 😄

What do you think?

[GertyP]

I went and added a slider reset button. Takes up a bit more space but I agree that it can help to be able to navigate around the mesh with it slightly exploded.

[baldurk]

I don't dislike the auto-return behaviour necessarily and it can be useful sometimes, but I'm not sure the value is justified by the cost of keeping it implemented and having this atypical behaviour. As I think you've come around to as well it's not really feasible to provide both mechanisms. It ends up either being overcomplicated if there's an explicit UI control, or too hidden and confusing if there's some kind of shortcut like holding shift while dragging to make it temporary. Someone could do that by accident and get really lost.

My intuition is that you really have to pick one way and stick to it, and although it's occasionally nice to have it return I don't see a sufficient benefit. I'd be fine if you wanted to maybe add a shortcut key for escape to reset while the slider is focussed - which it should be immediately after interacting with it. It's not discoverable and isn't quite fully automatic but it is at least something.

[GertyP]

I hope this wasn't premature but I've squashed all the feedback changes down into one feature commit now and also changed from a draft PR to a normal PR.