-
Notifications
You must be signed in to change notification settings - Fork 10.1k
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
[Editor] (WIP) Add a new tool in order to add an handwritten signature to a pdf (bug 1942343) #19339
base: master
Are you sure you want to change the base?
Conversation
/botio-linux preview |
272b79e
to
d4acae6
Compare
/botio-linux preview |
From: Bot.io (Linux m4)ReceivedCommand cmd_preview from @calixteman received. Current queue size: 0 Live output at: http://54.241.84.105:8877/cfe6a210281f1ba/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.241.84.105:8877/cfe6a210281f1ba/output.txt Total script time: 0.86 mins Published |
d4acae6
to
e51f339
Compare
e51f339
to
b770e3c
Compare
…e to a pdf (bug 1942343) This patch is adding some code in order to extract a drawing as curves from an image. The algorithm is basically the following: - reduce the dimensions - make it gray - apply a bilateral filter in order to add some blurryness while keeping the edges - compute the histogram - guess what's the background color which should contain a large majority of the pixels - make a binary image - extract the contours in using the Suzuki algorithm - apply the Douglas-Peucker algorithm in order to reduce the number of points The algorithm is improvable but it should work pretty well if there's a clear difference between the background and the drawing. In a v2 we could use a ML model in order to improve the extraction. There's few changes related to the UI in order to make the tool usable, but they're very basic for the moment.
b770e3c
to
fdc8e45
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've not reviewed signaturedraw.js
in any detail, but overall the patch seems reasonable.
/* | ||
3 2 1 | ||
4 X 0 | ||
5 6 7 | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please add a little bit more context (e.g. one sentence) to this comment?
return -1; | ||
} | ||
|
||
static #counterclockwiseNonZero(buf, width, i0, j0, i, j, offset) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit:
static #counterclockwiseNonZero(buf, width, i0, j0, i, j, offset) { | |
static #counterClockwiseNonZero(buf, width, i0, j0, i, j, offset) { |
const invS = 1 / dist; | ||
const phi = Math.atan(m); | ||
const cosPhi = Math.cos(phi); | ||
const sinPHi = Math.sin(phi); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit:
const sinPHi = Math.sin(phi); | |
const sinPhi = Math.sin(phi); |
if (FeatureTest.isLittleEndian) { | ||
for (let i = 0, ii = src.length; i < ii; i++) { | ||
dest[i] = (src[i] * 0x10101) | 0xff000000; | ||
} | ||
} else { | ||
for (let i = 0, ii = src.length; i < ii; i++) { | ||
dest[i] = (src[i] * 0x1010100) | 0x000000ff; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It appears that this re-implements the grayToRGBA
helper function, can we please use that one instead here?
pdf.js/src/shared/image_utils.js
Lines 123 to 133 in 3880071
function grayToRGBA(src, dest) { | |
if (FeatureTest.isLittleEndian) { | |
for (let i = 0, ii = src.length; i < ii; i++) { | |
dest[i] = (src[i] * 0x10101) | 0xff000000; | |
} | |
} else { | |
for (let i = 0, ii = src.length; i < ii; i++) { | |
dest[i] = (src[i] * 0x1010100) | 0x000000ff; | |
} | |
} | |
} |
// Also, we want to normalize the values between 0 and 255 in order to | ||
// increase the contrast. | ||
const N = buf.length; | ||
const out = new Uint8Array(N >> 2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we use an Uint8ClampedArray
instead here, rather than manually calling Math.round
at line 392 below?
@@ -39,6 +39,7 @@ class EditorUndoBar { | |||
freetext: "pdfjs-editor-undo-bar-message-freetext", | |||
stamp: "pdfjs-editor-undo-bar-message-stamp", | |||
ink: "pdfjs-editor-undo-bar-message-ink", | |||
signature: "pdfjs-editor-undo-bar-message-signature", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This l10n string is missing in the en-US
Fluent file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given that we have a specification now, could we add a "correct" image directly instead?
(Also, all the other icons are 16x16 pixels in size.)
@@ -317,6 +317,18 @@ | |||
</div> | |||
</div> | |||
</div> | |||
<div id="editorSignature" class="toolbarButtonWithContainer" hidden="true"> | |||
<button id="editorSignatureButton" class="toolbarButton" type="button" disabled="disabled" title="Add or edit images" role="radio" aria-expanded="false" aria-haspopup="true" aria-controls="editorSignatureParamsToolbar" tabindex="0"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The title="Add or edit images"
-part looks like too much copy-and-paste.
<button id="editorSignatureButton" class="toolbarButton" type="button" disabled="disabled" title="Add or edit images" role="radio" aria-expanded="false" aria-haspopup="true" aria-controls="editorSignatureParamsToolbar" tabindex="0"> | ||
<span>Add or edit signatures</span> | ||
</button> | ||
<div class="editorParamsToolbar hidden doorHangerRight menu" id="editorSignatureParamsToolbar"> | ||
<div class="menuContainer"> | ||
<button id="editorSignatureAddSignature" class="toolbarButton labeled" type="button" title="Add signature" tabindex="0"> | ||
<span class="editorParamsLabel">Add signature</span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
None of the new buttons/spans have l10n-attributes, nor entries in en-US
Fluent file.
async #extractSignature() { | ||
const input = document.createElement("input"); | ||
input.type = "file"; | ||
input.accept = StampEditor.supportedTypesStr; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is StampEditor
intended/correct here?
This patch is adding some code in order to extract a drawing as curves from an image. The algorithm is basically the following:
The algorithm is improvable but it should work pretty well if there's a clear difference between the background and the drawing.
In a v2 we could use a ML model in order to improve the extraction.
There's few changes related to the UI in order to make the tool usable, but they're very basic for the moment.