Fixed several form flattening issues #992

Lonzak · 2023-11-15T20:25:46Z

Fixed several document flattening issues for certain documents. As a result form fields were stretched or distorted.

the main changes can be found in the PdfStamper
Added testdocuments and junit

Additionally the following things were fixed:

minor jdoc improvements
new methods
fixed junit tests
refactored junits tests

- Main change is in PdfStamper - Added testdocuments and junit - some smaller refactoring, jdoc and fixing other junits

Lonzak · 2023-11-15T20:28:56Z

@mkl-public I would be interested in a code review. PdfStamper lines 943 - 1049

sonarqubecloud · 2023-11-15T20:45:40Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

andreasrosdal · 2023-11-16T07:37:47Z

Can you please share screenshots of PDF's before+after this change, demonstrating that the form flattening issue has been fixed?

Lonzak · 2023-11-17T08:04:09Z

Sure:

First file:

Second file:

Third file:

Lonzak · 2023-11-20T14:34:31Z

@mkl-public The merge was quite quick. I would feel better if you could have a look at the changes, since it is not my area of expertise...

andreasrosdal · 2023-11-24T11:26:50Z

Thank you for this contribution. More pull requests are welcome. This was merged in the philosophy of "Release early, release often", and can still be changed and improved if needed. I did look though the code and it looks good, however, if there are specific issues then these can be resolved.

The screenshots you made of this change indicate that this is a very good improvement, the signature text is much more readable with this change.

Lonzak · 2023-11-24T11:28:42Z

the signature text is much more readable with this change.

Yes this, but also it has the size it originally had (which can have legal implications...)

andreasrosdal · 2023-11-24T11:39:03Z

Do you see a need to make any changes to this PR that you submitted to OpenPDF? This looks like an overall good improvement.

http://www.catb.org/esr/writings/homesteading/cathedral-bazaar/ar01s04.html

Lonzak · 2023-11-24T12:56:47Z

No - since everything is working it is fine for me. But I am waiting for MKLs CodeReview - most times he has some improvements :-) But it is okay to improve this then. Fine with me...

mkl-public · 2023-11-28T13:47:57Z

I'll be taking a look.
At first glance it looks quite ok but form flattening can be tricky in its details.

mkl-public

All in all this is a clear improvement to the previous state of form field flattening but there are some details that might require looking into some more.

mkl-public · 2023-12-04T15:35:01Z

openpdf/src/main/java/com/lowagie/text/pdf/PdfStamperImp.java

+		        				}
+		                    }
+	                    }
+                	}


Here you try to find out whether creation of the appearance stream is necessary; you only want to call regenerateField if there was no appearance stream yet (except probably an empty one).
Strictly speaking, though, NeedAppearances set to true requires you to recreate all appearances.
For example some PDF processors set the form values but don't update the appearances. They then set NeedAppearances to delegate that task to the next processor, leaving the PDF in a state with appearances showing the incorrect former value.
(Of course one does not need to re-create appearances that have been created in the current PdfStamperImp, and trying to re-create signature appearances makes no sense, either.)

I was trying to be conservative/careful to not regenerate appearances unnecessarily... But you are right - all appearances need to be regenerated. Lets hope the PDF processor can do this without any errors :-) (And luckily this mechanism is deprecated in 2.0) So the following would suffice?

