Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed annotation getting rotated when moving between pages #18272

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions src/display/editor/annotation_editor_layer.js
Original file line number Diff line number Diff line change
Expand Up @@ -488,13 +488,35 @@ class AnnotationEditorLayer {

this.attach(editor);
editor.parent?.detach(editor);
this.resetRotation(editor);
editor.setParent(this);
if (editor.div && editor.isAttachedToDOM) {
editor.div.remove();
this.div.append(editor.div);
}
}

/**
* When a page is rotated, the editor must be rotated as well
* to maintain the same orientation.
* @param {AnnotationEditor} editor
*/
resetRotation(editor) {
if (this.viewport.rotation === editor.parent.viewport.rotation) {
return;
}
const rotationOfThisPage = this.viewport.rotation;
const currentRotation =
360 - Number(this.div.getAttribute("data-main-rotation"));

const pageRotation =
(360 + rotationOfThisPage - editor._uiManager.viewParameters.rotation) %
360;
editor.pageRotation = pageRotation;
editor.rotation = rotationOfThisPage;
editor.div.setAttribute("data-editor-rotation", currentRotation);
}

/**
* Add a new editor in the current view.
* @param {AnnotationEditor} editor
Expand Down
78 changes: 74 additions & 4 deletions src/display/editor/editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,42 @@ class AnnotationEditor {
return false;
}

/**
* Rotate a point about 0.5 0.5 origin
* @param {number} x
* @param {number} y
* @param {number} angle
* @returns {any} The rotated point
*/
static rotatePointByMidPoint(x, y, angle) {
const originX = 0.5;
const originY = 0.5;
// Translate the point to the origin (originX, originY)
const translatedX = x - originX;
const translatedY = y - originY;
let rotatedX, rotatedY;

// Perform the rotation based on the given angle
switch (angle) {
case 90:
rotatedX = -translatedY;
rotatedY = translatedX;
break;
case 270:
rotatedX = translatedY;
rotatedY = -translatedX;
break;
default:
throw new Error("Invalid angle. Valid angles are 90 and 270 degrees.");
}

// Translate the point back
const finalX = rotatedX + originX;
const finalY = rotatedY + originY;

return { x: finalX, y: finalY };
}

/**
* Extract the data from the clipboard item and delegate the creation of the
* editor to the parent.
Expand Down Expand Up @@ -469,19 +505,53 @@ class AnnotationEditor {
const [parentWidth, parentHeight] = this.parentDimensions;
this.x += tx / parentWidth;
this.y += ty / parentHeight;

const oldRotation = this.rotation;

if (this.parent && (this.x < 0 || this.x > 1 || this.y < 0 || this.y > 1)) {
// It's possible to not have a parent: for example, when the user is
// dragging all the selected editors but this one on a page which has been
// destroyed.
// It's why we need to check for it. In such a situation, it isn't really
// a problem to not find a new parent: it's something which is related to
// what the user is seeing, hence it depends on how pages are layed out.

// The element will be outside of its parent so change the parent.
const { x, y } = this.div.getBoundingClientRect();
if (this.parent.findNewParent(this, x, y)) {
this.x -= Math.floor(this.x);
this.y -= Math.floor(this.y);
const newRotation = this.rotation;
if (oldRotation !== newRotation) {
const {
x: layerX,
y: layerY,
width,
height,
} = this.parent.div.getBoundingClientRect();
this.x = (x - layerX) / width;
this.y = (y - layerY) / height;

if (newRotation === 90) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can simplify the two cases 90 and 270 in using newRotation % 180 !== 0

const points = AnnotationEditor.rotatePointByMidPoint(
this.x,
this.y,
270
);
this.x = points.x;
this.y = points.y;
} else if (newRotation === 270) {
const points = AnnotationEditor.rotatePointByMidPoint(
this.x,
this.y,
90
);
this.x = points.x;
this.y = points.y;
} else if (newRotation === 180) {
this.x = 1 - this.x;
this.y = 1 - this.y;
}
} else {
this.x -= Math.floor(this.x);
this.y -= Math.floor(this.y);
}
}
}

Expand Down