[Editor] Add a basic stamp editor (bug 1790255)

[calixteman]

For now it allows to add a stamp annotation with an image selected from the file system.

[Snuffleupagus]

Another question that I had, without actually having tested this patch (assuming that it's rebased on top of PR #16588).
Given that the bitmaps will be transferred to the worker-thread e.g. during saving, how does that affect future editing?
Imagine the following sequence of events:

1. Add a new Stamp-annotation to the PDF document.
2. Save the PDF document (which will transfer the bitmap to the worker-thread).
3. Copy the existing Stamp-annotation and paste it in another page.
4. Save the PDF document again.

Assuming that I understand things correctly, which I'm probably not, won't this cause problems in step 3 (or possibly 4) now?
When copying we'll invoke StampEditor.serialize which in turns calls StampEditor.#serializeBitmap, and that method tries to draw a bitmap (to a canvas) that's previously been transferred to the worker-thread; does this actually work correctly?

[calixteman]

Another question that I had, without actually having tested this patch (assuming that it's rebased on top of PR #16588). Given that the bitmaps will be transferred to the worker-thread e.g. during saving, how does that affect future editing? Imagine the following sequence of events:

1. Add a new Stamp-annotation to the PDF document.

2. Save the PDF document (which will _transfer_ the `bitmap` to the worker-thread).

3. Copy the existing Stamp-annotation and paste it in another page.

4. Save the PDF document again.

Assuming that I understand things correctly, which I'm probably not, won't this cause problems in step 3 (or possibly 4) now? When copying we'll invoke StampEditor.serialize which in turns calls StampEditor.#serializeBitmap, and that method tries to draw a bitmap (to a canvas) that's previously been transferred to the worker-thread; does this actually work correctly?

The image we get from a file or an url, a.k.a the base image, is never exposed. Each time we need it somewhere else it has to be cloned and it's exactly what serializeBitmap is supposed to do.
In the specific case you described above, when we copy the annotation, the image is serialized into an URL in order to have it in the clipboard and when it has been saved, the base image is drawn onto a canvas and we use this canvas to generate a clone of the base image. Hence it should be safe (notice the "should").
That said, I think your comment is highlighting a potential issue with the annotation storage: once an image is transferred, then it's lost, so when we we re-use the storage we must be sure to serialize again the annotation containing the bitmap.
I noticed that we're using a structuredClone in the PrintAnnotationStorage and I was wondering if it could lead to uselessly clone ImageBitmap here.
Anyway, I just rebased the patch and slightly simplify the way to clone the bitmap in using a structuredClone.

[Snuffleupagus]

I noticed that we're using a structuredClone in the PrintAnnotationStorage and I was wondering if it could lead to uselessly clone ImageBitmap here.

Unfortunately I believe that it will lead to a copy of the bitmap here, however we can probably re-factor things a bit to avoid that; I'll try to look into that.

[Snuffleupagus]

Having tested this a little bit everything seems to work well, however there's one feature that I feel is "missing" here:
When inserting a image it's probably going to be very common for the user to need to resize it to make it fit a particular space, and most likely you'll want to maintain the "proper" aspect ratio of the image which currently is very difficult.
Hence, when resizing the image, could we maintain the aspect ratio by default and e.g. add a toggle (in a stampAnnotation-toolbar) to disable that behaviour?

[marco-c]

We could do like most other editing software: the corner ones resize while maintaining aspect ratio, the other four resize without maintaining aspect ratio. This would be simpler than adding other things in the UI.

[calixteman]

We could do like most other editing software: the corner ones resize while maintaining aspect ratio, the other four resize without maintaining aspect ratio. This would be simpler than adding other things in the UI.

Yes it'd be nice but as far as I know it isn't possible to achieve this with basic css/html (see resize on MDN).
So we'd probably need to implement that stuff by ourselves.

[Snuffleupagus]

We could do like most other editing software: the corner ones resize while maintaining aspect ratio, the other four resize without maintaining aspect ratio. This would be simpler than adding other things in the UI.

Yes it'd be nice but as far as I know it isn't possible to achieve this with basic css/html (see resize on MDN). So we'd probably need to implement that stuff by ourselves.

In that case, how about we simply maintain the original aspect ratio when resizing stamp-images[1] and worry about "arbitrary" resizing later (in a follow-up)?

---

[1] Looking at the Ink-editor, it only allows resizing with maintained aspect ratio.

[marco-c]

I agree with that idea, seems more useful than arbitrary resizing. We can then ask UX for their opinion.

[Snuffleupagus]

I've not really had time to look at the src/display/editor/stamp.js file yet, but will try to do that tomorrow.

[Snuffleupagus]

r=me, with the final comments addressed and passing tests; thank you!

[calixteman]

/botio integrationtest

[moz-tools-bot]

From: Bot.io (Linux m4)

---

Received

Command cmd_integrationtest from @calixteman received. Current queue size: 1

Live output at: http://54.241.84.105:8877/fbc57048aeec52b/output.txt

[moz-tools-bot]

From: Bot.io (Windows)

---

Received

Command cmd_integrationtest from @calixteman received. Current queue size: 1

Live output at: http://54.193.163.58:8877/79f1db19ae20342/output.txt

[moz-tools-bot]

From: Bot.io (Linux m4)

---

Failed

Full output at http://54.241.84.105:8877/fbc57048aeec52b/output.txt

Total script time: 0.18 mins

[calixteman]

/botio integrationtest

[moz-tools-bot]

From: Bot.io (Linux m4)

---

Received

Command cmd_integrationtest from @calixteman received. Current queue size: 0

Live output at: http://54.241.84.105:8877/1f42814096d2714/output.txt

[moz-tools-bot]

From: Bot.io (Windows)

---

Received

Command cmd_integrationtest from @calixteman received. Current queue size: 2

Live output at: http://54.193.163.58:8877/ba27d748066db2c/output.txt

[moz-tools-bot]

From: Bot.io (Windows)

---

Failed

Full output at http://54.193.163.58:8877/79f1db19ae20342/output.txt

Total script time: 0.48 mins

[moz-tools-bot]

From: Bot.io (Linux m4)

---

Failed

Full output at http://54.241.84.105:8877/1f42814096d2714/output.txt

Total script time: 4.40 mins

• Integration Tests: FAILED

[moz-tools-bot]

From: Bot.io (Windows)

---

Failed

Full output at http://54.193.163.58:8877/ba27d748066db2c/output.txt

Total script time: 18.25 mins

• Integration Tests: FAILED