From d4392bdd7dc767618918972465f6c453c09f04ef Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Thibault=20Bruy=C3=A8re?= <thibault.bruyere@kitware.com>
Date: Tue, 31 Jan 2023 15:04:27 +0100
Subject: [PATCH] fix(HardwareSelector): fix picking of actors with both
 polylines and polys

Take cellOffset into account in selectionMaps when creating a VBO with createVBO()
Disable rendering of edges when rendering for picking

Closes #2664
---
 .../test/testHardwareSelectorPoints.js        | 16 +++++++-
 .../OpenGL/CellArrayBufferObject/index.js     |  2 +-
 .../Rendering/OpenGL/PolyDataMapper/index.js  | 39 +++++++++++--------
 3 files changed, 37 insertions(+), 20 deletions(-)

diff --git a/Sources/Rendering/Core/HardwareSelector/test/testHardwareSelectorPoints.js b/Sources/Rendering/Core/HardwareSelector/test/testHardwareSelectorPoints.js
index ed93f7f9a29..65dc812d265 100644
--- a/Sources/Rendering/Core/HardwareSelector/test/testHardwareSelectorPoints.js
+++ b/Sources/Rendering/Core/HardwareSelector/test/testHardwareSelectorPoints.js
@@ -65,18 +65,23 @@ test.onlyIfWebGL('Test HardwareSelector Points', (tapeContext) => {
 
   renderer.addActor(actor3);
 
-  // Square polyline upper left --------------------------------
+  // Square polyline upper left with triangle poly inside --------------------------------
   // Triangle polyline lower right -----------------------------
   const mapper4 = gc.registerResource(vtkMapper.newInstance());
   const polygon = gc.registerResource(vtkPolydata.newInstance());
   const squarePoints = [-0.25, 1.25, 0, 0, 1.25, 0, 0, 1, 0, -0.25, 1, 0];
   const trianglePoints = [1, 0, 0, 1, -0.25, 0, 1.25, -0.125, 0];
+  const polyPoints = [-0.2, 1.05, 0, -0.05, 1.05, 0, -0.125, 1.2, 0];
   polygon
     .getPoints()
-    .setData(Float32Array.from([...squarePoints, ...trianglePoints]), 3);
+    .setData(
+      Float32Array.from([...squarePoints, ...trianglePoints, ...polyPoints]),
+      3
+    );
   polygon
     .getLines()
     .setData(Uint16Array.from([5, 0, 1, 2, 3, 0, 4, 4, 5, 6, 4]));
+  polygon.getPolys().setData(Uint16Array.from([3, 7, 8, 9]));
   mapper4.setInputData(polygon);
 
   const actor4 = gc.registerResource(vtkActor.newInstance());
@@ -160,6 +165,13 @@ test.onlyIfWebGL('Test HardwareSelector Points', (tapeContext) => {
       tapeContext.ok(res[0].getProperties().attributeID === 1);
     })
   );
+  // On the triangle of the actor 4 inside the square
+  promises.push(
+    sel.selectAsync(renderer, 134, 265, 134, 265).then((res) => {
+      tapeContext.ok(res[0].getProperties().propID === 7);
+      tapeContext.ok(res[0].getProperties().attributeID === 2);
+    })
+  );
   Promise.all(promises).then(() => {
     gc.releaseResources();
   });
diff --git a/Sources/Rendering/OpenGL/CellArrayBufferObject/index.js b/Sources/Rendering/OpenGL/CellArrayBufferObject/index.js
index 8c499b27a2f..929e310631a 100644
--- a/Sources/Rendering/OpenGL/CellArrayBufferObject/index.js
+++ b/Sources/Rendering/OpenGL/CellArrayBufferObject/index.js
@@ -308,7 +308,7 @@ function vtkOpenGLCellArrayBufferObject(publicAPI, model) {
       // Keep track of original point and cell ids, for selection
       if (selectionMaps) {
         selectionMaps.points[pointCount] = i;
-        selectionMaps.cells[pointCount] = cellCount;
+        selectionMaps.cells[pointCount] = cellCount + options.cellOffset;
       }
       ++pointCount;
 
diff --git a/Sources/Rendering/OpenGL/PolyDataMapper/index.js b/Sources/Rendering/OpenGL/PolyDataMapper/index.js
index d4cbfb4c6ac..d3e1e0368c9 100755
--- a/Sources/Rendering/OpenGL/PolyDataMapper/index.js
+++ b/Sources/Rendering/OpenGL/PolyDataMapper/index.js
@@ -1747,7 +1747,11 @@ function vtkOpenGLPolyDataMapper(publicAPI, model) {
         model.drawingEdges =
           drawSurfaceWithEdges &&
           (i === primTypes.TrisEdges || i === primTypes.TriStripsEdges);
-        if (!model.drawingEdges || !model.renderDepth) {
+        // don't draw edges when rendering depth or rendering for picking
+        if (
+          !model.drawingEdges ||
+          !(model.renderDepth || model.lastSelectionState >= 0)
+        ) {
           model.lastBoundBO = model.primitives[i];
           model.primitiveIDOffset += model.primitives[i].drawArrays(
             ren,
@@ -1962,7 +1966,7 @@ function vtkOpenGLPolyDataMapper(publicAPI, model) {
         { inRep: 'polys', cells: poly.getPolys() },
         { inRep: 'strips', cells: poly.getStrips() },
       ];
-      const drawSurfaceWithEdges = // TODO: false if picking
+      const drawSurfaceWithEdges =
         actor.getProperty().getEdgeVisibility() &&
         representation === Representation.SURFACE;
 
@@ -1983,22 +1987,23 @@ function vtkOpenGLPolyDataMapper(publicAPI, model) {
         } else {
           // if we have edge visibility build the edge VBOs
           if (drawSurfaceWithEdges) {
-            options.cellOffset += model.primitives[i].getCABO().createVBO(
-              primitives[i].cells,
-              primitives[i].inRep,
-              Representation.WIREFRAME,
-              {
-                ...options,
-                tcoords: null,
-                colors: null,
-                haveCellScalars: false,
-                haveCellNormals: false,
-              },
-              model.selectionWebGLIdsToVTKIds
-            );
-            options.vertexOffset += model.primitives[i]
+            // VBOs for edges in "surface with edges" are the last to be built,
+            // they are not used when picking with a hardware selector so they
+            // don't need selectionWebGLIdsToVTKIds and don't update cellOffset and vertexOffset
+            model.primitives[i]
               .getCABO()
-              .getElementCount();
+              .createVBO(
+                primitives[i].cells,
+                primitives[i].inRep,
+                Representation.WIREFRAME,
+                {
+                  ...options,
+                  tcoords: null,
+                  colors: null,
+                  haveCellScalars: false,
+                  haveCellNormals: false,
+                }
+              );
           } else {
             // otherwise free them
             model.primitives[i].releaseGraphicsResources();