#5097 - Selection frame is displayed improperly for overlapped images (…

…#5756)

* #5097 - Selection frame is displayed improperly for overlapped images (#5647)

* #5097 - гpdated selection frame display for images and simple objects
* #5097 - updated tests
* #5097 - updated width of stroke to pass tests

(cherry picked from commit f49f357)

* Backmerge: #5581 - fixed view only toggle fragment issue and multitail arrows undo\redo not saving coordinates (#5736)

(cherry picked from commit f9c4df6)

---------

Co-authored-by: Daniil Sloboda <daniil_sloboda@epam.com>

packages/ketcher-core/__tests__/application/render/restruct/resimpleObject.test.ts

@@ -59,7 +59,7 @@ it('should get hover path and style for simple objects correctly', () => {
     const render = new Render(document as unknown as HTMLElement, option);
     const paths = reSimpleObject.hoverPath(render);
     expect(
-      paths.filter((path) => path.path.attrs.fill === '#fff')?.length,
+      paths.filter((path) => path.path.attrs.stroke === '#CCFFDD')?.length,
     ).toBeGreaterThanOrEqual(1);
   });
 });

packages/ketcher-core/src/application/editor/operations/image/imageResize.ts

@@ -64,8 +64,9 @@ export class ImageResize extends BaseOperation {
     }
 
     item.resize(topLeftPosition, bottomRightPosition);
+    const next = renderItem.visel.paths[0].next;
     reStruct.clearVisel(renderItem.visel);
