fix: LSDV-4684: Fix check for image boundaries (#1294)

* fix: LSDV-4684: Fix check for image boundaries

When DEV-3793 FF is on this check is broken.
Have to check against 100 instead of real sizes.

* Test that regions can't be created outside of the canvas

* Adjust tests

* Test fine tuning

* Fix FF init

* Fix ellipses behavior

* Change waiting order

Co-authored-by: Sergey <[email protected]>

* Fix regression and tests

---------

Co-authored-by: Nick Skriabin <[email protected]>
Co-authored-by: Nick Skriabin <[email protected]>
Co-authored-by: Sergey <[email protected]>

Author: 3 people

e2e/fragments/LabelStudio.js

@@ -1,6 +1,5 @@
 const { I } = inject();
 const Helpers = require('../tests/helpers');
-const Asserts = require('../utils/asserts');
 
 module.exports = {
   init({ events = {}, ...params }) {

e2e/tests/image.test.js

@@ -13,6 +13,15 @@ const config = `
     </RectangleLabels>
   </View>`;
 
+const configEllipse = `
+  <View>
+    <Image name="img" value="$image"></Image>
+    <EllipseLabels name="tag" toName="img">
+      <Label value="Planet"></Label>
+      <Label value="Moonwalker" background="blue"></Label>
+    </EllipseLabels>
+  </View>`;
+
 const perRegionConfig = `
   <View>
     <Image name="img" value="$image"></Image>
@@ -138,3 +147,150 @@ Scenario('Image with perRegion tags', async function({ I, AtImageView, AtSidebar
   assert.strictEqual(result.length, 1);
   assert.deepStrictEqual(result[0].value.rectanglelabels, ['Moonwalker']);
 });
+
+const outOfBoundsFFs = new DataTable(['FF_DEV_3793'])
+
+outOfBoundsFFs.add([true]);
+outOfBoundsFFs.add([false])
+
+Data(outOfBoundsFFs)
+  .Scenario('Can\'t create rectangles outside of canvas', async ({
+    I,
+    AtLabels,
+    AtSidebar,
+    AtImageView,
+    LabelStudio,
+    current,
+  }) => {
+    LabelStudio.setFeatureFlags({
+      fflag_fix_front_dev_3793_relative_coords_short: current.FF_DEV_3793,
+    });
+
+    I.amOnPage('/');
+
+    LabelStudio.init({
+      config,
+      data: { image },
+      task: {
+        id: 0,
+        annotations: [{ id: 1001, result: [] }],
+        predictions: [],
+      },
+    });
+
+    await AtImageView.waitForImage();
+    await AtImageView.lookForStage();
+
+    const stage = AtImageView.stageBBox();
+
+    I.say('Drawing region in the upper left corner');
+    AtLabels.clickLabel('Planet');
+    AtImageView.drawByDrag(100, 100, -200, -200);
+
+    I.say('Drawing region in the upper right corner');
+    AtLabels.clickLabel('Planet');
+    AtImageView.drawByDrag(stage.width - 100, 100, stage.width + 100, -100);
+
+    I.say('Drawing region in the bottom left corner');
+    AtLabels.clickLabel('Planet');
+    AtImageView.drawByDrag(100, stage.height - 100, -100, stage.height + 100);
+
+    I.say('Drawing region in the bottom right corner');
+    AtLabels.clickLabel('Planet');
+    AtImageView.drawByDrag(stage.width - 100, stage.height - 100, stage.width + 100, stage.height + 100);
+
+    AtSidebar.seeRegions(4);
+
+    const result = await LabelStudio.serialize();
+
+    const [r1, r2, r3, r4] = result.map(r => r.value);
+
+    I.say('First region should be in the corner');
+    assert.strictEqual(r1.x, 0);
+    assert.equal(r1.y, 0);
+
+    I.say('Second region should be in the corner');
+    assert.equal(Math.round(r2.x + r2.width), 100);
+    assert.equal(r2.y, 0);
+
+    I.say('Third region should be in the corner');
+    assert.equal(r3.x, 0);
+    assert.equal(Math.round(r3.y + r3.height), 100);
+
+    I.say('Fourth region should be in the corner');
+    assert.equal(Math.round(r4.x + r4.width), 100);
+    assert.equal(Math.round(r4.y + r4.height), 100);
+  });
+
+Data(outOfBoundsFFs).
+  Scenario('Can\'t create ellipses outside of canvas', async ({
+    I,
+    AtLabels,
+    AtSidebar,
+    AtImageView,
+    LabelStudio,
+    current,
+  }) => {
+    LabelStudio.setFeatureFlags({
+      fflag_fix_front_dev_3793_relative_coords_short: current.FF_DEV_3793,
+    });
+
+    I.amOnPage('/');
+
+    LabelStudio.init({
+      config: configEllipse,
+      data: { image },
+      task: {
+        id: 0,
+        annotations: [{ id: 1001, result: [] }],
+        predictions: [],
+      },
+    });
+
+    await AtImageView.waitForImage();
+    await AtImageView.lookForStage();
+
+    const stage = AtImageView.stageBBox();
+    const ellipses = [
+      // top-left corner
+      [100, 100, -200, -200],
+      // top-right corner
+      [stage.width - 100, 100, stage.width + 100, -100],
+      // bottom-left corner
+      [100, stage.height - 100, -100, stage.height + 100],
+      // bottom-right corner
+      [stage.width - 100, stage.height - 100, stage.width + 100, stage.height + 100],
+    ];
+
+    for (const ellipse of ellipses) {
+      I.say('Drawing region in the upper left corner');
+      AtLabels.clickLabel('Planet');
+      AtImageView.drawByDrag(...ellipse);
+    }
+
+    AtSidebar.seeRegions(4);
+
+    const result = await LabelStudio.serialize();
+    const radiusX = 100 / stage.width * 100;
+    const radiusY = 100 / stage.height * 100;
+
+    for (let i = 0; i < result.length; i++) {
+      const res = result[i].value;
+      const region = ellipses[i];
+
+      I.say('Make sure ellipse radius is correct (should be same for all)');
+      // toFixed is to bypass JS floating point precision limitations (f32 sucks)
+      assert.strictEqual(res.radiusX.toFixed(3), radiusX.toFixed(3));
+      assert.strictEqual(res.radiusY.toFixed(3), radiusY.toFixed(3));
+
+      I.say('Make sure that center is in correct spot');
+      const [expectedX, expectedY] = [
+        region[0] / stage.width * 100,
+        region[1] / stage.height * 100,
+      ];
+
+      assert.strictEqual(res.x.toFixed(3), expectedX.toFixed(3));
+      assert.strictEqual(res.y.toFixed(3), expectedY.toFixed(3));
+    }
+  });
+

src/mixins/DrawingTool.js

@@ -12,7 +12,7 @@ const DrawingTool = types
     unselectRegionOnToolChange: true,
   })
   .volatile(() => {
-    return { 
+    return {
       currentArea: null,
     };
   })
@@ -230,16 +230,30 @@ const TwoPointsDrawingTool = DrawingTool.named('TwoPointsDrawingTool')
         const shape = self.getCurrentArea();
 
         if (!shape) return;
-        const { stageWidth, stageHeight } = self.obj;
+        const isEllipse = shape.type.includes('ellipse');
+        const maxStageWidth = isFF(FF_DEV_3793) ? 100 : self.obj.stageWidth;
+        const maxStageHeight = isFF(FF_DEV_3793) ? 100 : self.obj.stageHeight;
 
-        let { x1, y1, x2, y2 } = Utils.Image.reverseCoordinates({ x: shape.startX, y: shape.startY }, { x, y });
+        let { x1, y1, x2, y2 } = isEllipse ? {
+          x1: shape.startX,
+          y1: shape.startY,
+          x2: x,
+          y2: y,
+        } : Utils.Image.reverseCoordinates({ x: shape.startX, y: shape.startY }, { x, y });
 
         x1 = Math.max(0, x1);
         y1 = Math.max(0, y1);
-        x2 = Math.min(stageWidth, x2);
-        y2 = Math.min(stageHeight, y2);
+        x2 = Math.min(maxStageWidth, x2);
+        y2 = Math.min(maxStageHeight, y2);
+
+        let [distX, distY] = [x2 - x1, y2 - y1].map(Math.abs);
+
+        if (isEllipse) {
+          distX = Math.min(distX, Math.min(x1, maxStageWidth - x1));
+          distY = Math.min(distY, Math.min(y1, maxStageHeight - y1));
+        }
 
-        shape.setPositionInternal(x1, y1, x2 - x1, y2 - y1, shape.rotation);
+        shape.setPositionInternal(x1, y1, distX, distY, shape.rotation);
       },
 
       finishDrawing(x, y) {
@@ -420,7 +434,7 @@ const MultipleClicksDrawingTool = DrawingTool.named('MultipleClicksMixin')
       },
 
       drawDefault() {
-        const { x,y } = startPoint;
+        const { x, y } = startPoint;
         let dX = self.defaultDimensions.length;
         let dY = self.defaultDimensions.length;