Merge branch 'map-scaling-incorrectly-des-692'

Author: raksooo

gui/src/main/window-controller.ts

@@ -271,6 +271,10 @@ export default class WindowController {
     _display: Display,
     changedMetrics: string[],
   ) => {
+    if (changedMetrics.includes('scaleFactor')) {
+      IpcMainEventChannel.window.notifyScaleFactorChange?.();
+    }
+
     if (changedMetrics.includes('workArea') && this.window?.isVisible()) {
       this.onWorkAreaSizeChange();
       if (process.platform === 'win32') {

gui/src/renderer/components/Map.tsx

@@ -1,11 +1,11 @@
-import { useCallback, useEffect, useMemo, useRef, useState } from 'react';
+import { useCallback, useEffect, useMemo, useRef } from 'react';
 import styled from 'styled-components';
 
 import { TunnelState } from '../../shared/daemon-rpc-types';
 import log from '../../shared/logging';
 import { useAppContext } from '../context';
 import GlMap, { ConnectionState, Coordinate } from '../lib/3dmap';
-import { useCombinedRefs } from '../lib/utilityHooks';
+import { useCombinedRefs, useRerenderer } from '../lib/utilityHooks';
 import { useSelector } from '../redux/store';
 
 // Default to Gothenburg when we don't know the actual location.
@@ -22,8 +22,6 @@ interface MapParams {
   connectionState: ConnectionState;
 }
 
-type AnimationFrameCallback = (now: number, newParams?: MapParams) => void;
-
 export default function Map() {
   const connection = useSelector((state) => state.connection);
   const animateMap = useSelector((state) => state.settings.guiSettings.animateMap);
@@ -77,29 +75,47 @@ interface MapInnerProps extends MapParams {
 function MapInner(props: MapInnerProps) {
   const { getMapData } = useAppContext();
 
-  // Callback that should be passed to requestAnimationFrame. This is initialized after the canvas
-  // has been rendered.
-  const animationFrameCallback = useRef<AnimationFrameCallback>();
   // When location or connection state changes it's stored here until passed to 3dmap
   const newParams = useRef<MapParams>();
 
   // This is set to true when rendering should be paused
   const pause = useRef<boolean>(false);
 
+  const mapRef = useRef<GlMap>();
   const canvasRef = useRef<HTMLCanvasElement>();
-  const [canvasWidth, setCanvasWidth] = useState(window.innerWidth);
+  const width = applyPixelRatio(canvasRef.current?.clientWidth ?? window.innerWidth);
   // This constant is used for the height the first frame that is rendered only.
-  const [canvasHeight, setCanvasHeight] = useState(493);
+  const height = applyPixelRatio(canvasRef.current?.clientHeight ?? 493);
 
-  const updateCanvasSize = useCallback((canvas: HTMLCanvasElement) => {
-    const canvasRect = canvas.getBoundingClientRect();
+  // Hack to rerender when window size changes or when ref is set.
+  const [onSizeChange, sizeChangeCounter] = useRerenderer();
 
-    canvas.width = applyScaleFactor(canvasRect.width);
-    canvas.height = applyScaleFactor(canvasRect.height);
+  const render = useCallback(() => requestAnimationFrame(animationFrameCallback), []);
 
-    setCanvasWidth(canvasRect.width);
-    setCanvasHeight(canvasRect.height);
-  }, []);
+  const animationFrameCallback = useCallback(
+    (now: number) => {
+      now *= 0.001; // convert to seconds
+
+      // Propagate location change to the map
+      if (newParams.current) {
+        mapRef.current?.setLocation(
+          newParams.current.location,
+          newParams.current.connectionState,
+          now,
+          props.animate,
+        );
+        newParams.current = undefined;
+      }
+
+      mapRef.current?.draw(now);
+
+      // Stops rendering if pause is true. This happens when there is no ongoing movements
+      if (!pause.current) {
+        render();
+      }
+    },
+    [props.animate],
+  );
 
   // This is called when the canvas has been rendered the first time and initializes the gl context
   // and the map.
@@ -108,42 +124,19 @@ function MapInner(props: MapInnerProps) {
       return;
     }
 
-    updateCanvasSize(canvas);
+    onSizeChange();
 
     const gl = canvas.getContext('webgl2', { antialias: true })!;
 
-    const map = new GlMap(
+    mapRef.current = new GlMap(
       gl,
       await getMapData(),
       props.location,
       props.connectionState,
       () => (pause.current = true),
     );
 
-    // Function to be used when calling requestAnimationFrame
-    animationFrameCallback.current = (now: number) => {
-      now *= 0.001; // convert to seconds
-
-      // Propagate location change to the map
-      if (newParams.current) {
-        map.setLocation(
-          newParams.current.location,
-          newParams.current.connectionState,
-          now,
-          props.animate,
-        );
-        newParams.current = undefined;
-      }
-
-      map.draw(now);
-
-      // Stops rendering if pause is true. This happens when there is no ongoing movements
-      if (!pause.current) {
-        requestAnimationFrame(animationFrameCallback.current!);
-      }
-    };
-
-    requestAnimationFrame(animationFrameCallback.current);
+    render();
   }, []);
 
   // Set new params when the location or connection state has changed, and unpause if paused
@@ -155,41 +148,47 @@ function MapInner(props: MapInnerProps) {
 
     if (pause.current) {
       pause.current = false;
-      if (animationFrameCallback.current) {
-        requestAnimationFrame(animationFrameCallback.current);
-      }
+      render();
     }
   }, [props.location, props.connectionState]);
 
+  useEffect(() => {
+    mapRef.current?.updateViewport();
+    render();
+  }, [width, height, sizeChangeCounter]);
+
   // Resize canvas if window size changes
   useEffect(() => {
-    const resizeCallback = () => {
-      if (canvasRef.current) {
-        updateCanvasSize(canvasRef.current);
-      }
-    };
+    addEventListener('resize', onSizeChange);
+    return () => removeEventListener('resize', onSizeChange);
+  }, []);
 
-    addEventListener('resize', resizeCallback);
-    return () => removeEventListener('resize', resizeCallback);
-  }, [updateCanvasSize]);
+  useEffect(() => {
+    const unsubscribe = window.ipc.window.listenScaleFactorChange(onSizeChange);
+    return () => unsubscribe();
+  }, []);
 
   // Log new scale factor if it changes
-  useEffect(() => log.verbose('Map canvas scale factor:', window.devicePixelRatio), [
-    window.devicePixelRatio,
-  ]);
+  useEffect(() => {
+    log.verbose(`Map canvas scale factor: ${window.devicePixelRatio}, using: ${getPixelRatio()}`);
+  }, [window.devicePixelRatio]);
 
   const combinedCanvasRef = useCombinedRefs(canvasRef, canvasCallback);
 
-  return (
-    <StyledCanvas
-      ref={combinedCanvasRef}
-      width={applyScaleFactor(canvasWidth)}
-      height={applyScaleFactor(canvasHeight)}
-    />
-  );
+  return <StyledCanvas ref={combinedCanvasRef} width={width} height={height} />;
+}
+
+function getPixelRatio(): number {
+  let pixelRatio = window.devicePixelRatio;
+
+  // Wayland renders non-integer values as the next integer and then scales it back down.
+  if (window.env.platform === 'linux') {
+    pixelRatio = Math.ceil(pixelRatio);
+  }
+
+  return pixelRatio;
 }
 
-function applyScaleFactor(dimension: number): number {
-  const scaleFactor = window.devicePixelRatio;
-  return Math.floor(dimension * scaleFactor);
+function applyPixelRatio(dimension: number): number {
+  return Math.floor(dimension * getPixelRatio());
 }

gui/src/renderer/lib/3dmap.ts

@@ -451,6 +451,10 @@ export default class GlMap {
     this.zoomAnimations = [];
   }
 
+  public updateViewport() {
+    this.gl.viewport(0, 0, this.gl.drawingBufferWidth, this.gl.drawingBufferHeight);
+  }
+
   // Move the location marker to `newCoordinate` (with state `connectionState`).
   // Queues an animation to `newCoordinate` if `animate` is true. Otherwise it moves
   // directly to that location.

gui/src/renderer/lib/utilityHooks.ts

@@ -69,3 +69,12 @@ export function useNormalBridgeSettings() {
   const bridgeSettings = useSelector((state) => state.settings.bridgeSettings);
   return bridgeSettings.normal;
 }
+
+// This hook returns a function that can be used to force a rerender of a component, and
+// additionally also returns a variable that can be used to trigger effects as a result. This is a
+// hack and should be avoided unless there are no better ways.
+export function useRerenderer(): [() => void, number] {
+  const [count, setCount] = useState(0);
+  const rerender = useCallback(() => setCount((count) => count + 1), []);
+  return [rerender, count];
+}

gui/src/shared/ipc-helpers.ts

@@ -6,7 +6,7 @@ import { capitalize } from './string-helpers';
 type Handler<T, R> = (callback: (arg: T) => R) => void;
 type Sender<T, R> = (arg: T) => R;
 type Notifier<T> = ((arg: T) => void) | undefined;
-type Listener<T> = (callback: (arg: T) => void) => void;
+type Listener<T> = (callback: (arg: T) => void) => () => void;
 
 interface MainToRenderer<T> {
   direction: 'main-to-renderer';
@@ -154,7 +154,9 @@ export function notifyRenderer<T>(): MainToRenderer<T> {
     direction: 'main-to-renderer',
     send: notifyRendererImpl,
     receive: (event, ipcRenderer) => (fn: (value: T) => void) => {
-      ipcRenderer.on(event, (_event, newState: T) => fn(newState));
+      const listener = (_event: unknown, newState: T) => fn(newState);
+      ipcRenderer.on(event, listener);
+      return () => ipcRenderer.off(event, listener);
     },
   };
 }

gui/src/shared/ipc-schema.ts

@@ -128,6 +128,7 @@ export const ipcSchema = {
     shape: notifyRenderer<IWindowShapeParameters>(),
     focus: notifyRenderer<boolean>(),
     macOsScrollbarVisibility: notifyRenderer<MacOsScrollbarVisibility>(),
+    scaleFactorChange: notifyRenderer<void>(),
   },
   navigation: {
     reset: notifyRenderer<void>(),