if (needAppearance!=null && needAppearance.booleanValue()) { boolean regenerate = false; int type = this.acroFields.getFieldType(name); if(type!=AcroFields.FIELD_TYPE_SIGNATURE) { if(appDic != null && appDic.getDirectObject(PdfName.N) instanceof PdfIndirectReference) { //since newly added regenerate=false; } else { regenerate=true; } } ....

Looks good. But please do test,

mkl-public · 2023-12-04T15:52:09Z

openpdf/src/main/java/com/lowagie/text/pdf/PdfStamperImp.java

+                boolean transformNeeded=false;
+                double rotation = 0;
+                if(merged.getAsDict(PdfName.MK) != null && merged.getAsDict(PdfName.MK).get(PdfName.R) != null){
+                    rotation = merged.getAsDict(PdfName.MK).getAsNumber(PdfName.R).floatValue();


You use this rotation value to determine how the existing appearance stream needs to be rotated and, consequentially, scaled when put into the field Rect. Strictly speaking, though, the MK dictionary only contains instructions for a PDF processor how to create the appearance for a field. For fitting the appearance into the field Rect, one should use the Matrix of the appearance as described in ISO 32000-2:2020 section 12.5.5.

Usually using the MK value will render the correct result because the annotation Matrix usually is built according to the MK/R value. But in some cases, in particular for signature appearances, PDF processors may have used a different actual rotation (visible in the appearance Matrix), assuming no-one would touch a signature appearance...

Ah ok interesting - so should I leave the rotation code in there in case there is no matrix defined? But if it is it will overwrite the value?

double rotation = 0; if(merged.getAsDict(PdfName.MK) != null && merged.getAsDict(PdfName.MK).get(PdfName.R) != null){ rotation = merged.getAsDict(PdfName.MK).getAsNumber(PdfName.R).floatValue(); } if (appDic != null && appDic.getAsArray(PdfName.MATRIX) != null) { PdfArray matrixArray = appDic.getAsArray(PdfName.MATRIX); if(matrixArray.size() == 6) { float a = matrixArray.getAsNumber(0).floatValue(); float b = matrixArray.getAsNumber(1).floatValue(); double rotationRadians = Math.atan2(b, a); //in degrees rotation = Math.toDegrees(rotationRadians); } }

Working with the matrix should be done strictly according to spec, i.e. using the algorithm in 12.5.5. If that looks too complicated, continue using the MK/R value

The spec says something like this:

Algorithm: appearance streams

The appearance’s bounding box (specified by its BBox entry) shall be transformed, using Matrix, to produce a quadrilateral with arbitrary orientation. The transformed appearance box is the smallest upright rectangle that encompasses this quadrilateral.

A matrix A shall be computed that scales and translates the transformed appearance box to align with the edges of the annotation’s rectangle (specified by the Rect entry). A maps the lower-left corner (the corner with the smallest x and y coordinates) and the upper-right corner (the corner with the greatest x and y coordinates) of the transformed appearance box to the corresponding corners of the annotation’s rectangle.

Matrix shall be concatenated with A to form a matrix AA that maps from the appearance’s coordinate system to the annotation’s rectangle in default user space:
AA = Matrix × 𝐴

Ok I think we'll stick with /R for now....

mkl-public · 2023-12-04T15:54:17Z

openpdf/src/main/java/com/lowagie/text/pdf/PdfStamperImp.java

+                        float scaleFactorHeight = Math.abs(bbox.height() != 0 ? rectHeight / bbox.height() : 1.0f);
+
+                        PdfArray array = new PdfArray(new float[]{scaleFactorWidth, 0, 0, scaleFactorHeight, 0, 0});
+                        appStream.put(PdfName.MATRIX, array);


Here you might overwrite a previously existing Matrix with its own ideas in respect to the transformation of the appearance, in particular to its rotation.

Of course the Matrix does not need to remain as it was (as the meaning of the Matrix in form Xobjects used as annotation appearances and in form Xobjects used content stream resources differs somewhat), but the original value should be taken into account and not be overwritten without consideration.

Like this?

if (scaleFactorWidth != 1 || scaleFactorHeight != 1) { PdfArray array = new PdfArray(new float[]{scaleFactorWidth, 0, 0, scaleFactorHeight, 0, 0}); appStream.put(PdfName.MATRIX, array); markUsed(appStream); }

Well, if you do as said above, i.e. implement the 12.5.5 algorithm, you would construct the new matrix with that algorithm, not like this here... ;)

So if we would stick with the above /R - then this part stays as well (like it is)?

Essentially yes.
Usually the rotation of the Matrix is constructed based on the MK/R value, but you apply the rotation separately later, so you usually are ok with overwriting the Matrix with a pure scaling one to make dimensions match.

mkl-public · 2023-12-04T15:59:18Z

openpdf/src/main/java/com/lowagie/text/pdf/PdfStamperImp.java

+                            double x = box.getLeft();
+                            double y = box.getBottom();
+
+                            if (rotation != 0 && rotation % 90 == 0 && rotation % 270 != 0) {


To test rotation % 270 != 0 makes no sense. I assume you mean something like rotation % 360 != 270.
And for the whole term you probably actually mean rotation % 360 == 90 || rotation % 360 == 180.

Right this makes it more clear:

rotation = rotation % 360; if (rotation == 90 || rotation == 270) { x += box.getWidth(); } if (rotation == 180 || rotation == 270) { y += box.getHeight(); }

Yes, much clearer.

mkl-public · 2023-12-04T16:03:19Z

openpdf/src/main/java/com/lowagie/text/pdf/PdfStamperImp.java

+                            if (rotation != 0 && rotation % 90 == 0 && rotation % 270 != 0) {
+                                x += box.getWidth();
+                            }
+                            if (rotation != 0 && (rotation % 180 == 0 || rotation % 270 == 0)) {


To test rotation % 270 == 0 makes no sense, it is true for 270°, 540° (equivalent to 180°), 810° (equivalent to 90°), 1080° (equivalent to 0°), ...
I assume you mean something like rotation % 360 == 180 || rotation % 360 == 270.

andreasrosdal · 2023-12-08T07:39:58Z

Please submit an update. @Lonzak I plan on making a new release of OpenPDF very soon.

Lonzak · 2023-12-08T08:55:52Z

I plan to do it today....

* Fixed issues discussed in #992 * Fix for Issue #1047

Summary: Fixed several form flattening issues.

f5ad660

- Main change is in PdfStamper - Added testdocuments and junit - some smaller refactoring, jdoc and fixing other junits

Lonzak added 2 commits November 15, 2023 21:37

Fixed checkstyle

79f2903

added test documents

d04d66e

andreasrosdal merged commit 47ea1ad into LibrePDF:master Nov 20, 2023
6 checks passed

mkl-public reviewed Dec 4, 2023

View reviewed changes

andreasrosdal pushed a commit that referenced this pull request Dec 8, 2023

Fixed issues discussed in #992 (#1003)

51ff9b0

This was referenced Feb 13, 2024

Remaining issue with form flattening #1047

Closed

Fix for issues #1047 #1050

Merged

andreasrosdal pushed a commit that referenced this pull request Feb 14, 2024

Fix for issues #1047 (#1050)

c6027b5

* Fixed issues discussed in #992 * Fix for Issue #1047

Lonzak mentioned this pull request Oct 23, 2024

Signature form field flattening distortion issue #1227

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed several form flattening issues #992

Fixed several form flattening issues #992

Lonzak commented Nov 15, 2023

Lonzak commented Nov 15, 2023

sonarqubecloud bot commented Nov 15, 2023

andreasrosdal commented Nov 16, 2023

Lonzak commented Nov 17, 2023

Lonzak commented Nov 20, 2023

andreasrosdal commented Nov 24, 2023

Lonzak commented Nov 24, 2023 •

edited

Loading

andreasrosdal commented Nov 24, 2023 •

edited

Loading

Lonzak commented Nov 24, 2023

mkl-public commented Nov 28, 2023

mkl-public left a comment

mkl-public Dec 4, 2023

Lonzak Dec 5, 2023 •

edited

Loading

mkl-public Dec 5, 2023

mkl-public Dec 4, 2023

Lonzak Dec 5, 2023 •

edited

Loading

mkl-public Dec 5, 2023 •

edited

Loading

Lonzak Dec 5, 2023

mkl-public Dec 4, 2023

mkl-public Dec 4, 2023

Lonzak Dec 5, 2023

mkl-public Dec 5, 2023

Lonzak Dec 6, 2023

mkl-public Dec 7, 2023

mkl-public Dec 4, 2023 •

edited

Loading

Lonzak Dec 5, 2023 •

edited

Loading

mkl-public Dec 5, 2023

mkl-public Dec 4, 2023

Lonzak Dec 5, 2023

andreasrosdal commented Dec 8, 2023

Lonzak commented Dec 8, 2023

Fixed several form flattening issues #992

Fixed several form flattening issues #992

Conversation

Lonzak commented Nov 15, 2023

Lonzak commented Nov 15, 2023

sonarqubecloud bot commented Nov 15, 2023

andreasrosdal commented Nov 16, 2023

Lonzak commented Nov 17, 2023

Lonzak commented Nov 20, 2023

andreasrosdal commented Nov 24, 2023

Lonzak commented Nov 24, 2023 • edited Loading

andreasrosdal commented Nov 24, 2023 • edited Loading

Lonzak commented Nov 24, 2023

mkl-public commented Nov 28, 2023

mkl-public left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Lonzak Dec 5, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Lonzak Dec 5, 2023 • edited Loading

Choose a reason for hiding this comment

mkl-public Dec 5, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mkl-public Dec 4, 2023 • edited Loading

Choose a reason for hiding this comment

Lonzak Dec 5, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andreasrosdal commented Dec 8, 2023

Lonzak commented Dec 8, 2023

Lonzak commented Nov 24, 2023 •

edited

Loading

andreasrosdal commented Nov 24, 2023 •

edited

Loading

Lonzak Dec 5, 2023 •

edited

Loading

Lonzak Dec 5, 2023 •

edited

Loading

mkl-public Dec 5, 2023 •

edited

Loading

mkl-public Dec 4, 2023 •

edited

Loading

Lonzak Dec 5, 2023 •

edited

Loading