fix: LEAP-1191: Fix drawing tools scenarios with ctrl pressed (#6056)

Co-authored-by: Gondragos <[email protected]> Co-authored-by: hlomzik <[email protected]> Co-authored-by: triklozoid <[email protected]>
HumanSignal · Jul 25, 2024 · 0d980a2 · 0d980a2
1 parent 027401e
commit 0d980a2
Show file tree

Hide file tree

Showing 9 changed files with 240 additions and 2 deletions.
diff --git a/web/libs/editor/src/components/ImageView/ImageView.jsx b/web/libs/editor/src/components/ImageView/ImageView.jsx
@@ -689,7 +689,6 @@ export default observer(
       }
 
       item.freezeHistory();
-      item.setSkipInteractions(false);
 
       return this.triggerMouseUp(e, e.evt.offsetX, e.evt.offsetY);
     };

diff --git a/web/libs/editor/src/mixins/DrawingTool.js b/web/libs/editor/src/mixins/DrawingTool.js
@@ -11,6 +11,7 @@ const DrawingTool = types
     default: true,
     mode: types.optional(types.enumeration(["drawing", "viewing"]), "viewing"),
     unselectRegionOnToolChange: true,
+    isDrawingTool: true,
   })
   .volatile(() => {
     return {

diff --git a/web/libs/editor/src/mixins/Tool.js b/web/libs/editor/src/mixins/Tool.js
@@ -93,6 +93,20 @@ const ToolMixin = types
 
       if (typeof self[fn] !== "undefined") self[fn].call(self, ev, args);
     },
+
+    /**
+     * Indicates will the tool interact with the regions or not
+     * It doesn't affect interactions with the canvas (zooming, drawing, etc.)
+     * Some tools might override this method (at least MoveTool and ZoomTool)
+     * @param e
+     * @returns {boolean}
+     */
+    shouldSkipInteractions(e) {
+      const isCtrlPressed = e.evt && (e.evt.metaKey || e.evt.ctrlKey);
+      const hasSelection = self.control.annotation.hasSelection;
+
+      return !!isCtrlPressed && !hasSelection;
+    },
   }));
 
 export default types.compose(ToolMixin, AnnotationMixin);
