fix: LEAP-148: Fix MagicWand in MIG scenario with image preloading (#…

…5385) * [submodules] Copy src HumanSignal/label-studio-frontend Workflow run: https://github.com/HumanSignal/label-studio/actions/runs/7699172685 * ci: Build frontend Workflow run: https://github.com/HumanSignal/label-studio/actions/runs/7699195838 * [submodules] Copy src HumanSignal/label-studio-frontend Workflow run: https://github.com/HumanSignal/label-studio/actions/runs/7706462833 * ci: Build frontend Workflow run: https://github.com/HumanSignal/label-studio/actions/runs/7706472467 * [submodules] Copy src HumanSignal/label-studio-frontend Workflow run: https://github.com/HumanSignal/label-studio/actions/runs/7707296501 * ci: Build frontend Workflow run: https://github.com/HumanSignal/label-studio/actions/runs/7707317741 * [submodules] Copy src HumanSignal/label-studio-frontend Workflow run: https://github.com/HumanSignal/label-studio/actions/runs/7728686009 * ci: Build frontend Workflow run: https://github.com/HumanSignal/label-studio/actions/runs/7728724639 * [submodules] Copy src HumanSignal/label-studio-frontend Workflow run: https://github.com/HumanSignal/label-studio/actions/runs/7729000983 --------- Co-authored-by: Nick Skriabin <nr@fenelon.ru> Co-authored-by: robot-ci-heartex <robot-ci-heartex@users.noreply.github.com> Co-authored-by: hlomzik <hlomzik@gmail.com> Co-authored-by: Sergey <sergey.koshevarov@heartex.com>
HumanSignal · Jan 31, 2024 · 3750ef6 · 3750ef6
1 parent b442374
commit 3750ef6
Show file tree

Hide file tree

