fix(annotation-editor): ricalcola marker/frecce/aree al resize del canvas (#28)
_fitCanvasToImage() prima scalava solo canvas e sfondo: gli oggetti già piazzati restavano alle vecchie coordinate pixel, e un salvataggio subito dopo scriveva il nuovo imageScale accanto a posizioni mai spostate - markers/frecce/aree finivano fuori posto anche nel viewer dell'operatore. Ora, quando la scala cambia (f = newScale/oldScale, conservata in this._currentScale), ogni oggetto viene ricalcolato coerentemente con getAnnotationsJson/loadAnnotationsJson (il riferimento): - left/top sempre *= f - area (fabric.Rect): width/height *= f, non scaleX/scaleY - è quello che getAnnotationsJson legge e loadAnnotationsJson ricostruisce - text (IText): fontSize *= f, stesso motivo - marker/arrow (Group): scaleX/scaleY *= f, il loro width/height non viene mai riletto al caricamento - arrow: arrowX1/Y1/X2/Y2 *= f, sono nello stesso spazio di left/top (li aggiorna anche object:modified con un delta) e finiscono nel JSON Aggiunta guardia beforeunload quando isDirty (annotazioni non salvate). Test in tests/js/test_annotation_editor_resize.js (node, senza Fabric.js reale: richiede jsdom non installato e il progetto non ha npm deps - canvas stand-in minimale che copre solo la superficie usata da _fitCanvasToImage). annotation-viewer.js non toccato. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -68,6 +68,12 @@ function annotationEditor() {
|
|||||||
_bgImageNatW: 0,
|
_bgImageNatW: 0,
|
||||||
_bgImageNatH: 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 */
|
/** Annotations JSON to load after background image is ready */
|
||||||
_pendingAnnotations: null,
|
_pendingAnnotations: null,
|
||||||
|
|
||||||
@@ -218,6 +224,16 @@ function annotationEditor() {
|
|||||||
this._debouncedResize = debounce(this.resizeCanvas.bind(this), 200);
|
this._debouncedResize = debounce(this.resizeCanvas.bind(this), 200);
|
||||||
window.addEventListener('resize', this._debouncedResize);
|
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 ----
|
// ---- Bridge: listen for commands from recipeEditor ----
|
||||||
|
|
||||||
this._onToolChange = function (e) {
|
this._onToolChange = function (e) {
|
||||||
@@ -471,6 +487,15 @@ function annotationEditor() {
|
|||||||
* so there is zero empty space and canvas coords map 1:1 to
|
* so there is zero empty space and canvas coords map 1:1 to
|
||||||
* scaled-image coords.
|
* 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)
|
* @param {fabric.Image} img - Fabric image object (background)
|
||||||
*/
|
*/
|
||||||
_fitCanvasToImage(img) {
|
_fitCanvasToImage(img) {
|
||||||
@@ -486,9 +511,12 @@ function annotationEditor() {
|
|||||||
if (!natW || !natH) return;
|
if (!natW || !natH) return;
|
||||||
|
|
||||||
// Scale to fit container width; never upscale
|
// Scale to fit container width; never upscale
|
||||||
var scale = Math.min(containerWidth / natW, 1);
|
var newScale = Math.min(containerWidth / natW, 1);
|
||||||
var canvasW = Math.round(natW * scale);
|
var oldScale = this._currentScale || newScale;
|
||||||
var canvasH = Math.round(natH * scale);
|
var f = newScale / oldScale;
|
||||||
|
|
||||||
|
var canvasW = Math.round(natW * newScale);
|
||||||
|
var canvasH = Math.round(natH * newScale);
|
||||||
|
|
||||||
this.canvas.setDimensions({ width: canvasW, height: canvasH });
|
this.canvas.setDimensions({ width: canvasW, height: canvasH });
|
||||||
|
|
||||||
@@ -496,15 +524,55 @@ function annotationEditor() {
|
|||||||
img,
|
img,
|
||||||
this.canvas.renderAll.bind(this.canvas),
|
this.canvas.renderAll.bind(this.canvas),
|
||||||
{
|
{
|
||||||
scaleX: scale,
|
scaleX: newScale,
|
||||||
scaleY: scale,
|
scaleY: newScale,
|
||||||
originX: 'left',
|
originX: 'left',
|
||||||
originY: 'top',
|
originY: 'top',
|
||||||
}
|
}
|
||||||
);
|
);
|
||||||
|
|
||||||
this.canvas.calcOffset();
|
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();
|
this.canvas.renderAll();
|
||||||
},
|
},
|
||||||
|
|
||||||
@@ -1269,6 +1337,9 @@ function annotationEditor() {
|
|||||||
if (this._debouncedResize) {
|
if (this._debouncedResize) {
|
||||||
window.removeEventListener('resize', this._debouncedResize);
|
window.removeEventListener('resize', this._debouncedResize);
|
||||||
}
|
}
|
||||||
|
if (this._onBeforeUnload) {
|
||||||
|
window.removeEventListener('beforeunload', this._onBeforeUnload);
|
||||||
|
}
|
||||||
if (this._onToolChange) {
|
if (this._onToolChange) {
|
||||||
window.removeEventListener('anno-tool-change', this._onToolChange);
|
window.removeEventListener('anno-tool-change', this._onToolChange);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -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');
|
||||||
@@ -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}"
|
||||||
|
)
|
||||||
Reference in New Issue
Block a user