diff --git a/web/libs/editor/src/stores/Annotation/Annotation.js b/web/libs/editor/src/stores/Annotation/Annotation.js
@@ -287,7 +287,7 @@ export const Annotation = types
     },
 
     get hasSelection() {
-      return self.regionStore.selection.hasSelection;
+      return self.regionStore.hasSelection;
     },
     get selectionSize() {
       return self.regionStore.selection.size;

diff --git a/web/libs/editor/src/tools/Selection.js b/web/libs/editor/src/tools/Selection.js
@@ -31,6 +31,9 @@ const _Tool = types
     let isSelecting = false;
 
     return {
+      /**
+       * Indicates that move tool always interacts with regions
+       */
       shouldSkipInteractions() {
         return false;
       },

diff --git a/web/libs/editor/src/tools/Zoom.jsx b/web/libs/editor/src/tools/Zoom.jsx
@@ -82,6 +82,9 @@ const _Tool = types
     },
   }))
   .actions((self) => ({
+    /**
+     * Indicates that zoom tool can't interact with regions at all
+     */
     shouldSkipInteractions() {
       return true;
     },

diff --git a/web/libs/editor/tests/integration/data/image_segmentation/ctrl.ts b/web/libs/editor/tests/integration/data/image_segmentation/ctrl.ts
@@ -0,0 +1,10 @@
+export const imageToolsConfig = `
+<View>
+  <Image name="image" value="$image" />
+  <Rectangle name="rect" toName="image" />
+  <Ellipse name="ellipse" toName="image" />
+  <Polygon name="polygon" toName="image" />
+</View>`;
+
+export const image =
+  "https://htx-pub.s3.us-east-1.amazonaws.com/examples/images/nick-owuor-astro-nic-visuals-wDifg5xc9Z4-unsplash.jpg";
diff --git a/web/libs/editor/tests/integration/e2e/image_segmentation/ctrl.cy.ts b/web/libs/editor/tests/integration/e2e/image_segmentation/ctrl.cy.ts
@@ -0,0 +1,162 @@
+import { ImageView, LabelStudio, Sidebar } from "@humansignal/frontend-test/helpers/LSF";
+import { image, imageToolsConfig } from "../../data/image_segmentation/ctrl";
+
+describe("Image Segmentation - Drawing with ctrl pressed", () => {
+  it("should add region to selection when something selected - Rectangle", () => {
+    LabelStudio.params().config(imageToolsConfig).data({ image }).withResult([]).init();
+
+    ImageView.waitForImage();
+    ImageView.selectRectangleToolByButton();
+    cy.log("Prepare 2 rectangles");
+    ImageView.drawRectRelative(0.1, 0.1, 0.4, 0.8);
+    ImageView.drawRectRelative(0.55, 0.1, 0.4, 0.8);
+    cy.log("Select first rectangle");
+    ImageView.clickAtRelative(0.3, 0.3);
+    cy.log("Add second rectangle to selection with ctrl pressed");
+    ImageView.clickAtRelative(0.7, 0.3, { metaKey: true });
+    Sidebar.hasSelectedRegions(2);
+  });
+
+  it("should add region to selection when something selected - Ellipse", () => {
+    LabelStudio.params().config(imageToolsConfig).data({ image }).withResult([]).init();
+
+    ImageView.waitForImage();
+    ImageView.selectEllipseToolByButton();
+    cy.log("Prepare 2 ellipses");
+    ImageView.drawRectRelative(0.25, 0.5, 0.2, 0.4);
+    ImageView.drawRectRelative(0.75, 0.5, 0.2, 0.4);
+    cy.log("Select first ellipse");
+    ImageView.clickAtRelative(0.25, 0.5);
+    cy.log("Add second ellipse to selection with ctrl pressed");
+    ImageView.clickAtRelative(0.75, 0.5, { metaKey: true });
+    Sidebar.hasSelectedRegions(2);
+  });
+
+  it("should add region to selection when something selected - Polygon", () => {
+    LabelStudio.params().config(imageToolsConfig).data({ image }).withResult([]).init();
+
+    ImageView.waitForImage();
+    ImageView.selectPolygonToolByButton();
+    cy.log("Prepare 2 polygons");
+    ImageView.drawPolygonRelative(
+      [
+        [0.1, 0.1],
+        [0.4, 0.1],
+        [0.25, 0.9],
+      ],
+      true,
+    );
+    ImageView.drawPolygonRelative(
+      [
+        [0.6, 0.1],
+        [0.9, 0.1],
+        [0.75, 0.9],
+      ],
+      true,
+    );
+    cy.log("Select first polygon");
+    ImageView.clickAtRelative(0.25, 0.5);
+    cy.log("Add second polygon to selection with ctrl pressed");
+    ImageView.clickAtRelative(0.75, 0.5, { metaKey: true });
+    Sidebar.hasSelectedRegions(2);
+  });
+
+  it("should draw region through other region - Rectangle", () => {
+    LabelStudio.params().config(imageToolsConfig).data({ image }).withResult([]).init();
+
+    ImageView.waitForImage();
+    ImageView.selectRectangleToolByButton();
+    cy.log("Prepare 2 rectangles");
+    ImageView.drawRectRelative(0.1, 0.1, 0.4, 0.8);
+    ImageView.drawRectRelative(0.55, 0.1, 0.4, 0.8);
+    cy.log("Draw rectangle through other rectangle with ctrl pressed");
+    ImageView.drawRectRelative(0.3, 0.3, 0.4, 0.4, { metaKey: true });
+    Sidebar.hasSelectedRegions(0);
+    Sidebar.hasRegions(3);
+  });
+
+  it("should draw region through other region - Ellipse", () => {
+    LabelStudio.params().config(imageToolsConfig).data({ image }).withResult([]).init();
+
+    ImageView.waitForImage();
+    ImageView.selectEllipseToolByButton();
+    cy.log("Prepare 2 ellipses");
+    ImageView.drawRectRelative(0.25, 0.5, 0.2, 0.4);
+    ImageView.drawRectRelative(0.75, 0.5, 0.2, 0.4);
+    cy.log("Draw ellipse through other ellipse with ctrl pressed");
+    ImageView.drawRectRelative(0.25, 0.5, 0.4, 0.4, { metaKey: true });
+    Sidebar.hasSelectedRegions(0);
+    Sidebar.hasRegions(3);
+  });
+
+  it("should draw region through other region - Polygon", () => {
+    LabelStudio.params().config(imageToolsConfig).data({ image }).withResult([]).init();
+
+    ImageView.waitForImage();
+    ImageView.selectPolygonToolByButton();
+    cy.log("Prepare 2 polygons");
+    ImageView.drawPolygonRelative(
+      [
+        [0.1, 0.1],
+        [0.4, 0.1],
+        [0.25, 0.9],
+      ],
+      true,
+    );
+    ImageView.drawPolygonRelative(
+      [
+        [0.6, 0.1],
+        [0.9, 0.1],
+        [0.75, 0.9],
+      ],
+      true,
+    );
+    cy.log("Draw polygon through other polygon with ctrl pressed");
+    ImageView.drawPolygonRelative(
+      [
+        [0.25, 0.2],
+        [0.75, 0.2],
+        [0.75, 0.3],
+        [0.25, 0.3],
+      ],
+      true,
+      { metaKey: true },
+    );
+    Sidebar.hasSelectedRegions(0);
+    Sidebar.hasRegions(3);
+  });
+
+  it("should add region to selection when something selected - MoveTool", () => {
+    LabelStudio.params().config(imageToolsConfig).data({ image }).withResult([]).init();
+
+    ImageView.waitForImage();
+    ImageView.selectRectangleToolByButton();
+    cy.log("Prepare 2 rectangles");
+    ImageView.drawRectRelative(0.1, 0.1, 0.4, 0.8);
+    ImageView.drawRectRelative(0.55, 0.1, 0.4, 0.8);
+    cy.log("Switch to MoveTool");
+    ImageView.selectMoveToolByButton();
+    cy.log("Select first rectangle");
+    ImageView.clickAtRelative(0.3, 0.3);
+    cy.log("Add second rectangle to selection with ctrl pressed");
+    ImageView.clickAtRelative(0.7, 0.3, { metaKey: true });
+    Sidebar.hasSelectedRegions(2);
+  });
+
+  it("should add region to selection from scratch - MoveTool", () => {
+    LabelStudio.params().config(imageToolsConfig).data({ image }).withResult([]).init();
+
+    ImageView.waitForImage();
+    ImageView.selectRectangleToolByButton();
+    cy.log("Prepare 2 rectangles");
+    ImageView.drawRectRelative(0.1, 0.1, 0.4, 0.8);
+    ImageView.drawRectRelative(0.55, 0.1, 0.4, 0.8);
+    cy.log("Switch to MoveTool");
+    ImageView.selectMoveToolByButton();
+    cy.log("Select first rectangle");
+    ImageView.clickAtRelative(0.3, 0.3, { metaKey: true });
+    cy.log("Add second rectangle to selection with ctrl pressed");
+    ImageView.clickAtRelative(0.7, 0.3, { metaKey: true });
+    Sidebar.hasSelectedRegions(2);
+  });
+});
diff --git a/web/libs/frontend-test/src/helpers/LSF/ImageView.ts b/web/libs/frontend-test/src/helpers/LSF/ImageView.ts
@@ -107,6 +107,40 @@ export const ImageView = {
       this.drawRect(realX, realY, realWidth, realHeight, options);
     });
   },
+
+  /**
+   * Draws a polygon on the drawing area.
+   * @param {Array<[number, number]>} points
+   * @param {boolean} autoclose
+   * @param {MouseInteractionOptions} options
+   */
+  drawPolygon(points: [number, number][], autoclose, options: MouseInteractionOptions = {}) {
+    const drawingArea = this.drawingArea.scrollIntoView();
+    if (autoclose) {
+      points = [...points, points[0]];
+    }
+    points.forEach((point, index) => {
+      drawingArea
+        .trigger("mousemove", point[0], point[1], { eventConstructor: "MouseEvent", ...options })
+        .trigger("mousedown", point[0], point[1], { eventConstructor: "MouseEvent", buttons: 1, ...options })
+        .trigger("mouseup", point[0], point[1], { eventConstructor: "MouseEvent", buttons: 1, ...options });
+    });
+  },
+
+  /**
+   * Draws the polygon on the drawing area with coordinates relative to the drawing area.
+   * @param {Array<[number, number]>} points
+   * @param {boolean} autoclose
+   * @param {MouseInteractionOptions} options
+   */
+  drawPolygonRelative(points: [number, number][], autoclose, options: MouseInteractionOptions = {}) {
+    this.drawingFrame.then((el) => {
+      const bbox: DOMRect = el[0].getBoundingClientRect();
+      const realPoints = points.map(([x, y]) => [x * bbox.width, y * bbox.height]);
+
+      this.drawPolygon(realPoints, autoclose, options);
+    });
+  },
   /**
    * Captures a screenshot of an element to compare later
    * @param {string} name name of the screenshot
@@ -152,6 +186,18 @@ export const ImageView = {
       .should("have.class", "lsf-tool_active");
   },
 
+  selectEllipseToolByButton() {
+    this.toolBar
+      .find('[aria-label="ellipse-tool"]')
+      .should("be.visible")
+      .click()
+      .should("have.class", "lsf-tool_active");
+  },
+
+  selectPolygonToolByButton() {
+    this.toolBar.find('[aria-label="polygon-tool"]').should("be.visible").click().should("have.class", "lsf-tool");
+  },
+
   selectMoveToolByButton() {
     this.toolBar.find('[aria-label="move-tool"]').should("be.visible").click().should("have.class", "lsf-tool_active");
   },
-Original file line number
+Diff line change
@@ Expand Up / @@ -689,7 +689,6 @@ export default observer( @@
           }
           item.freezeHistory();
-          item.setSkipInteractions(false);
           return this.triggerMouseUp(e, e.evt.offsetX, e.evt.offsetY);
         };
@@ Expand Down @@