diff --git a/src/frontend/flask_app/static/js/annotation-editor.js b/src/frontend/flask_app/static/js/annotation-editor.js index 7bddbe5..3f489b7 100644 --- a/src/frontend/flask_app/static/js/annotation-editor.js +++ b/src/frontend/flask_app/static/js/annotation-editor.js @@ -68,6 +68,12 @@ function annotationEditor() { _bgImageNatW: 0, _bgImageNatH: 0, + /** Background image scale applied at the last _fitCanvasToImage() call + * (#28) - the ratio between this and the newly computed scale is how + * existing annotation objects get repositioned/resized on a resize, + * instead of being left at their old pixel coordinates. */ + _currentScale: 0, + /** Annotations JSON to load after background image is ready */ _pendingAnnotations: null, @@ -218,6 +224,16 @@ function annotationEditor() { this._debouncedResize = debounce(this.resizeCanvas.bind(this), 200); window.addEventListener('resize', this._debouncedResize); + // Unsaved annotations are easy to lose by accident (a stray back-nav, + // an accidental tab close) - isDirty already tracks exactly that (#28). + this._onBeforeUnload = function (e) { + if (!self.isDirty) return; + e.preventDefault(); + e.returnValue = ''; + return ''; + }; + window.addEventListener('beforeunload', this._onBeforeUnload); + // ---- Bridge: listen for commands from recipeEditor ---- this._onToolChange = function (e) { @@ -471,6 +487,15 @@ function annotationEditor() { * so there is zero empty space and canvas coords map 1:1 to * scaled-image coords. * + * Existing annotation objects are repositioned/resized by the same + * factor the background just changed by (#28): before this, only the + * canvas and the background image were rescaled on a resize + * (window resize, sidebar toggle, ...) - every marker/arrow/area stayed + * at its old pixel coordinates, drifting visually off the drawing, and + * a save right after wrote the *new* imageScale next to coordinates + * that had never actually moved to match it, corrupting the position + * for the operator's viewer too. + * * @param {fabric.Image} img - Fabric image object (background) */ _fitCanvasToImage(img) { @@ -486,9 +511,12 @@ function annotationEditor() { if (!natW || !natH) return; // Scale to fit container width; never upscale - var scale = Math.min(containerWidth / natW, 1); - var canvasW = Math.round(natW * scale); - var canvasH = Math.round(natH * scale); + var newScale = Math.min(containerWidth / natW, 1); + var oldScale = this._currentScale || newScale; + var f = newScale / oldScale; + + var canvasW = Math.round(natW * newScale); + var canvasH = Math.round(natH * newScale); this.canvas.setDimensions({ width: canvasW, height: canvasH }); @@ -496,15 +524,55 @@ function annotationEditor() { img, this.canvas.renderAll.bind(this.canvas), { - scaleX: scale, - scaleY: scale, + scaleX: newScale, + scaleY: newScale, originX: 'left', originY: 'top', } ); this.canvas.calcOffset(); - this.canvas.forEachObject(function (obj) { obj.setCoords(); }); + this.canvas.forEachObject(function (obj) { + if (f !== 1) { + obj.left *= f; + obj.top *= f; + + if (obj.objectType === 'area') { + // fabric.Rect: getAnnotationsJson serialises obj.width/height + // directly (not width * scaleX), and loadAnnotationsJson + // reconstructs the rect from that same width/height - so the + // size has to travel through width/height too, or a save right + // after this resize would silently drop it. + obj.width = (obj.width || 0) * f; + obj.height = (obj.height || 0) * f; + } else if (obj.objectType === 'text') { + // fabric.IText: same reasoning, fontSize is the serialised field. + var newFontSize = (obj.fontSize || 18) * f; + obj.set('fontSize', newFontSize); + obj.textFontSize = newFontSize; + } else { + // Groups (marker, arrow): their own width/height are never read + // back on load (addMarker/addArrow only use position and, for + // arrows, the endpoints below), so scaleX/scaleY - which drives + // 100% of their rendered size - is what has to change. + obj.scaleX = (obj.scaleX || 1) * f; + obj.scaleY = (obj.scaleY || 1) * f; + } + + if (obj.objectType === 'arrow') { + // Stored endpoints live in the same coordinate space as + // obj.left/top (see the object:modified handler, which adds a + // plain pixel delta to them) - so they scale by f exactly like + // left/top do, independently of the group's own scaleX/scaleY. + obj.arrowX1 = (obj.arrowX1 || 0) * f; + obj.arrowY1 = (obj.arrowY1 || 0) * f; + obj.arrowX2 = (obj.arrowX2 || 0) * f; + obj.arrowY2 = (obj.arrowY2 || 0) * f; + } + } + obj.setCoords(); + }); + this._currentScale = newScale; this.canvas.renderAll(); }, @@ -1269,6 +1337,9 @@ function annotationEditor() { if (this._debouncedResize) { window.removeEventListener('resize', this._debouncedResize); } + if (this._onBeforeUnload) { + window.removeEventListener('beforeunload', this._onBeforeUnload); + } if (this._onToolChange) { window.removeEventListener('anno-tool-change', this._onToolChange); } diff --git a/src/frontend/flask_app/tests/js/test_annotation_editor_resize.js b/src/frontend/flask_app/tests/js/test_annotation_editor_resize.js new file mode 100644 index 0000000..6fbd050 --- /dev/null +++ b/src/frontend/flask_app/tests/js/test_annotation_editor_resize.js @@ -0,0 +1,133 @@ +/** + * annotation-editor.js _fitCanvasToImage() resize behaviour, run with plain + * node - no real Fabric.js (fabric-5.3.1.min.js requires jsdom under node, + * which is not installed; this project has no package.json / npm deps at + * all, only vendored browser copies, so pulling one in is out of scope for + * this fix). Instead: a minimal stand-in for the one Fabric surface + * _fitCanvasToImage actually touches (canvas.setDimensions/ + * setBackgroundImage/calcOffset/renderAll/forEachObject) and plain objects + * shaped like what addMarker/addArrow/addRect/addText produce. + * + * annotationEditor() itself never references the `fabric` global outside + * init()/addMarker()/addArrow()/addRect()/addText(), so calling + * _fitCanvasToImage() directly, without ever calling init(), needs no + * fabric at all. + * + * Covers gitea:Adriano/TieMeasureFlow#28. + * Run from src/frontend/flask_app: node tests/js/test_annotation_editor_resize.js + */ +'use strict'; +const fs = require('fs'); +const path = require('path'); + +const src = fs.readFileSync( + path.join(__dirname, '..', '..', 'static', 'js', 'annotation-editor.js'), 'utf8', +); +eval(src + ';global.annotationEditor = annotationEditor;'); + +let failures = 0; +function check(label, cond, extra) { + if (cond) { + console.log('ok - ' + label); + } else { + failures++; + console.error('NOT OK - ' + label + (extra ? ' (' + extra + ')' : '')); + } +} + +function mkEditor(containerWidth) { + const e = annotationEditor(); + e.canvasEl = { parentElement: { parentElement: { clientWidth: containerWidth } } }; + const objects = []; + e.canvas = { + setDimensions() {}, + setBackgroundImage(img, cb) { if (cb) cb(); }, + calcOffset() {}, + renderAll() {}, + forEachObject(fn) { objects.forEach(fn); }, + getObjects() { return objects; }, + }; + e._objects = objects; + return e; +} + +function mkObj(props) { + return Object.assign( + { scaleX: 1, scaleY: 1, setCoords() {}, set(k, v) { this[k] = v; } }, + props, + ); +} + +// Natural image 1000x600. +{ + const e = mkEditor(500); // container 500 -> scale 0.5 + const marker = mkObj({ objectType: 'marker', left: 100, top: 60 }); + const arrow = mkObj({ + objectType: 'arrow', left: 40, top: 20, + arrowX1: 40, arrowY1: 20, arrowX2: 140, arrowY2: 70, + }); + const area = mkObj({ objectType: 'area', left: 10, top: 10, width: 80, height: 40 }); + const text = mkObj({ objectType: 'text', left: 5, top: 5, fontSize: 20, textFontSize: 20 }); + e._objects.push(marker, arrow, area, text); + + // First fit: no prior scale recorded, so nothing should move (f == 1). + e._fitCanvasToImage({ width: 1000, height: 600 }); + check('#28 prima chiamata: _currentScale = 0.5', e._currentScale === 0.5, e._currentScale); + check( + '#28 prima chiamata: marker invariato (niente scala precedente, f=1)', + marker.left === 100 && marker.top === 60, + JSON.stringify(marker), + ); + + // Container shrinks to 250 -> new scale 0.25 -> f = 0.25/0.5 = 0.5. + e.canvasEl.parentElement.parentElement.clientWidth = 250; + e._fitCanvasToImage({ width: 1000, height: 600 }); + + check('#28 _currentScale aggiornata a 0.25', e._currentScale === 0.25, e._currentScale); + + check('#28 marker.left/top dimezzati', marker.left === 50 && marker.top === 30, JSON.stringify(marker)); + check('#28 marker.scaleX/scaleY dimezzati', marker.scaleX === 0.5 && marker.scaleY === 0.5, JSON.stringify(marker)); + + check('#28 arrow.left/top dimezzati', arrow.left === 20 && arrow.top === 10, JSON.stringify(arrow)); + check('#28 arrow.scaleX/scaleY dimezzati (gruppo)', arrow.scaleX === 0.5 && arrow.scaleY === 0.5, JSON.stringify(arrow)); + check( + '#28 arrow endpoints (arrowX1..Y2) dimezzati come left/top', + arrow.arrowX1 === 20 && arrow.arrowY1 === 10 && arrow.arrowX2 === 70 && arrow.arrowY2 === 35, + JSON.stringify(arrow), + ); + + check('#28 area.width/height dimezzati', area.width === 40 && area.height === 20, JSON.stringify(area)); + check( + '#28 area.scaleX/scaleY invariati (getAnnotationsJson legge width/height, non la scala)', + area.scaleX === 1 && area.scaleY === 1, + JSON.stringify(area), + ); + + check( + '#28 text.fontSize/textFontSize dimezzati', + text.fontSize === 10 && text.textFontSize === 10, + JSON.stringify(text), + ); +} + +// A resize that leaves the effective scale unchanged (e.g. a taller +// container, same width) must be a no-op on every object. +{ + const e = mkEditor(500); + const marker = mkObj({ objectType: 'marker', left: 33, top: 77 }); + e._objects.push(marker); + + e._fitCanvasToImage({ width: 1000, height: 600 }); + e._fitCanvasToImage({ width: 1000, height: 600 }); // same scale again + check( + '#28 stessa scala -> nessuno spostamento', + marker.left === 33 && marker.top === 77, + JSON.stringify(marker), + ); +} + +if (failures > 0) { + console.error(failures + ' test falliti su annotation-editor.js'); + process.exit(1); +} +console.log('tutti i test annotation-editor.js (_fitCanvasToImage) superati'); diff --git a/src/frontend/flask_app/tests/test_annotation_editor_resize_js.py b/src/frontend/flask_app/tests/test_annotation_editor_resize_js.py new file mode 100644 index 0000000..633f2b6 --- /dev/null +++ b/src/frontend/flask_app/tests/test_annotation_editor_resize_js.py @@ -0,0 +1,40 @@ +"""Runs the plain-node annotation-editor.js resize test +(tests/js/test_annotation_editor_resize.js). + +Same thin pytest wrapper pattern as test_numpad_js.py/test_csv_export_js.py: +shells out to ``node`` and asserts the script's own exit code. Skips (does +not fail) when node is not installed. + +No real Fabric.js involved (see the .js file's own docstring for why) - this +exercises _fitCanvasToImage()'s resize/rescale arithmetic directly against a +minimal canvas stand-in. + +Covers gitea:Adriano/TieMeasureFlow#28. +""" +import shutil +import subprocess +from pathlib import Path + +import pytest + +FLASK_APP_DIR = Path(__file__).resolve().parent.parent +JS_TEST = Path(__file__).resolve().parent / "js" / "test_annotation_editor_resize.js" + + +def test_annotation_editor_resize_rescales_existing_objects(): + node = shutil.which("node") + if not node: + pytest.skip("node non installato: test JS di annotation-editor.js saltato") + + result = subprocess.run( + [node, str(JS_TEST)], + cwd=str(FLASK_APP_DIR), + capture_output=True, + text=True, + timeout=30, + ) + + assert result.returncode == 0, ( + f"tests/js/test_annotation_editor_resize.js fallito (exit {result.returncode})\n" + f"stdout:\n{result.stdout}\nstderr:\n{result.stderr}" + )