Showing 11 changed files with 299 additions and 156 deletions.
diff --git a/web/dist/libs/editor/main.js b/web/dist/libs/editor/main.js
diff --git a/web/dist/libs/editor/main.js.map b/web/dist/libs/editor/main.js.map
diff --git a/web/libs/editor/src/components/ImageView/Image.js b/web/libs/editor/src/components/ImageView/Image.js
@@ -30,6 +30,7 @@ export const Image = observer(forwardRef(({
   updateImageSize,
   usedValue,
   size,
+  overlay,
 }, ref) => {
   const imageSize = useMemo(() => {
     return {
@@ -45,6 +46,7 @@ export const Image = observer(forwardRef(({
 
   return (
     <Block name="image" style={imageSize}>
+      {overlay}
       <ImageProgress
         downloading={imageEntity.downloading}
         progress={imageEntity.progress}
@@ -76,7 +78,7 @@ const ImageProgress = observer(({
   return downloading ? (
     <Block name="image-progress">
       <Elem name="message">Downloading image</Elem>
-      <Elem tag="progress" name="bar" value={progress} min="0" max={1} step={0.0001}/>
+      <Elem tag="progress" name="bar" value={progress} min="0" max={1} step={0.0001} />
     </Block>
   ) : error ? (
     <ImageLoadingError src={src} value={usedValue} />
@@ -121,6 +123,6 @@ const ImageLoadingError = ({ src, value }) => {
   }, [src]);
 
   return (
-    <ErrorMessage error={error}/>
+    <ErrorMessage error={error} />
   );
 };
diff --git a/web/libs/editor/src/components/ImageView/ImageView.js b/web/libs/editor/src/components/ImageView/ImageView.js
@@ -490,6 +490,22 @@ const Crosshair = memo(forwardRef(({ width, height }, ref) => {
   );
 }));
 
+/**
+ * Component that creates an overlay on top
+ * of the image to support Magic Wand tool
+ */
+const CanvasOverlay = observer(({ item }) => {
+  return isFF(FF_DEV_4081) ? (
+    <canvas
+      className={styles.overlay}
+      ref={ref => {
+        item.setOverlayRef(ref);
+      }}
+      style={item.imageTransform}
+    />
+  ) : null;
+});
+
 export default observer(
   class ImageView extends Component {
     // stored position of canvas before creating region
@@ -1020,6 +1036,7 @@ export default observer(
                 imageTransform={item.imageTransform}
                 updateImageSize={item.updateImageSize}
                 size={item.canvasSize}
+                overlay={<CanvasOverlay item={item} />}
               />
             ) : (
               <div
@@ -1045,15 +1062,7 @@ export default observer(
                   crossOrigin={item.imageCrossOrigin}
                   alt="LS"
                 />
-                {isFF(FF_DEV_4081) ? (
-                  <canvas
-                    className={styles.overlay}
-                    ref={ref => {
-                      item.setOverlayRef(ref);
-                    }}
-                    style={item.imageTransform}
-                  />
-                ) : null}
+                <CanvasOverlay item={item} />
               </div>
             )}
             {/* @todo this is dirty hack; rewrite to proper async waiting for data to load */}

diff --git a/web/libs/editor/src/mixins/PerItem.js b/web/libs/editor/src/mixins/PerItem.js
@@ -22,6 +22,35 @@ const PerItemMixin = types
     },
   }))
   .actions(self => ({
+    /**
+     * Validates all values related to the current classification per item (Multi Items Segmentation).
+     *
+     * - This method should not be overridden.
+     * - It is used only in validate method of the ClassificationBase mixin.
+     *
+     * @returns {boolean}
+     * @private
+     */
+    _validatePerItem() {
+      const objectTag = self.toNameTag;
+
+      return self.annotation.regions
+        .every((reg) => {
+          const result = reg.results.find(s => s.from_name === self);
+
+          if (!result?.hasValue) {
+            return true;
+          }
+          const value = result.mainValue;
+          const isValid = self.validateValue(value);
+
+          if (!isValid) {
+            objectTag.setCurrentItem(reg.item_index);
+            return false;
+          }
+          return true;
+        });
+    },
     createPerItemResult() {
       self.createPerObjectResult({
         item_index: self.toNameTag.currentItemIndex,

diff --git a/web/libs/editor/src/mixins/PerRegion.js b/web/libs/editor/src/mixins/PerRegion.js
@@ -55,6 +55,30 @@ const PerRegionMixin = types
     },
   }))
   .actions(self => ({
+    /**
+     * Validates all values related to the current classification per region.
+     *
+     * - This method should not be overridden.
+     * - It is used only in validate method of the ClassificationBase mixin.
+     *
+     * @returns {boolean}
+     * @private
+     */
+    _validatePerRegion() {
+      const objectTag = self.toNameTag;
+
+      for (const reg of objectTag.allRegs) {
+        const value = reg.results.find(s => s.from_name === self)?.mainValue;
+        const isValid = self.validateValue(value);
+
+        if (!isValid) {
+          self.annotation.selectArea(reg);
+          return false;
+        }
+      }
+
+      return true;
+    },
     createPerRegionResult() {
       self.perRegionArea?.setValue(self);
     },

diff --git a/web/libs/editor/src/mixins/Required.js b/web/libs/editor/src/mixins/Required.js
@@ -6,71 +6,77 @@ const RequiredMixin = types
     required: types.optional(types.boolean, false),
     requiredmessage: types.maybeNull(types.string),
   })
-  .actions(self => ({
-    validate() {
-      if (!self.required) return true;
+  .actions(self => {
+    const Super = {
+      validate: self.validate,
+    };
 
-      if (self.perregion) {
+    return {
+      validate() {
+        if (!Super.validate()) return false;
+        if (!self.required) return true;
+
+        if (self.perregion) {
         // validating when choices labeling is done per region,
         // for example choice may be required to be selected for
         // every bbox
-        const objectTag = self.toNameTag;
+          const objectTag = self.toNameTag;
 
-        // if regions don't meet visibility conditions skip validation
-        for (const reg of objectTag.allRegs) {
-          const s = reg.results.find(s => s.from_name === self);
+          // if regions don't meet visibility conditions skip validation
+          for (const reg of objectTag.allRegs) {
+            const s = reg.results.find(s => s.from_name === self);
 
-          if (self.visiblewhen === 'region-selected') {
-            if (self.whentagname) {
-              const label = reg.labeling?.from_name?.name;
+            if (self.visiblewhen === 'region-selected') {
+              if (self.whentagname) {
+                const label = reg.labeling?.from_name?.name;
 
-              if (label && label !== self.whentagname) continue;
+                if (label && label !== self.whentagname) continue;
+              }
             }
-          }
 
-          if (self.whenlabelvalue && !reg.hasLabel(self.whenlabelvalue)) {
-            continue;
-          }
+            if (self.whenlabelvalue && !reg.hasLabel(self.whenlabelvalue)) {
+              continue;
+            }
 
-          if (!s?.hasValue) {
-            self.annotation.selectArea(reg);
-            self.requiredModal();
+            if (!s?.hasValue) {
+              self.annotation.selectArea(reg);
+              self.requiredModal();
 
-            return false;
+              return false;
+            }
           }
-        }
-      } else if (isFF(FF_LSDV_4583) && self.peritem) {
+        } else if (isFF(FF_LSDV_4583) && self.peritem) {
         // validating when choices labeling is done per item,
-        const objectTag = self.toNameTag;
-        const maxItemIndex = objectTag.maxItemIndex;
-        const existingResultsIndexes = self.annotation.regions
-          .reduce((existingResultsIndexes, reg) => {
-            const result = reg.results.find(s => s.from_name === self);
+          const objectTag = self.toNameTag;
+          const maxItemIndex = objectTag.maxItemIndex;
+          const existingResultsIndexes = self.annotation.regions
+            .reduce((existingResultsIndexes, reg) => {
+              const result = reg.results.find(s => s.from_name === self);
 
-            if (result?.hasValue) {
-              existingResultsIndexes.add(reg.item_index);
-            }
-            return existingResultsIndexes;
-          }, new Set());
+              if (result?.hasValue) {
+                existingResultsIndexes.add(reg.item_index);
+              }
+              return existingResultsIndexes;
+            }, new Set());
 
-        for (let idx = 0; idx <= maxItemIndex; idx++) {
-          if (!existingResultsIndexes.has(idx)) {
-            objectTag.setCurrentItem(idx);
-            self.requiredModal();
-            return false;
+          for (let idx = 0; idx <= maxItemIndex; idx++) {
+            if (!existingResultsIndexes.has(idx)) {
+              objectTag.setCurrentItem(idx);
+              self.requiredModal();
+              return false;
+            }
           }
-        }
-      } else {
+        } else {
         // validation when its classifying the whole object
         // isVisible can be undefined (so comparison is true) or boolean (so check for visibility)
-        if (!self.holdsState && self.isVisible !== false && getParent(self, 2)?.isVisible !== false) {
-          self.requiredModal();
-          return false;
+          if (!self.holdsState && self.isVisible !== false && getParent(self, 2)?.isVisible !== false) {
+            self.requiredModal();
+            return false;
+          }
         }
-      }
-
-      return true;
-    },
-  }));
+        return true;
+      },
+    };
+  });
 
 export default RequiredMixin;
diff --git a/web/libs/editor/src/tags/control/ClassificationBase.js b/web/libs/editor/src/tags/control/ClassificationBase.js
@@ -1,4 +1,5 @@
 import { types } from 'mobx-state-tree';
+import { FF_LSDV_4583, isFF } from '../../utils/feature-flags';
 
 /**
  * This is a mixin for a control-tag that is a base of creating classification-like tags.
@@ -40,6 +41,65 @@ const ClassificationBase = types.model('ClassificationBase', {
     };
   }).actions(self => {
     return {
+      /**
+       * Validates the input based on certain conditions.
+       *
+       * Generally, this method does not need to be overridden. And you need to override the validateValue method instead.
+       * However, there are exceptions. For example, RequiredMixin, Choices, and
+       * Taxonomy have their own additional logic, for which a broader context is needed.
+       * In this case, the parent method call is added at the beginning or end
+       * of the method to maintain all functionality in a predictable manner.
+       *
+       * @returns {boolean}
+       */
+      validate() {
+        if (self.perregion) {
+          return self._validatePerRegion();
+        } else if (self.peritem && isFF(FF_LSDV_4583)) {
+          return self._validatePerItem();
+        } else {
+          return self._validatePerObject();
+        }
+      },
+      /**
+       * Validates the value.
+       *
+       * Override to add your custom validation logic specific for the tag.
+       * Per-item, per-region and per-object validation will be applied automatically.
+       *
+       * @example
+       * SomeModel.actions(self => {
+       *     const Super = { validateValue: self.validateValue };
+       *
+       *     return {
+       *       validateValue(value) {
+       *         if (!Super.validateValue(value)) return false;
+       *         // your validation logic
+       *       }
+       *       // other actions
+       *     }
+       * });
+       *
+       * @param {*} value - The value to be validated.
+       * @returns {boolean}
+       *
+       */
+      // eslint-disable-next-line @typescript-eslint/no-unused-vars
+      validateValue(value) {
+        return true;
+      },
+      /**
+       * Validates all values related to the current classification per object.
+       *
+       * - This method should not be overridden.
+       * - It is used only in validate method of the ClassificationBase mixin.
+       *
+       * @returns {boolean}
+       * @private
+       */
+      _validatePerObject() {
+        return self.validateValue(self.selectedValues());
+      },
       createPerObjectResult(areaValues = {}) {
         self.annotation.createResult(
           areaValues,