-    renderItem.show(reStruct, reStruct.render.options);
+    renderItem.show(reStruct, reStruct.render.options, next);
   }
 
   invert(): BaseOperation {

packages/ketcher-core/src/application/editor/operations/multitailArrow/multitailArrowAddRemoveTail.ts

@@ -2,9 +2,14 @@
 import { BaseOperation } from 'application/editor/operations/base';
 import { OperationType } from 'application/editor';
 import { ReStruct } from 'application/render';
+import { FixedPrecisionCoordinates } from 'domain/entities';
 
 export class MultitailArrowAddTail extends BaseOperation {
-  constructor(private itemId: number, private tailId?: number) {
+  constructor(
+    private itemId: number,
+    private tailId?: number,
+    private coordinate?: FixedPrecisionCoordinates,
+  ) {
     super(OperationType.MULTITAIL_ARROW_ADD_TAIL);
   }
 
@@ -15,8 +20,7 @@ export class MultitailArrowAddTail extends BaseOperation {
     if (!reMultitailArrow || !multitailArrow) {
       return;
     }
-
-    this.tailId = multitailArrow.addTail(this.tailId);
+    this.tailId = multitailArrow.addTail(this.tailId, this.coordinate);
     BaseOperation.invalidateItem(reStruct, 'multitailArrows', this.itemId, 1);
   }
 
@@ -26,15 +30,17 @@ export class MultitailArrowAddTail extends BaseOperation {
 }
 
 export class MultitailArrowRemoveTail extends BaseOperation {
+  private coordinate?: FixedPrecisionCoordinates;
   constructor(private itemId: number, private tailId: number) {
     super(OperationType.MULTITAIL_ARROW_REMOVE_TAIL);
   }
 
   execute(reStruct: ReStruct) {
     const reMultitailArrow = reStruct.multitailArrows.get(this.itemId);
     const multitailArrow = reStruct.molecule.multitailArrows.get(this.itemId);
+    this.coordinate = multitailArrow?.getTailCoordinate(this.tailId);
 
-    if (!reMultitailArrow || !multitailArrow) {
+    if (!reMultitailArrow || !multitailArrow || !this.coordinate) {
       return;
     }
 
@@ -43,6 +49,6 @@ export class MultitailArrowRemoveTail extends BaseOperation {
   }
 
   invert(): BaseOperation {
-    return new MultitailArrowAddTail(this.itemId, this.tailId);
+    return new MultitailArrowAddTail(this.itemId, this.tailId, this.coordinate);
   }
 }

packages/ketcher-core/src/application/render/options.ts

@@ -101,6 +101,11 @@ function defaultOptions(renderOptions: RenderOptions): RenderOptions {
       fill: '#CCFFDD',
       'stroke-width': (0.6 * scaleFactorMicro) / 20,
     },
+    innerHoverStyle: {
+      stroke: '#CCFFDD',
+      fill: 'none',
+      'stroke-width': (4.6 * scaleFactorMicro) / 20,
+    },
     sgroupBracketStyle: {
       stroke: 'darkgray',
       'stroke-width': (0.5 * scaleFactorMicro) / 20,

packages/ketcher-core/src/application/render/render.types.ts

@@ -77,6 +77,7 @@ export type RenderOptions = {
   bondSnappingStyle: RenderOptionStyles;
   selectionStyle: RenderOptionStyles;
   hoverStyle: RenderOptionStyles;
+  innerHoverStyle: RenderOptionStyles;
   movingStyle: RenderOptionStyles;
   sgroupBracketStyle: RenderOptionStyles;
   lassoStyle: RenderOptionStyles;

packages/ketcher-core/src/application/render/restruct/generalEnumTypes.ts

@@ -20,9 +20,10 @@
 */
 export enum LayerMap {
   background = 'background',
+  images = 'images',
   selectionPlate = 'selectionPlate',
+  selectionPoints = 'selectionPoints',
   hovering = 'hovering',
-  images = 'images',
   atom = 'atom',
   bondSkeleton = 'bondSkeleton',
   warnings = 'warnings',

packages/ketcher-core/src/application/render/restruct/reImage.ts

@@ -162,46 +162,63 @@ export class ReImage extends ReObject {
       this.selectionPointsSet.push(element);
     });
     reStruct.addReObjectPath(
-      LayerMap.selectionPlate,
+      LayerMap.selectionPoints,
       this.visel,
       this.selectionPointsSet,
     );
   }
 
-  show(restruct: ReStruct, renderOptions: RenderOptions) {
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  show(restruct: ReStruct, renderOptions: RenderOptions, nextPath?: any) {
     const scaledTopLeftWithOffset = this.getScaledPointWithOffset(
       this.image.getTopLeftPosition(),
       renderOptions,
     );
     const dimensions = this.getDimensions(renderOptions);
 
-    restruct.addReObjectPath(
-      LayerMap.images,
-      this.visel,
-      restruct.render.paper.image(
-        this.image.bitmap,
-        scaledTopLeftWithOffset.x,
-        scaledTopLeftWithOffset.y,
-        dimensions.x,
-        dimensions.y,
-      ),
+    const image = restruct.render.paper.image(
+      this.image.bitmap,
+      scaledTopLeftWithOffset.x,
+      scaledTopLeftWithOffset.y,
+      dimensions.x,
+      dimensions.y,
     );
+    restruct.addReObjectPath(LayerMap.images, this.visel, image);
+
+    if (nextPath) {
+      image.insertBefore(nextPath);
+    }
   }
 
   drawHover(render: Render) {
     const offset =
       this.getScale(render.options) *
       (1 + REFERENCE_POINT_LINE_WIDTH_MULTIPLIER);
+    const STROKE_WIDTH = 0.6;
+    const FILL_WIDTH = 2 * offset - STROKE_WIDTH;
     const { topLeftPosition, bottomRightPosition } =
       this.getSelectionReferencePositions(render.options);
     const outerBorderOffset = new Vec2(offset, offset);
     const topLeftCorner = topLeftPosition.sub(outerBorderOffset);
-    const dimensions = bottomRightPosition
-      .sub(topLeftPosition)
-      .add(outerBorderOffset.scaled(2));
-    const paths = render.paper
-      .rect(topLeftCorner.x, topLeftCorner.y, dimensions.x, dimensions.y)
-      .attr({ ...render.options.hoverStyle, fill: '#fff' });
+    const dimensions = bottomRightPosition.sub(topLeftPosition);
+    const dimensionsWithBorders = dimensions.add(outerBorderOffset.scaled(2));
+
+    const paths = [
+      render.paper
+        .rect(
+          topLeftCorner.x,
+          topLeftCorner.y,
+          dimensionsWithBorders.x,
+          dimensionsWithBorders.y,
+        )
+        .attr({ ...render.options.hoverStyle, fill: 'none' }),
+      render.paper
+        .rect(topLeftPosition.x, topLeftPosition.y, dimensions.x, dimensions.y)
+        .attr({
+          ...render.options.innerHoverStyle,
+          'stroke-width': FILL_WIDTH,
+        }),
+    ];
 
     render.ctab.addReObjectPath(LayerMap.hovering, this.visel, paths);
 

packages/ketcher-core/src/application/render/restruct/reobject.ts

@@ -19,6 +19,7 @@ import ReStruct from './restruct';
 import { Render } from '../raphaelRender';
 import { Scale } from 'domain/helpers';
 import Visel from './visel';
+import { IMAGE_KEY } from 'domain/constants';
 
 class ReObject {
   public visel: Visel;
@@ -33,7 +34,7 @@ class ReObject {
 
   changeSelectionStyle(options: any) {
     const { hoverStyle } = options;
-    if (this.visel.type === 'simpleObject') {
+    if (['simpleObject', IMAGE_KEY].includes(this.visel.type)) {
       this.hovering?.attr({
         'fill-opacity': this.selected ? 1 : 0,
       });

packages/ketcher-core/src/application/render/restruct/resimpleObject.ts

@@ -191,15 +191,11 @@ class ReSimpleObject extends ReObject {
     return refPoints;
   }
 
-  getHoverPathStyle(
-    path: any,
-    render: Render,
-    isOuterShapeOfHoverPath: boolean,
-  ) {
-    if (isOuterShapeOfHoverPath) {
-      return path.attr(render.options.hoverStyle);
+  getFigureHoverPath(path: any, render: Render, isBorder = true) {
+    if (isBorder) {
+      return path.attr({ ...render.options.hoverStyle, fill: 'none' });
     } else {
-      return path.attr({ ...render.options.hoverStyle, fill: '#fff' });
+      return path.attr(render.options.innerHoverStyle);
     }
   }
 
@@ -214,84 +210,88 @@ class ReSimpleObject extends ReObject {
     const paths: Array<StyledPath> = [];
 
     // TODO: It seems that inheritance will be the better approach here
+    const lineOffset = scaleFactor / 8;
     switch (this.item.mode) {
       case SimpleObjectMode.ellipse: {
         const rad = Vec2.diff(point[1], point[0]);
         const rx = rad.x / 2;
         const ry = rad.y / 2;
-        const outerEllipse = render.paper.ellipse(
-          tfx(point[0].x + rx),
-          tfx(point[0].y + ry),
-          tfx(Math.abs(rx) + scaleFactor / 8),
-          tfx(Math.abs(ry) + scaleFactor / 8),
+        const centerX = tfx(point[0].x + rx);
+        const centerY = tfx(point[0].y + ry);
+        const outerBorderEllipse = render.paper.ellipse(
+          centerX,
+          centerY,
+          tfx(Math.abs(rx) + lineOffset),
+          tfx(Math.abs(ry) + lineOffset),
         );
         paths.push({
-          path: this.getHoverPathStyle(outerEllipse, render, true),
+          path: this.getFigureHoverPath(outerBorderEllipse, render),
+          stylesApplied: true,
+        });
+
+        const fillEllipse = render.paper.ellipse(
+          centerX,
+          centerY,
+          tfx(Math.abs(rx)),
+          tfx(Math.abs(ry)),
+        );
+        paths.push({
+          path: this.getFigureHoverPath(fillEllipse, render, false),
           stylesApplied: true,
         });
         if (
           Math.abs(rx) - scaleFactor / 8 > 0 &&
           Math.abs(ry) - scaleFactor / 8 > 0
         ) {
-          const innerEllipse = render.paper.ellipse(
-            tfx(point[0].x + rx),
-            tfx(point[0].y + ry),
-            tfx(Math.abs(rx) - scaleFactor / 8),
-            tfx(Math.abs(ry) - scaleFactor / 8),
+          const innerBorderEllipse = render.paper.ellipse(
+            centerX,
+            centerY,
+            tfx(Math.abs(rx) - lineOffset),
+            tfx(Math.abs(ry) - lineOffset),
           );
           paths.push({
-            path: this.getHoverPathStyle(innerEllipse, render, false),
+            path: this.getFigureHoverPath(innerBorderEllipse, render),
             stylesApplied: true,
           });
         }
         break;
       }
 
       case SimpleObjectMode.rectangle: {
-        const outerRect = render.paper.rect(
-          tfx(Math.min(point[0].x, point[1].x) - scaleFactor / 8),
-          tfx(Math.min(point[0].y, point[1].y) - scaleFactor / 8),
-          tfx(
-            Math.max(point[0].x, point[1].x) -
-              Math.min(point[0].x, point[1].x) +
-              scaleFactor / 4,
-          ),
-          tfx(
-            Math.max(point[0].y, point[1].y) -
-              Math.min(point[0].y, point[1].y) +
-              scaleFactor / 4,
-          ),
+        const leftX = Math.min(point[0].x, point[1].x);
+        const topY = Math.min(point[0].y, point[1].y);
+        const rightX = Math.max(point[0].x, point[1].x) - leftX;
+        const bottomY = Math.max(point[0].y, point[1].y) - topY;
+
+        const outerBorderRect = render.paper.rect(
+          tfx(leftX - lineOffset),
+          tfx(topY - lineOffset),
+          tfx(rightX + 2 * lineOffset),
+          tfx(bottomY + 2 * lineOffset),
         );
         paths.push({
-          path: this.getHoverPathStyle(outerRect, render, true),
+          path: this.getFigureHoverPath(outerBorderRect, render),
           stylesApplied: true,
         });
-        if (
-          Math.max(point[0].x, point[1].x) -
-            Math.min(point[0].x, point[1].x) -
-            scaleFactor / 4 >
-            0 &&
-          Math.max(point[0].y, point[1].y) -
-            Math.min(point[0].y, point[1].y) -
-            scaleFactor / 4 >
-            0
-        ) {
+        const fillRect = render.paper.rect(
+          tfx(leftX),
+          tfx(topY),
+          tfx(rightX),
+          tfx(bottomY),
+        );
+        paths.push({
+          path: this.getFigureHoverPath(fillRect, render, false),
+          stylesApplied: true,
+        });
+        if (rightX - 2 * lineOffset > 0 && bottomY - 2 * lineOffset > 0) {
           const innerRect = render.paper.rect(
-            tfx(Math.min(point[0].x, point[1].x) + scaleFactor / 8),
-            tfx(Math.min(point[0].y, point[1].y) + scaleFactor / 8),
-            tfx(
-              Math.max(point[0].x, point[1].x) -
-                Math.min(point[0].x, point[1].x) -
-                scaleFactor / 4,
-            ),
-            tfx(
-              Math.max(point[0].y, point[1].y) -
-                Math.min(point[0].y, point[1].y) -
-                scaleFactor / 4,
-            ),
+            tfx(leftX + lineOffset),
+            tfx(topY + lineOffset),
+            tfx(rightX - 2 * lineOffset),
+            tfx(bottomY - 2 * lineOffset),
           );
           paths.push({
-            path: this.getHoverPathStyle(innerRect, render, false),
+            path: this.getFigureHoverPath(innerRect, render),
             stylesApplied: true,
           });
         }
@@ -342,7 +342,7 @@ class ReSimpleObject extends ReObject {
           p0.y - ((k * scaleFactor) / 8) * Math.cos(angle),
         );
         paths.push({
-          path: this.getHoverPathStyle(render.paper.path(poly), render, true),
+          path: render.paper.path(poly).attr(render.options.hoverStyle),
           stylesApplied: true,
         });
         break;
@@ -390,7 +390,7 @@ class ReSimpleObject extends ReObject {
       );
     });
     restruct.addReObjectPath(
-      LayerMap.selectionPlate,
+      LayerMap.selectionPoints,
       this.visel,
       this.selectionPointsSet,
     );

packages/ketcher-core/src/application/render/restruct/restruct.ts

@@ -84,7 +84,7 @@ class ReStruct {
   public multitailArrows = new Map<number, ReMultitailArrow>();
 
   private initialized = false;
-  private layers: Array<any> = [];
+  private layers: Record<LayerMap, any> = {} as Record<LayerMap, unknown>;
   public connectedComponents: Pool = new Pool();
   private ccFragmentType: Pool = new Pool();
   private structChanged = false;
@@ -789,7 +789,6 @@ class ReStruct {
           }
 
           this.showItemSelection(item, selected);
-          item.selectionPlate?.toBack();
         });
       }
     });