Merge pull request #1156 from psjostrom/annotate-scaling-bug

fix: SDA-2769, SDA-2789, SDA-2790 Fixing scaling bug etc

package.json

@@ -161,7 +161,6 @@
     "electron-spellchecker": "git+https://github.com/symphonyoss/electron-spellchecker.git#v2.0.4",
     "ffi-napi": "3.0.0",
     "filesize": "6.1.0",
-    "image-size": "^0.9.3",
     "lazy-brush": "^1.0.1",
     "react": "16.13.0",
     "react-dom": "16.13.0",

src/app/screen-snippet-handler.ts

@@ -1,6 +1,5 @@
-import { app, BrowserWindow, ipcMain } from 'electron';
+import { app, BrowserWindow, ipcMain, nativeImage } from 'electron';
 import * as fs from 'fs';
-import sizeOf from 'image-size';
 import * as os from 'os';
 import * as path from 'path';
 
@@ -26,7 +25,7 @@ const readFile = util.promisify(fs.readFile);
 class ScreenSnippet {
   private readonly tempDir: string;
   private readonly captureUtil: string;
-  private outputFileName: string | undefined;
+  private outputFilePath: string | undefined;
   private captureUtilArgs: ReadonlyArray<string> | undefined;
   private child: ChildProcess | undefined;
   private focusedWindow: BrowserWindow | null = null;
@@ -70,31 +69,31 @@ class ScreenSnippet {
       }
     }
     logger.info(`screen-snippet-handler: Starting screen capture!`);
