optimizations for selection performance

[ericwa]

I was messing around with #1404 a bit, tried some micro-optimization and got MapView3D::doRenderMap about 33% faster when making a selection on a large map.

IMO there are two approaches to improving the selection performance:

1. "proper" ones, involving trying to rendering only the changed parts of the map, using arguments of functions likeMapRenderer::selectionDidChange. (or partitioning the brushes into subsets, and only re-rendering the subsets containing the brushes that changed selection state). As you were saying in Selection performance on large maps #1404 , re-rendering only the exact set of changed brushes would interfere with OpenGL batching. It might be a performance win overall to forget about OpenGL batching.
2. micro-optimizing the current code, this is what I tried in this PR. What I noticed was a lot of stuff was being recomputed per-edge and per-face that only needed to be evaluated per-brush.

The code is certainly uglier with this change.. personally I'm not sure it's worth 33% better performance ;)

In particular, in the doCollectShownFaces implementations in MapRenderer.cpp I inlined some bits of code from EditorContext. e.g. replacing per-face calls to visible(const Model::BrushFace* face) with checking visible(brush) once, and using that, since the implementation of visible(const Model::BrushFace* face) is:

bool EditorContext::visible(const Model::BrushFace* face) const {
            return visible(face->brush());
        }

That's the worst thing.

Also, the CollectShownFaces/CollectShownEdges classes could be replaced with C++11 lambdas when TB switches to C++11.

[ericwa]

btw, this was my benchmarking code:

diff --git a/common/src/View/MapView3D.cpp b/common/src/View/MapView3D.cpp
index ad45adf..5256852 100644
--- a/common/src/View/MapView3D.cpp
+++ b/common/src/View/MapView3D.cpp
@@ -566,8 +566,13 @@ namespace TrenchBroom {
         void MapView3D::doRenderGrid(Renderer::RenderContext& renderContext, Renderer::RenderBatch& renderBatch) {}

         void MapView3D::doRenderMap(Renderer::MapRenderer& renderer, Renderer::RenderContext& renderContext, Renderer::RenderBatch& renderBatch) {
+            wxMilliClock_t start = wxGetLocalTimeMillis();
+            
             renderer.render(renderContext, renderBatch);

+            wxMilliClock_t end = wxGetLocalTimeMillis();
+            printf("MapView3D::doRenderMap took %d ms\n", (int)wxMilliClockToLong(end - start));
+            
             MapDocumentSPtr document = lock(m_document);
             if (renderContext.showSelectionGuide() && document->hasSelectedNodes()) {
                 const BBox3& bounds = document->selectionBounds();

[kduske]

Alright, will have a look soon. One thing though, can you please make sure that face selection still works, esp. with the correct edges being shown as selected? Because that might be affected by this change.

[ericwa]

I'm away from my PC for a few days, but will check on Wednesday

[ericwa]

Face selection (including rendering of selected edges) still looks correct with this.

[kduske]

Excellent, thanks.

[kduske]

The code is actually not ugly, but the control flow is a bit murky. I have mostly changed the names of the new classes / methods to reflect the new provider / acceptor nature of the control flow. Thanks for the contribution!