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

Export and import annotations (annotations imported and placed on the wrong position) #1825

Closed
xchrx opened this issue Jul 31, 2023 · 11 comments
Assignees
Labels
bug Something isn't working Confirmed I've managed to reproduce the bug. That's great news: chances are I'll solve it. Original topic solved - ticket contains followup The original problem has been solved. However, as discussions go, a new issue has been raised. Solved Version 18 Scheduled for version 18

Comments

@xchrx
Copy link

xchrx commented Jul 31, 2023

First of all thank you for your work!

We have the case to store and restore the annotations on the pdf. if you try to restore the annotation, the anntation would created in the right size, but placed on the wrong position. It is offset by the size of the annotation.

Steps for recreating the bug:

  1. temporary save the current annotation information with the method getSerializedAnnoations();
  2. remove the set annotations with removeEditorAnnotations();
  3. restore the temporary saved annotations with addEditorAnnoation();

There you have the bug - placed on the wrong position

@xchrx xchrx changed the title Export and Import Annotations (Annotations imported on the wrong position) Export and import annotations (annotations imported and placed on the wrong position) Jul 31, 2023
@stephanrauh stephanrauh self-assigned this Jul 31, 2023
@stephanrauh stephanrauh added bug Something isn't working Confirmed I've managed to reproduce the bug. That's great news: chances are I'll solve it. labels Jul 31, 2023
@stephanrauh
Copy link
Owner

The offset is the same offset used by the copy-and-paste feature. That might be a clue.

@slimouGit
Copy link

slimouGit commented Aug 1, 2023

unfortunately I ve the same issue.
For instance with following code:

const serializedAnnotations = this.pdfService.getSerializedAnnotations();
this.pdfService.removeEditorAnnotations();
this.pdfService.addEditorAnnotation(serializedAnnotations[0]);

the inserted annotation wont be placed at the original position

@PatrickZimmerer
Copy link

I have the same issue, when copy-pasting it works fine, but where can I adjust the offset? The pasted annotation is always moved to the bottom right for the exact height & width of the annotation.

@stephanrauh
Copy link
Owner

By now, I'm pretty sure that the offset is caused by me being lazy. I used the "paste" part of the copy-and-paste implementation of the editors. Unfortunately, yesterdays debugging session didn't reveal where the offset is added. Maybe you can help me? During the next two or three weeks I'm short of time, so I'd appreciate your time.

Set [minifiedJSLibraries]="false", open pdf-xx.xxx.js in the debugger, and add a breakpoint here: https://github.com/stephanrauh/pdf.js/blob/6f55fbbdeba9d55227211c2799f1f770b95767a0/src/display/editor/tools.js#L835.

Thanks in advance,
Stephan

@stephanrauh
Copy link
Owner

Found it! I've managed to solve the issue, at least for drawings. It'll take a few days to fix it for texts and stamps, but after the first breakthrough it should be easy.

stephanrauh added a commit to stephanrauh/pdf.js that referenced this issue Aug 14, 2023
stephanrauh added a commit to stephanrauh/pdf.js that referenced this issue Aug 19, 2023
stephanrauh added a commit that referenced this issue Aug 19, 2023
@stephanrauh
Copy link
Owner

Your bugfix has landed with version 18.0.0-beta.2.

Please note that the internal coordinates of the serialized drawings have changed. This caught me by surprise, too, and it took a while until I figured it out. The bezier and path attributes now use absolute coordinates (i.e. relative to the page). Previously, they used coordinates relative to the origin of the drawing.

@stephanrauh stephanrauh added Solved Version 18 Scheduled for version 18 labels Aug 19, 2023
@stephanrauh stephanrauh reopened this Aug 20, 2023
stephanrauh added a commit to stephanrauh/pdf.js that referenced this issue Aug 20, 2023
stephanrauh added a commit to stephanrauh/pdf.js that referenced this issue Aug 20, 2023
@stephanrauh
Copy link
Owner

Oops. The version only fixed the ink editor. Version 18.0.0-beta.3 fixes the offset for all three editors.

The showcase often shows an offset of 2 units. I don't know where this comes from. But at least it's a predictable offset now, and chances are it's caused by the showcase and not by the library itself.

stephanrauh added a commit that referenced this issue Aug 20, 2023
stephanrauh added a commit to stephanrauh/extended-pdf-viewer-showcase that referenced this issue Aug 20, 2023
stephanrauh added a commit to stephanrauh/extended-pdf-viewer-showcase that referenced this issue Aug 20, 2023
@slimouGit
Copy link

slimouGit commented Aug 23, 2023

great job, thanks a lot. Annotations are placed at origin position now.
But: in case you draw multiple annotations, each on seperate page, all annotations are placed at this page its currently in view.
The padgeindex is known, but is not used for insertion

behavior is the same for both text annotations and draw annotations

@stephanrauh stephanrauh reopened this Aug 23, 2023
@stephanrauh
Copy link
Owner

What? Oh no! That's unexpected. As a short-term workaround, use [page] to navigate to the target page before adding an annotatation.

@stephanrauh stephanrauh added the Original topic solved - ticket contains followup The original problem has been solved. However, as discussions go, a new issue has been raised. label Aug 23, 2023
@slimouGit
Copy link

slimouGit commented Aug 24, 2023

yes indeed, I allready tryed this workaround, for instance like this:

annotations.forEach(a => {
      this.scroll(a.pageIndex+1,0)
      this.pdfService.addEditorAnnotation(a);
      });

it works fine so far

stephanrauh added a commit to stephanrauh/pdf.js that referenced this issue Aug 25, 2023
…vice.addEditorAnnotation()` respects the `pageIndex`if it's defined in the annotation: otherwise, the annotation is added to the current page
stephanrauh added a commit to stephanrauh/pdf.js that referenced this issue Aug 25, 2023
…vice.addEditorAnnotation()` respects the `pageIndex`if it's defined in the annotation: otherwise, the annotation is added to the current page
@stephanrauh
Copy link
Owner

I haven't published it yet, but version 18.0.0-beta.5 is going to fix that. Most likely, I'm going to publish the version this week-end, so I'm closing the ticket now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Confirmed I've managed to reproduce the bug. That's great news: chances are I'll solve it. Original topic solved - ticket contains followup The original problem has been solved. However, as discussions go, a new issue has been raised. Solved Version 18 Scheduled for version 18
Projects
None yet
Development

No branches or pull requests

4 participants