-    this.outputFileName = path.join(
+    this.outputFilePath = path.join(
       this.tempDir,
       'symphonyImage-' + Date.now() + '.png',
     );
     if (isMac) {
-      this.captureUtilArgs = ['-i', '-s', '-t', 'png', this.outputFileName];
+      this.captureUtilArgs = ['-i', '-s', '-t', 'png', this.outputFilePath];
     } else if (isWindowsOS) {
       if (windowHandler.isMana) {
         this.captureUtilArgs = [
           '--no-annotate',
-          this.outputFileName,
+          this.outputFilePath,
           i18n.getLocale(),
         ];
       } else {
-        this.captureUtilArgs = [this.outputFileName, i18n.getLocale()];
+        this.captureUtilArgs = [this.outputFilePath, i18n.getLocale()];
       }
     } else if (isLinux) {
-      this.captureUtilArgs = ['-a', '-f', this.outputFileName];
+      this.captureUtilArgs = ['-a', '-f', this.outputFilePath];
     } else {
       this.captureUtilArgs = [];
     }
     this.focusedWindow = BrowserWindow.getFocusedWindow();
 
     logger.info(
-      `screen-snippet-handler: Capturing snippet with file ${this.outputFileName} and args ${this.captureUtilArgs}!`,
+      `screen-snippet-handler: Capturing snippet with file ${this.outputFilePath} and args ${this.captureUtilArgs}!`,
     );
 
     // only allow one screen capture at a time.
@@ -107,14 +106,15 @@ class ScreenSnippet {
     try {
       await this.execCmd(this.captureUtil, this.captureUtilArgs);
       if (windowHandler.isMana) {
-        const dimensions = this.getImageSize();
+        logger.info('screen-snippet-handler: Attempting to extract image dimensions from: ' + this.outputFilePath);
-        const imageSize = { width: dimensions?.width || -1, height: dimensions?.height || -1 };
+        const dimensions = this.getImageDimensions(this.outputFilePath);
-        if (imageSize.width === -1 || imageSize.width === -1) {
+        logger.info('screen-snippet-handler: Extracted dimensions from image: ' + JSON.stringify(dimensions));
+        if (!dimensions) {
           logger.error('screen-snippet-handler: Could not get image size');
           return;
         }
         windowHandler.closeSnippingToolWindow();
-        windowHandler.createSnippingToolWindow(this.outputFileName, imageSize);
+        windowHandler.createSnippingToolWindow(this.outputFilePath, dimensions);
         this.uploadSnippet(webContents);
         this.sendAnalytics();
       return;
@@ -125,14 +125,14 @@ class ScreenSnippet {
         type,
       }: IScreenSnippet = await this.convertFileToData();
       logger.info(
-        `screen-snippet-handler: Snippet captured! Sending data to SFE`,
+        `screen-snippet-handler: Snippet captured! Sending data straight to SFE without opening annotate tool`,
       );
       webContents.send('screen-snippet-data', { message, data, type });
       await this.verifyAndUpdateAlwaysOnTop();
     } catch (error) {
       await this.verifyAndUpdateAlwaysOnTop();
       logger.error(
-        `screen-snippet-handler: screen capture failed with error: ${error}!`,
+        `screen-snippet-handler: screen capture failed, user probably escaped the capture. Error: ${error}!`,
       );
     }
   }
@@ -208,13 +208,13 @@ class ScreenSnippet {
    */
   private async convertFileToData(): Promise<IScreenSnippet> {
     try {
-      if (!this.outputFileName) {
+      if (!this.outputFilePath) {
         logger.info(
           `screen-snippet-handler: screen capture failed! output file doesn't exist!`,
         );
         return { message: 'output file name is required', type: 'ERROR' };
       }
-      const data = await readFile(this.outputFileName);
+      const data = await readFile(this.outputFilePath);
       if (!data) {
         logger.info(
           `screen-snippet-handler: screen capture failed! data doesn't exist!`,
@@ -235,30 +235,9 @@ class ScreenSnippet {
       if (this.focusedWindow && windowExists(this.focusedWindow)) {
         this.focusedWindow.moveTop();
       }
-      // remove tmp file (async)
-      if (this.outputFileName) {
-        this.deleteFile(this.outputFileName);
-      }
     }
   }
 
-  /**
-   * Deletes a locally stored file
-   * @param filePath Path for the file to delete
-   */
-  private deleteFile(filePath: string) {
-    fs.unlink(filePath, (removeErr) => {
-      logger.info(
-        `screen-snippet-handler: cleaning up temp snippet file: ${filePath}!`,
-      );
-      if (removeErr) {
-        logger.error(
-          `screen-snippet-handler: error removing temp snippet file: ${filePath}, err: ${removeErr}`,
-        );
-      }
-    });
-  }
-
   /**
    * Verify and updates always on top
    */
@@ -270,23 +249,17 @@ class ScreenSnippet {
   }
 
   /**
-   * Gets the height & width of an image
+   * Gets the dimensions of an image
+   * @param filePath path to file to get image dimensions of
    */
-  private getImageSize():
+  private getImageDimensions(filePath: string): {
-    | {
+    height: number;
-      height: number | undefined;
+    width: number;
-      width: number | undefined;
+  } {
-    }
+    const img = nativeImage.createFromPath(filePath);
-    | undefined {
+    const size = img.getSize();
-    if (!this.outputFileName) {
-      return undefined;
-    }
 
-    const dimensions = sizeOf(this.outputFileName);
+    return size;
-    return {
-      height: dimensions.height,
-      width: dimensions.width,
-    };
   }
 
   /**
@@ -303,25 +276,21 @@ class ScreenSnippet {
           data,
           type,
         };
-        logger.info(
+        logger.info('screen-snippet-handler: Snippet uploaded correctly, sending payload to SFE');
-          `screen-snippet-handler: Snippet captured! Sending data to SFE`,
-        );
         webContents.send('screen-snippet-data', payload);
         await this.verifyAndUpdateAlwaysOnTop();
       } catch (error) {
         await this.verifyAndUpdateAlwaysOnTop();
         logger.error(
-          `screen-snippet-handler: screen capture failed with error: ${error}!`,
+          `screen-snippet-handler: upload of screen capture failed with error: ${error}!`,
         );
-      } finally {
-        this.deleteFile(snippetData.screenSnippetPath);
       }
     });
   }
 
-  /**
+    /**
-   * Send analytics data to analytics module
+     * Send analytics data to analytics module
-   */
+     */
   private sendAnalytics() {
     ipcMain.on('send-tracking-data-to-main', async (_event, eventData: { element: AnalyticsElements, type: ScreenSnippetActionTypes }) => {
       analytics.track({element: eventData.element, action_type: eventData.type});

src/app/window-handler.ts

@@ -992,6 +992,11 @@ export class WindowHandler {
       return;
     }
 
+    logger.info('window-handler, createSnippingToolWindow: Receiving snippet props: ' + JSON.stringify({ snipImage, snipDimensions }));
+
+    const allDisplays = electron.screen.getAllDisplays();
+    logger.info('window-handler, createSnippingToolWindow: User has these displays: ' + JSON.stringify(allDisplays));
+
     const electronWindows = BrowserWindow.getAllWindows();
     const mainWindow = electronWindows[0];
 
@@ -1013,31 +1018,35 @@ export class WindowHandler {
     const maxToolWidth = Math.floor(calculatePercentage(workAreaSize.width, 90));
     const availableAnnotateAreaHeight = maxToolHeight - BUTTON_BARS_HEIGHT;
     const availableAnnotateAreaWidth = maxToolWidth;
+    const scaleFactor = display.scaleFactor;
+    const scaledImageDimensions = { height: Math.floor(snipDimensions.height / scaleFactor), width: Math.floor(snipDimensions.width / scaleFactor) };
+    logger.info('window-handler, createSnippingToolWindow: Image will open with scaled dimensions: ' + JSON.stringify(scaledImageDimensions));
+    // const scaledImageDimensions = snipDimensions;
 
-    const annotateAreaHeight = snipDimensions.height > availableAnnotateAreaHeight ?
+    const annotateAreaHeight = scaledImageDimensions.height > availableAnnotateAreaHeight ?
       availableAnnotateAreaHeight :
-      snipDimensions.height;
+      scaledImageDimensions.height;
-    const annotateAreaWidth = snipDimensions.width > availableAnnotateAreaWidth ?
+    const annotateAreaWidth = scaledImageDimensions.width > availableAnnotateAreaWidth ?
       availableAnnotateAreaWidth :
-      snipDimensions.width;
+      scaledImageDimensions.width;
 
     let toolHeight: number;
     let toolWidth: number;
 
-    if (snipDimensions.height + BUTTON_BARS_HEIGHT >= maxToolHeight) {
+    if (scaledImageDimensions.height + BUTTON_BARS_HEIGHT >= maxToolHeight) {
       toolHeight = maxToolHeight + OS_PADDING;
-    } else if (snipDimensions.height + BUTTON_BARS_HEIGHT <= MIN_TOOL_HEIGHT) {
+    } else if (scaledImageDimensions.height + BUTTON_BARS_HEIGHT <= MIN_TOOL_HEIGHT) {
       toolHeight = MIN_TOOL_HEIGHT + OS_PADDING;
     } else {
-      toolHeight = snipDimensions.height + BUTTON_BARS_HEIGHT + OS_PADDING;
+      toolHeight = scaledImageDimensions.height + BUTTON_BARS_HEIGHT + OS_PADDING;
     }
 
-    if (snipDimensions.width >= maxToolWidth) {
+    if (scaledImageDimensions.width >= maxToolWidth) {
       toolWidth = maxToolWidth;
-    } else if (snipDimensions.width <= MIN_TOOL_WIDTH) {
+    } else if (scaledImageDimensions.width <= MIN_TOOL_WIDTH) {
       toolWidth = MIN_TOOL_WIDTH;
     } else {
-      toolWidth = snipDimensions.width;
+      toolWidth = scaledImageDimensions.width;
     }
 
     const opts: ICustomBrowserWindowConstructorOpts = this.getWindowOpts(
@@ -1050,7 +1059,7 @@ export class WindowHandler {
         fullscreenable: false,
       },
       {
-        devTools: isDevEnv,
+        devTools: true,
       },
     );
 
@@ -1075,13 +1084,20 @@ export class WindowHandler {
         snipImage,
         annotateAreaHeight,
         annotateAreaWidth,
-        snippetImageHeight: snipDimensions.height,
+        snippetImageHeight: scaledImageDimensions.height,
-        snippetImageWidth: snipDimensions.width,
+        snippetImageWidth: scaledImageDimensions.width,
       };
       if (this.snippingToolWindow && windowExists(this.snippingToolWindow)) {
+        const windowBounds = this.snippingToolWindow.getBounds();
         logger.info('window-handler: Opening snipping tool window on display: ' + JSON.stringify(display));
         logger.info('window-handler: Opening snipping tool window with size: ' + JSON.stringify({ toolHeight, toolWidth }));
         logger.info('window-handler: Opening snipping tool content with metadata: ' + JSON.stringify(snippingToolInfo));
+        logger.info('window-handler: Actual window size: ' + JSON.stringify(windowBounds));
+        if (windowBounds.height !== toolHeight || windowBounds.width !== toolWidth) {
+          logger.info('window-handler: Could not create window with correct size, resizing with setBounds');
+          this.snippingToolWindow.setBounds({ height: toolHeight, width: toolWidth });
+          logger.info('window-handler: window bounds after resizing: ' + JSON.stringify(this.snippingToolWindow.getBounds()));
+        }
         this.snippingToolWindow.webContents.send(
           'snipping-tool-data',
           snippingToolInfo,
@@ -1090,6 +1106,8 @@ export class WindowHandler {
     });
 
     this.snippingToolWindow.once('closed', () => {
+      logger.info('window-handler, createSnippingToolWindow: Closing snipping window, attempting to delete temp snip image');
+      this.deleteFile(snipImage);
       this.removeWindow(opts.winKey);
       this.screenPickerWindow = null;
     });
@@ -1651,6 +1669,23 @@ export class WindowHandler {
     });
   }
 
+  /**
+   * Deletes a locally stored file
+   * @param filePath Path for the file to delete
+   */
+  public deleteFile(filePath: string) {
+    fs.unlink(filePath, (removeErr) => {
+      logger.info(
+        `window-handler: cleaning up temp snippet file: ${filePath}!`,
+      );
+      if (removeErr) {
+        logger.info(
+          `window-handler: error removing temp snippet file, is probably already removed: ${filePath}, err: ${removeErr}`,
+        );
+      }
+    });
+  }
+
   /**
    * Reloads symphony in case of network failures
    */

src/renderer/components/snipping-tool.tsx

@@ -281,8 +281,8 @@ const SnippingTool: React.FunctionComponent<ISnippingToolProps> = ({ existingPat
         <div className='imageContainer'>
           <AnnotateArea
             data-testid='annotate-component'
-          paths={paths}
+            paths={paths}
-          highlightColor={highlightColor.rgbaColor}
+            highlightColor={highlightColor.rgbaColor}
             penColor={penColor.rgbaColor}
             onChange={setPaths}
             imageDimensions={imageDimensions}