Fix occlusion drift again (#3443)

* Fix occlusion drift * Fix image editor occasionally not loading fully * Fix occlusion disassociation when browsing * Address oversights * Fix translucent modifier applies to newly created shapes incorrectly * Fix i-text turns yellow upon immediate note change * Fix image occlusion hot keys not disabled when typing * Improve text label creation experience * Remove redundant functions * Fix error when adding occlusion (dae)
2025-09-18 14:02:21 -04:00 · 2024-10-02 03:19:52 -04:00 · 2024-10-02 03:19:52 -04:00 · d6aa95950d
commit d6aa95950d
parent 59969f62f5
15 changed files with 111 additions and 84 deletions
--- a/qt/aqt/editor.py
+++ b/qt/aqt/editor.py
@ -1125,7 +1125,7 @@ require("anki/ui").loaded.then(() => require("anki/NoteEditor").instances[0].too
        self.web.eval("resetIOImageLoaded()")

    def update_occlusions_field(self) -> None:
-        self.web.eval("updateOcclusionsField()")
+        self.web.eval("saveOcclusions()")

    def _setup_mask_editor(self, io_options: dict):
        self.web.eval(
--- a/ts/editor/NoteEditor.svelte
+++ b/ts/editor/NoteEditor.svelte
@ -459,7 +459,7 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
    }
    globalThis.setImageField = setImageField;

-    function updateOcclusionsField(): void {
+    function saveOcclusions(): void {
        if (isImageOcclusion && globalThis.canvas) {
            const occlusionsData = exportShapesToClozeDeletions($hideAllGuessOne);
            fieldStores[ioFields.occlusions].set(occlusionsData.clozes);
@ -572,7 +572,7 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
            triggerChanges,
            setIsImageOcclusion,
            setupMaskEditor,
-            updateOcclusionsField,
+            saveOcclusions,
            ...oldEditorAdapter,
        });

@ -637,7 +637,7 @@ the AddCards dialog) should be implemented in the user of this component.
        <div style="display: {$ioMaskEditorVisible ? 'block' : 'none'};">
            <ImageOcclusionPage
                mode={imageOcclusionMode}
-                on:change={updateOcclusionsField}
+                on:save={saveOcclusions}
                on:image-loaded={onImageLoaded}
            />
        </div>
--- a/ts/routes/image-occlusion/ImageOcclusionPage.svelte
+++ b/ts/routes/image-occlusion/ImageOcclusionPage.svelte
@ -40,7 +40,7 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
    </div>

    <div hidden={activeTabValue != 1}>
-        <MasksEditor {mode} on:change on:image-loaded />
+        <MasksEditor {mode} on:save on:image-loaded />
    </div>

    <div hidden={activeTabValue != 2}>
--- a/ts/routes/image-occlusion/MaskEditor.svelte
+++ b/ts/routes/image-occlusion/MaskEditor.svelte
@ -2,15 +2,6 @@
 Copyright: Ankitects Pty Ltd and contributors
 License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 -->
-<script context="module" lang="ts">
-    import { writable } from "svelte/store";
-
-    const changeSignal = writable(Symbol());
-
-    export function emitChangeSignal() {
-        changeSignal.set(Symbol());
-    }
-</script>

 <script lang="ts">
    import type { fabric } from "fabric";
@ -25,6 +16,7 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
    import Toolbar from "./Toolbar.svelte";
    import { MaskEditorAPI } from "./tools/api";
    import { onResize } from "./tools/tool-zoom";
+    import { saveNeededStore } from "./store";

    export let mode: IOMode;
    const iconSize = 80;
@ -38,27 +30,29 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html

    const dispatch = createEventDispatcher();

-    function onChange() {
-        dispatch("change", { canvas });
-    }
-
    function onImageLoaded({ path, noteId }: ImageLoadedEvent) {
        dispatch("image-loaded", { path, noteId });
    }

-    $: $changeSignal, onChange();
+    const unsubscribe = saveNeededStore.subscribe((saveNeeded: boolean) => {
+        if (saveNeeded === false) {
+            return;
+        }
+        dispatch("save");
+        saveNeededStore.set(false);
+    });

    function init(_node: HTMLDivElement) {
        if (mode.kind == "add") {
-            setupMaskEditor(mode.imagePath, onChange, onImageLoaded).then((canvas1) => {
+            // Editing occlusions on a new note through the "Add" window
+            setupMaskEditor(mode.imagePath, onImageLoaded).then((canvas1) => {
                canvas = canvas1;
            });
        } else {
-            setupMaskEditorForEdit(mode.noteId, onChange, onImageLoaded).then(
-                (canvas1) => {
-                    canvas = canvas1;
-                },
-            );
+            // Editing occlusions on an existing note through the "Browser" window
+            setupMaskEditorForEdit(mode.noteId, onImageLoaded).then((canvas1) => {
+                canvas = canvas1;
+            });
        }
    }

@ -68,10 +62,14 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html

    onDestroy(() => {
        window.removeEventListener("resize", resizeEvent);
+        unsubscribe();
    });

    const resizeEvent = () => {
-        onResize(canvas!);
+        if (canvas === null) {
+            return;
+        }
+        onResize(canvas);
    };
 </script>

--- a/ts/routes/image-occlusion/Toolbar.svelte
+++ b/ts/routes/image-occlusion/Toolbar.svelte
@ -25,8 +25,13 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
    import Shortcut from "$lib/components/Shortcut.svelte";
    import WithFloating from "$lib/components/WithFloating.svelte";

-    import { emitChangeSignal } from "./MaskEditor.svelte";
-    import { hideAllGuessOne, ioMaskEditorVisible, textEditingState } from "./store";
+    import {
+        hideAllGuessOne,
+        ioMaskEditorVisible,
+        textEditingState,
+        saveNeededStore,
+        opacityStateStore,
+    } from "./store";
    import { drawEllipse, drawPolygon, drawRectangle, drawText } from "./tools/index";
    import { makeMaskTransparent } from "./tools/lib";
    import { enableSelectable, stopDraw } from "./tools/lib";
@ -55,7 +60,7 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
    export let activeTool = "cursor";
    let showAlignTools = false;
    let leftPos = 82;
-    let maksOpacity = false;
+    let maskOpacity = false;
    let showFloating = false;
    const direction = getContext<Readable<"ltr" | "rtl">>(directionKey);
    // handle zoom event when mouse scroll and ctrl key are hold for panzoom
@ -158,13 +163,13 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
        }
    }

-    const handleToolChanges = (activeTool: string) => {
+    const handleToolChanges = (newActiveTool: string) => {
        disableFunctions();
        enableSelectable(canvas, true);
        // remove unfinished polygon when switching to other tools
        removeUnfinishedPolygon(canvas);

-        switch (activeTool) {
+        switch (newActiveTool) {
            case "cursor":
                drawCursor(canvas);
                break;
@ -178,9 +183,10 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
                drawPolygon(canvas);
                break;
            case "draw-text":
-                drawText(canvas);
-                break;
-            default:
+                drawText(canvas, () => {
+                    activeTool = "cursor";
+                    handleToolChanges(activeTool);
+                });
                break;
        }
    };
@ -198,10 +204,11 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html

    function changeOcclusionType(occlusionType: "all" | "one"): void {
        $hideAllGuessOne = occlusionType === "all";
-        emitChangeSignal();
+        saveNeededStore.set(true);
    }

    onMount(() => {
+        opacityStateStore.set(maskOpacity);
        removeHandlers = singleCallback(
            on(document, "click", onClick),
            on(window, "mousemove", onMousemove),
@ -336,8 +343,8 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
                    toggleTranslucentKeyCombination,
                )})"
                on:click={() => {
-                    maksOpacity = !maksOpacity;
-                    makeMaskTransparent(canvas, maksOpacity);
+                    maskOpacity = !maskOpacity;
+                    makeMaskTransparent(canvas, maskOpacity);
                }}
            >
                <Icon icon={mdiEye} />
@ -346,8 +353,8 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
                <Shortcut
                    keyCombination={toggleTranslucentKeyCombination}
                    on:action={() => {
-                        maksOpacity = !maksOpacity;
-                        makeMaskTransparent(canvas, maksOpacity);
+                        maskOpacity = !maskOpacity;
+                        makeMaskTransparent(canvas, maskOpacity);
                    }}
                />
            {/if}
@ -372,7 +379,7 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
                        keyCombination={tool.shortcut}
                        on:action={() => {
                            tool.action(canvas);
-                            emitChangeSignal();
+                            saveNeededStore.set(true);
                        }}
                    />
                {/if}
@ -400,7 +407,7 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
                        keyCombination={tool.shortcut}
                        on:action={() => {
                            tool.action(canvas);
-                            emitChangeSignal();
+                            saveNeededStore.set(true);
                        }}
                    />
                {/if}
--- a/ts/routes/image-occlusion/mask-editor.ts
+++ b/ts/routes/image-occlusion/mask-editor.ts
@ -8,7 +8,7 @@ import { fabric } from "fabric";
 import { get } from "svelte/store";

 import { optimumCssSizeForCanvas } from "./canvas-scale";
-import { notesDataStore, tagsWritable } from "./store";
+import { notesDataStore, saveNeededStore, tagsWritable, textEditingState } from "./store";
 import Toast from "./Toast.svelte";
 import { addShapesToCanvasFromCloze } from "./tools/add-from-cloze";
 import { enableSelectable, makeShapesRemainInCanvas, moveShapeToCanvasBoundaries } from "./tools/lib";
@ -24,11 +24,10 @@ export interface ImageLoadedEvent {

 export const setupMaskEditor = async (
    path: string,
-    onChange: () => void,
    onImageLoaded: (event: ImageLoadedEvent) => void,
 ): Promise<fabric.Canvas> => {
    const imageData = await getImageForOcclusion({ path });
-    const canvas = initCanvas(onChange);
+    const canvas = initCanvas();

    // get image width and height
    const image = document.getElementById("image") as HTMLImageElement;
@ -46,7 +45,6 @@ export const setupMaskEditor = async (

 export const setupMaskEditorForEdit = async (
    noteId: number,
-    onChange: () => void,
    onImageLoaded: (event: ImageLoadedEvent) => void,
 ): Promise<fabric.Canvas> => {
    const clozeNoteResponse = await getImageOcclusionNote({ noteId: BigInt(noteId) });
@ -63,14 +61,17 @@ export const setupMaskEditorForEdit = async (
    }

    const clozeNote = clozeNoteResponse.value.value;
-    const canvas = initCanvas(onChange);
+    const canvas = initCanvas();

    // get image width and height
    const image = document.getElementById("image") as HTMLImageElement;
    image.src = getImageData(clozeNote.imageData!, clozeNote.imageFileName!);

    image.onload = async function() {
-        const size = optimumCssSizeForCanvas({ width: image.width, height: image.height }, containerSize());
+        const size = optimumCssSizeForCanvas(
+            { width: image.naturalWidth, height: image.naturalHeight },
+            containerSize(),
+        );
        setCanvasSize(canvas);
        const boundingBox = setupBoundingBox(canvas, size);
        addShapesToCanvasFromCloze(canvas, boundingBox, clozeNote.occlusions);
@ -85,7 +86,7 @@ export const setupMaskEditorForEdit = async (
    return canvas;
 };

-function initCanvas(onChange: () => void): fabric.Canvas {
+function initCanvas(): fabric.Canvas {
    const canvas = new fabric.Canvas("canvas");
    tagsWritable.set([]);
    globalThis.canvas = canvas;
@ -110,9 +111,18 @@ function initCanvas(onChange: () => void): fabric.Canvas {
            modifiedPolygon(canvas, evt.target);
            undoStack.onObjectModified();
        }
-        onChange();
+        saveNeededStore.set(true);
+    });
+    canvas.on("text:editing:entered", function() {
+        textEditingState.set(true);
+    });
+
+    canvas.on("text:editing:exited", function() {
+        textEditingState.set(false);
+    });
+    canvas.on("object:removed", () => {
+        saveNeededStore.set(true);
    });
-    canvas.on("object:removed", onChange);
    return canvas;
 }

--- a/ts/routes/image-occlusion/shapes/base.ts
+++ b/ts/routes/image-occlusion/shapes/base.ts
@ -18,7 +18,7 @@ export type ShapeOrShapes = Shape | Shape[];
 export class Shape {
    left: number;
    top: number;
-    fill: string = SHAPE_MASK_COLOR;
+    fill: string;
    /** Whether occlusions from other cloze numbers should be shown on the
     * question side. Used only in reviewer code.
     */
--- a/ts/routes/image-occlusion/shapes/text.ts
+++ b/ts/routes/image-occlusion/shapes/text.ts
@ -3,7 +3,7 @@

 import { fabric } from "fabric";

-import { TEXT_BACKGROUND_COLOR, TEXT_FONT_FAMILY, TEXT_FONT_SIZE, TEXT_PADDING } from "../tools/lib";
+import { TEXT_BACKGROUND_COLOR, TEXT_COLOR, TEXT_FONT_FAMILY, TEXT_FONT_SIZE, TEXT_PADDING } from "../tools/lib";
 import type { ConstructorParams, Size } from "../types";
 import type { ShapeDataForCloze } from "./base";
 import { Shape } from "./base";
@ -23,6 +23,7 @@ export class Text extends Shape {
        ...rest
    }: ConstructorParams<Text> = {}) {
        super(rest);
+        this.fill = TEXT_COLOR;
        this.text = text;
        this.scaleX = scaleX;
        this.scaleY = scaleY;
--- a/ts/routes/image-occlusion/shapes/to-cloze.ts
+++ b/ts/routes/image-occlusion/shapes/to-cloze.ts
@ -4,7 +4,7 @@
 import { fabric } from "fabric";
 import { cloneDeep } from "lodash-es";

-import { getBoundingBox } from "../tools/lib";
+import { getBoundingBoxSize } from "../tools/lib";
 import type { Size } from "../types";
 import type { Shape, ShapeOrShapes } from "./base";
 import { Ellipse } from "./ellipse";
@ -97,7 +97,7 @@ export function baseShapesFromFabric(): ShapeOrShapes[] {
        ? activeObject
        : null;
    const objects = canvas.getObjects() as fabric.Object[];
-    const boundingBox = getBoundingBox();
+    const boundingBox = getBoundingBoxSize();
    // filter transparent rectangles
    return objects
        .map((object) => {
@ -170,10 +170,6 @@ function fabricObjectToBaseShapeOrShapes(
        shape.top = newPosition.y;
    }

-    if (size == undefined) {
-        size = { width: 0, height: 0 };
-    }
-
    shape = shape.toNormal(size);
    return shape;
 }
--- a/ts/routes/image-occlusion/store.ts
+++ b/ts/routes/image-occlusion/store.ts
@ -17,3 +17,5 @@ export const ioImageLoadedStore = writable(false);
 export const opacityStateStore = writable(false);
 // store state of text editing
 export const textEditingState = writable(false);
+// Stores if the canvas shapes data needs to be saved
+export const saveNeededStore = writable(false);
--- a/ts/routes/image-occlusion/tools/from-shapes.ts
+++ b/ts/routes/image-occlusion/tools/from-shapes.ts
@ -12,9 +12,13 @@ export const addShape = (
    shape: Shape,
 ): void => {
    const fabricShape = shape.toFabric(boundingBox.getBoundingRect(true));
-    addBorder(fabricShape);
    if (fabricShape.type === "i-text") {
        enableUniformScaling(canvas, fabricShape);
+    } else {
+        // No border around i-text shapes since it will be interpretted
+        // as character stroke, this is supposed to create an outline
+        // around the entire shape.
+        addBorder(fabricShape);
    }
    canvas.add(fabricShape);
 };
--- a/ts/routes/image-occlusion/tools/lib.ts
+++ b/ts/routes/image-occlusion/tools/lib.ts
@ -5,6 +5,7 @@ import { fabric } from "fabric";
 import { get } from "svelte/store";

 import { opacityStateStore } from "../store";
+import type { Size } from "../types";

 export const SHAPE_MASK_COLOR = "#ffeba2";
 export const BORDER_COLOR = "#212121";
@ -12,6 +13,7 @@ export const TEXT_BACKGROUND_COLOR = "#ffffff";
 export const TEXT_FONT_FAMILY = "Arial";
 export const TEXT_PADDING = 5;
 export const TEXT_FONT_SIZE = 40;
+export const TEXT_COLOR = "#000000";

 let _clipboard;

@ -310,20 +312,31 @@ export const selectAllShapes = (canvas: fabric.Canvas) => {

 export const isPointerInBoundingBox = (pointer): boolean => {
    const boundingBox = getBoundingBox();
+    if (boundingBox === undefined) {
+        return false;
+    }
    boundingBox.selectable = false;
    boundingBox.evented = false;
    if (
-        pointer.x < boundingBox.left
-        || pointer.x > boundingBox.left + boundingBox.width
-        || pointer.y < boundingBox.top
-        || pointer.y > boundingBox.top + boundingBox.height
+        pointer.x < boundingBox.left!
+        || pointer.x > boundingBox.left! + boundingBox.width!
+        || pointer.y < boundingBox.top!
+        || pointer.y > boundingBox.top! + boundingBox.height!
    ) {
        return false;
    }
    return true;
 };

-export const getBoundingBox = () => {
-    const canvas = globalThis.canvas;
+export const getBoundingBox = (): fabric.Rect | undefined => {
+    const canvas: fabric.Canvas = globalThis.canvas;
    return canvas.getObjects().find((obj) => obj.fill === "transparent");
 };
+
+export const getBoundingBoxSize = (): Size => {
+    const boundingBoxSize = getBoundingBox()?.getBoundingRect(true);
+    if (boundingBoxSize) {
+        return { width: boundingBoxSize.width, height: boundingBoxSize.height };
+    }
+    return { width: 0, height: 0 };
+};
--- a/ts/routes/image-occlusion/tools/tool-text.ts
+++ b/ts/routes/image-occlusion/tools/tool-text.ts
@ -4,7 +4,8 @@
 import { fabric } from "fabric";
 import { get } from "svelte/store";

-import { opacityStateStore, textEditingState } from "../store";
+import type { Callback } from "@tslib/helpers";
+import { opacityStateStore } from "../store";
 import {
    enableUniformScaling,
    isPointerInBoundingBox,
@ -16,11 +17,11 @@ import {
 import { undoStack } from "./tool-undo-redo";
 import { onPinchZoom } from "./tool-zoom";

-export const drawText = (canvas: fabric.Canvas): void => {
+export const drawText = (canvas: fabric.Canvas, onActivated: Callback): void => {
    canvas.selectionColor = "rgba(0, 0, 0, 0)";
    stopDraw(canvas);

-    let text;
+    let text: fabric.IText;

    canvas.on("mouse:down", function(o) {
        if (o.target) {
@ -52,7 +53,9 @@ export const drawText = (canvas: fabric.Canvas): void => {
        canvas.add(text);
        canvas.setActiveObject(text);
        undoStack.onObjectAdded(text.id);
+        text.enterEditing();
        text.selectAll();
+        onActivated();
    });

    canvas.on("mouse:move", function(o) {
@ -62,12 +65,4 @@ export const drawText = (canvas: fabric.Canvas): void => {
            return;
        }
    });
-
-    canvas.on("text:editing:entered", function() {
-        textEditingState.set(true);
-    });
-
-    canvas.on("text:editing:exited", function() {
-        textEditingState.set(false);
-    });
 };
--- a/ts/routes/image-occlusion/tools/tool-undo-redo.ts
+++ b/ts/routes/image-occlusion/tools/tool-undo-redo.ts
@ -7,7 +7,7 @@ import { writable } from "svelte/store";

 import { mdiRedo, mdiUndo } from "$lib/components/icons";

-import { emitChangeSignal } from "../MaskEditor.svelte";
+import { saveNeededStore } from "../store";
 import { redoKeyCombination, undoKeyCombination } from "./shortcuts";

 /**
@ -84,7 +84,7 @@ class UndoStack {
        this.locked = true;
        this.canvas?.loadFromJSON(this.stack[this.index], () => {
            this.canvas?.renderAll();
-            emitChangeSignal();
+            saveNeededStore.set(true);
            this.locked = false;
        });
        // make bounding box unselectable
@ -100,12 +100,12 @@ class UndoStack {
            this.push();
        }
        this.shapeIds.add(id);
-        emitChangeSignal();
+        saveNeededStore.set(true);
    }

    onObjectModified(): void {
        this.push();
-        emitChangeSignal();
+        saveNeededStore.set(true);
    }

    private maybePush(obj: fabric.IEvent<MouseEvent>): void {
--- a/ts/routes/image-occlusion/tools/tool-zoom.ts
+++ b/ts/routes/image-occlusion/tools/tool-zoom.ts
@ -9,7 +9,8 @@ import Hammer from "hammerjs";

 import { isDesktop } from "$lib/tslib/platform";

-import { getBoundingBox, redraw } from "./lib";
+import type { Size } from "../types";
+import { getBoundingBoxSize, redraw } from "./lib";

 const minScale = 0.5;
 const maxScale = 5;
@ -192,7 +193,7 @@ const onMouseUp = () => {
 };

 export const constrainBoundsAroundBgImage = (canvas: fabric.Canvas) => {
-    const boundingBox = getBoundingBox();
+    const boundingBox = getBoundingBoxSize();
    const ioImage = document.getElementById("image") as HTMLImageElement;

    const width = boundingBox.width * canvas.getZoom();
@ -217,7 +218,7 @@ export const setCanvasSize = (canvas: fabric.Canvas) => {
 };

 const fitCanvasVptScale = (canvas: fabric.Canvas) => {
-    const boundingBox = getBoundingBox();
+    const boundingBox = getBoundingBoxSize();
    const ratio = getScaleRatio(boundingBox);
    const vpt = canvas.viewportTransform!;

@ -237,7 +238,7 @@ const fitCanvasVptScale = (canvas: fabric.Canvas) => {
    redraw(canvas);
 };

-const getScaleRatio = (boundingBox: fabric.Rect) => {
+const getScaleRatio = (boundingBox: Size) => {
    const h1 = boundingBox.height!;
    const w1 = boundingBox.width!;
    const w2 = innerWidth - 42;