[GitHub] [pdfbox] bernhardf-ro commented on a diff in pull request #149: Allow negative struct parent keys in PDFMergerUtility
bernhardf-ro commented on code in PR #149:
URL: https://github.com/apache/pdfbox/pull/149#discussion_r1082619424
##
pdfbox/src/main/java/org/apache/pdfbox/multipdf/PDFMergerUtility.java:
##
@@ -1498,8 +1499,8 @@ private void updateStructParentEntries(PDPage page, int
structParentOffset) thro
List newannots = new ArrayList<>(annots.size());
annots.forEach(annot ->
{
-int structParent = annot.getStructParent();
-if (structParent >= 0)
+int structParent =
annot.getCOSObject().getInt(COSName.STRUCT_PARENT, Integer.MIN_VALUE); // allow
for negative struct parent values
Review Comment:
That would be an API change which would break all specific (i.e. positiv or
negative equals) checks for -1 in all integrations.
If a one-parameter method that defaults to `Integer.MIN_VALUE` is necessary
(IMHO it is not), it would have to be a new one, e.g. `getIntSigned(COSName)`
(and similar for array, possibly for other number types).
The only change in this regard that I consider necessary is improving the
API documentation of `getInt(COSName)` (and similar methods) to clarify that
they cannot be used if the result may be negative.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org
For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] lehmi commented on a diff in pull request #149: Allow negative struct parent keys in PDFMergerUtility
lehmi commented on code in PR #149:
URL: https://github.com/apache/pdfbox/pull/149#discussion_r1082157988
##
pdfbox/src/main/java/org/apache/pdfbox/multipdf/PDFMergerUtility.java:
##
@@ -1498,8 +1499,8 @@ private void updateStructParentEntries(PDPage page, int
structParentOffset) thro
List newannots = new ArrayList<>(annots.size());
annots.forEach(annot ->
{
-int structParent = annot.getStructParent();
-if (structParent >= 0)
+int structParent =
annot.getCOSObject().getInt(COSName.STRUCT_PARENT, Integer.MIN_VALUE); // allow
for negative struct parent values
Review Comment:
So the question is, do we need to change the default value if the value
isn't set?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org
For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] knazarenkov commented on a diff in pull request #151: PDResource font cache improvement
knazarenkov commented on code in PR #151: URL: https://github.com/apache/pdfbox/pull/151#discussion_r1081602283 ## pdfbox/src/main/java/org/apache/pdfbox/pdmodel/PDResources.java: ## @@ -95,6 +96,24 @@ public PDResources(COSDictionary resourceDictionary, ResourceCache resourceCache } resources = resourceDictionary; cache = resourceCache; +directFontCache = new HashMap>(); +} + +/** + * Constructor for reading. + * + * @param resourceDictionary The cos dictionary for this resource. + * @param resourceCache The document's resource cache, may be null. + */ Review Comment: While adding javadoc I realised that mutability of cache cannot be deduced from method signature by reader - someone could try to use immutable map as parameter, so new type is introduced that is mutable by design - PDFontCache. Javadoc added also -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] Toparvion opened a new pull request, #152: FontBox: Add a method to view composite glyph components
Toparvion opened a new pull request, #152: URL: https://github.com/apache/pdfbox/pull/152 This PR is a follow-up of the [discussion](https://lists.apache.org/thread/d6zvnf7f5m9y200wvytm6zb824xkkxll) in Apache PDFBox mailing list. The change includes only the addition of a new method; no access modifiers were altered. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] THausherr commented on pull request #151: PDResource font cache improvement
THausherr commented on PR #151: URL: https://github.com/apache/pdfbox/pull/151#issuecomment-1387607173 Yeah I think this makes sense. Builds pass. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] THausherr commented on a diff in pull request #151: PDResource font cache improvement
THausherr commented on code in PR #151: URL: https://github.com/apache/pdfbox/pull/151#discussion_r1073956334 ## pdfbox/src/main/java/org/apache/pdfbox/pdmodel/PDResources.java: ## @@ -95,6 +96,24 @@ public PDResources(COSDictionary resourceDictionary, ResourceCache resourceCache } resources = resourceDictionary; cache = resourceCache; +directFontCache = new HashMap>(); +} + +/** + * Constructor for reading. + * + * @param resourceDictionary The cos dictionary for this resource. + * @param resourceCache The document's resource cache, may be null. + */ Review Comment: please add a good javadoc line here, for the parameter -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] THausherr commented on a diff in pull request #151: PDResource font cache improvement
THausherr commented on code in PR #151: URL: https://github.com/apache/pdfbox/pull/151#discussion_r1073956334 ## pdfbox/src/main/java/org/apache/pdfbox/pdmodel/PDResources.java: ## @@ -95,6 +96,24 @@ public PDResources(COSDictionary resourceDictionary, ResourceCache resourceCache } resources = resourceDictionary; cache = resourceCache; +directFontCache = new HashMap>(); +} + +/** + * Constructor for reading. + * + * @param resourceDictionary The cos dictionary for this resource. + * @param resourceCache The document's resource cache, may be null. + */ Review Comment: please add a good javadoc line here -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] knazarenkov opened a new pull request, #151: PDResource font cache improvement
knazarenkov opened a new pull request, #151: URL: https://github.com/apache/pdfbox/pull/151 Given: PDF with AcroForm with N text fields and M fonts loaded. Problem: When PDAcroForm.getDefaultResources() is called, new PDResource with new direct font cache is created. So whenever PDTextField.setValue() is called, PDFontFactory.createFont(), which is expensive operation, is called M + 1 times - M times in AppearanceGeneratorHelper and once when parsing appearance string in PDDefaultAppearanceString. So setting default appearance for N fields would cause fonts to be loaded N * (M + 1) times. This PR's intention is to introduce AcroForm-level direct font cache that would be reused in each instance created by PDAcroForm.getDefaultResources() -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] bernhardf-ro commented on a diff in pull request #149: Allow negative struct parent keys in PDFMergerUtility
bernhardf-ro commented on code in PR #149:
URL: https://github.com/apache/pdfbox/pull/149#discussion_r1069164981
##
pdfbox/src/main/java/org/apache/pdfbox/multipdf/PDFMergerUtility.java:
##
@@ -1498,8 +1499,8 @@ private void updateStructParentEntries(PDPage page, int
structParentOffset) thro
List newannots = new ArrayList<>(annots.size());
annots.forEach(annot ->
{
-int structParent = annot.getStructParent();
-if (structParent >= 0)
+int structParent =
annot.getCOSObject().getInt(COSName.STRUCT_PARENT, Integer.MIN_VALUE); // allow
for negative struct parent values
Review Comment:
I intentionally kept the patch to one class and as simple and minimal as
possible, e.g. not renaming variables. So feel free to adapt it and let me know
if you have any questions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org
For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] lehmi commented on a diff in pull request #149: Allow negative struct parent keys in PDFMergerUtility
lehmi commented on code in PR #149:
URL: https://github.com/apache/pdfbox/pull/149#discussion_r1068971320
##
pdfbox/src/main/java/org/apache/pdfbox/multipdf/PDFMergerUtility.java:
##
@@ -1498,8 +1499,8 @@ private void updateStructParentEntries(PDPage page, int
structParentOffset) thro
List newannots = new ArrayList<>(annots.size());
annots.forEach(annot ->
{
-int structParent = annot.getStructParent();
-if (structParent >= 0)
+int structParent =
annot.getCOSObject().getInt(COSName.STRUCT_PARENT, Integer.MIN_VALUE); // allow
for negative struct parent values
Review Comment:
This doesn't change anything but the default value if STRUCT_PARENT is null.
If Integer.MIN_VALUE makes more sense than -1 the getter should be changed
rather than this piece of code
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org
For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] bernhardf-ro commented on pull request #149: Allow negative struct parent keys in PDFMergerUtility
bernhardf-ro commented on PR #149:
URL: https://github.com/apache/pdfbox/pull/149#issuecomment-1378817161
Here is a document to verify the issue. (Sorry, I forgot to add that to the
issue in the first place.)
[merge.pdf](https://github.com/apache/pdfbox/files/10393126/merge.pdf)
It is a valid PDF/UA-1, according to PDF Accessibility Checker 2021.
Appending it to itself using PDFMergerUtility results in a document that is
not valid PDF/UA. ("Structural parent tree" issue)
With the patch applied the merge result is valid PDF/UA-1.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org
For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] Etienne-Gautier opened a new pull request, #150: better deviceCMYK profile
Etienne-Gautier opened a new pull request, #150: URL: https://github.com/apache/pdfbox/pull/150 I noticed that the color conversions were not always accurate for CMYK PDFs. I tested some examples and it seems like the default color profile used by PDFBox is not a very close match to U.S. Web Coated (SWOP) v2 (which can't be used directly for licensing reasons as per the in-code comment). I have found that the icc profile [here](https://github.com/saucecontrol/Compact-ICC-Profiles#cmyk) under creative commons CC0 license is much closer visually to U.S. Web Coated (SWOP) v2. As stated on their readme, this CMYK profile seems to be designed for the purpose of replacing U.S. Web Coated (SWOP) v2 when decoding images that don't have an embedded profile: > The primary purpose of this profile is to serve as a default for image viewing or conversion software to use when a CMYK image does not contain an embedded profile. It cannot be used for conversion to CMYK or for other rendering intents. Here are a few examples that show the difference between these color profiles for the attached PDF [HiAce_online_brochure JAN2021 page 2 only.pdf](https://github.com/apache/pdfbox/files/10387964/HiAce_online_brochure.JAN2021.page.2.only.pdf) With U.S. Web Coated (SWOP) v2 - as opened in Acrobat: https://user-images.githubusercontent.com/25138293/211694418-5d473b09-4b64-498f-965e-cea8fb5a6e66.png;> With ISOcoated_v2_300_bas.icc (used by PDFBox currently), screenshot taken from pdfbox debugger: https://user-images.githubusercontent.com/25138293/21169-c0525feb-42d4-4fed-b07d-5f8358468252.png;> With CGATS001Compat-v2-micro.icc (proposed change), screenshot taken from pdfbox debugger: https://user-images.githubusercontent.com/25138293/211694469-7f9db9b8-f90e-4e09-a27e-a6c9dc7882c8.png;> -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] THausherr commented on pull request #149: Allow negative struct parent keys in PDFMergerUtility
THausherr commented on PR #149: URL: https://github.com/apache/pdfbox/pull/149#issuecomment-1377581995 Do you have a PDF where this happens? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] asfgit closed pull request #148: Fix meta markup in HTML generation
asfgit closed pull request #148: Fix meta markup in HTML generation URL: https://github.com/apache/pdfbox/pull/148 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] bernhardf-ro opened a new pull request, #149: Allow negative struct parent keys in PDFMergerUtility
bernhardf-ro opened a new pull request, #149: URL: https://github.com/apache/pdfbox/pull/149 PDFMergerUtility expects structural parent tree keys to be non-negative. However, negative values don't seem to be forbidden by the specification and are accepted by validators. This patch adapts the class to handle negative struct parent keys correctly. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] guillaume-alvarez opened a new pull request, #148: Fix meta markup in HTML generation
guillaume-alvarez opened a new pull request, #148: URL: https://github.com/apache/pdfbox/pull/148 The current code generate incorrect HTML, a double-quote is never closed in ``` ``` The meta.content attribute does not need internal double-quotes so I removed it. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] lehmi commented on pull request #107: potential memory leaks and small performance improvements
lehmi commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1305881372 >  Fixed, thanks for pointer -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements
valerybokov commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1305398903  -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] velyan closed pull request #147: [PDFBOX-5526] Adding subsampling for masks
velyan closed pull request #147: [PDFBOX-5526] Adding subsampling for masks URL: https://github.com/apache/pdfbox/pull/147 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] velyan commented on pull request #147: [PDFBOX-5526] Adding subsampling for masks
velyan commented on PR #147: URL: https://github.com/apache/pdfbox/pull/147#issuecomment-1297808996 > You're not passing the region despite having it as a parameter Thanks, fixed! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] lehmi commented on pull request #107: potential memory leaks and small performance improvements
lehmi commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1297204723 Those buffers are closed when calling scratchFile.close and that is what the test is about -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements
valerybokov commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1297194157 A new test ScratchFileBufferTest.testBuffersClosed method. Should the scratchFileBuffer2 and scratchFileBuffer4 be closed after checks? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] lehmi commented on a diff in pull request #147: [PDFBOX-5526] Adding subsampling for masks
lehmi commented on code in PR #147:
URL: https://github.com/apache/pdfbox/pull/147#discussion_r1009187853
##
pdfbox/src/main/java/org/apache/pdfbox/pdmodel/graphics/image/PDImageXObject.java:
##
@@ -542,18 +542,22 @@ public BufferedImage getStencilImage(Paint paint) throws
IOException
/**
* Returns an RGB buffered image containing the opaque image stream
without any masks applied.
* If this Image XObject is a mask then the buffered image will contain
the raw mask.
+ * @param region The region of the source image to get, or null if the
entire image is needed.
+ * The actual region will be clipped to the dimensions of
the source image.
+ * @param subsampling The amount of rows and columns to advance for every
output pixel, a value
+ * of 1 meaning every pixel will be read. It must not be larger than the
image width or height.
* @return the image without any masks applied
* @throws IOException if the image cannot be read
*/
-public BufferedImage getOpaqueImage() throws IOException
+public BufferedImage getOpaqueImage(Rectangle region, int subsampling)
throws IOException
{
-return SampledImageReader.getRGBImage(this, null);
+return SampledImageReader.getRGBImage(this, null, subsampling, null);
Review Comment:
The region isn't passed to getRGBImage
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org
For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] velyan opened a new pull request, #147: [PDFBOX-5526] Adding subsampling for masks
velyan opened a new pull request, #147: URL: https://github.com/apache/pdfbox/pull/147 See[ Jira ticket ](https://issues.apache.org/jira/browse/PDFBOX-5526?page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel=17618666#comment-17618666) When `PDImageXObject.getImage()` gets invoked with subsampling and region, internally it loads the entire mask into memory: https://github.com/apache/pdfbox/blob/961c052d52dd9ab2dd3d7cd762a5046e5cc85a91/pdfbox/src/main/java/org/apache/pdfbox/pdmodel/graphics/image/PDImageXObject.java#L465 and applies the entire mask on the subsampled image. Which is extra work and can cause `OOM` exceptions. The proposed optimisation is to pass `region` and `subsampling` params to `PDImageXObject.getOpaqueImage() `here - https://github.com/apache/pdfbox/blob/961c052d52dd9ab2dd3d7cd762a5046e5cc85a91/pdfbox/src/main/java/org/apache/pdfbox/pdmodel/graphics/image/PDImageXObject.java#L548 such that masks are also subsampled before applying them... -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] lehmi commented on pull request #107: potential memory leaks and small performance improvements
lehmi commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1257503243 > > Hi, @THausherr! One test fails. SplitterTest was added by me a few months ago. PDDocument.setHighestImportedObjectNumber throws a NoSuchElementException because the indirectObjectKeys variable is empty and the test fails. Maybe i created wrong document, > > Please post the whole stack trace. I've found the issue and fixed it. @valerybokov Thanks for the pointer -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] lehmi commented on pull request #107: potential memory leaks and small performance improvements
lehmi commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1257267876 > Hi, @THausherr! One test fails. SplitterTest was added by me a few months ago. PDDocument.setHighestImportedObjectNumber throws a NoSuchElementException because the indirectObjectKeys variable is empty and the test fails. Maybe i created wrong document, Please post the whole stack trace. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements
valerybokov commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1257198654 > I don't really understand the last comment, but this may be related to changes in PDFBOX-5489. It is test in my fork. Yes, it looks like problem from PDFBOX-548. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements
THausherr commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1257194406 I don't really understand the last comment, but this may be related to changes in PDFBOX-5489. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements
valerybokov commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1257182580 Hi, @THausherr! I got a bug in my version of library. Several months ago SplitterTest was added from me. The PDDocument.setHighestImportedObjectNumber throws with a NoSuchElementException because importedPage variable is empty and test fails. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] asfgit closed pull request #146: PDFBOX-5514: case insensitive font lookup
asfgit closed pull request #146: PDFBOX-5514: case insensitive font lookup URL: https://github.com/apache/pdfbox/pull/146 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] Schmidor opened a new pull request, #146: PDFBOX-5514: case insensitive font lookup
Schmidor opened a new pull request, #146: URL: https://github.com/apache/pdfbox/pull/146 map key setting and checking always in lower case -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] AnkushMudgal closed pull request #141: [PDFBOX Refactoring] Refactored the Code to resolve some Design & Implementation Code Smells.
AnkushMudgal closed pull request #141: [PDFBOX Refactoring] Refactored the Code to resolve some Design & Implementation Code Smells. URL: https://github.com/apache/pdfbox/pull/141 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements
valerybokov commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1229397504 > ttfFont is still null if parse() fails. One could argue that `ff2Stream.getCOSObject().createView()` should be closed if parse() fails, but that wasn't your argument. :) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements
THausherr commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1229395121 ttfFont is still null if parse() fails. One could argue that `ff2Stream.getCOSObject().createView()` should be closed if parse() fails, but that wasn't your argument. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements
valerybokov commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1229394668 > Re 1: this is a known problem, a comment mentions PDFBOX-4963 Re 2: TTFParser does not have a close method It was ttfFont.  -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements
valerybokov commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1229394240 >  -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements
THausherr commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1229364861 Re 1: this is a known problem, a comment mentions PDFBOX-4963 Re 2: TTFParser does not have a close method -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements
valerybokov commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1229247301 Hi, @THausherr! I found two methods with possible memory leaks. 1 TrueTypeFont.finalize. If the super.finalize method will throw an exception then the close method will not be called. Maybe you will use try-finally. Additionally, it is a bit weird code when you call finalize and then close methods. 2 PDTrueTypeFont.PDTrueTypeFont(COSDictionary). The ttfParser instance will not be closed if the parse method will throw an exception. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] lgtm-com[bot] commented on pull request #107: potential memory leaks and small performance improvements
lgtm-com[bot] commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1214322456 This pull request **introduces 1 alert** and **fixes 2** when merging a392a0effcbbaebc3901da149ecabaa4a117496f into 8212dade7e677d303c3119294c429b7c0f4d6aa0 - [view on LGTM.com](https://lgtm.com/projects/g/apache/pdfbox/rev/pr-cb0d17215b91b9fc14e61a1daf6230ed97038791) **new alerts:** * 1 for Dereferenced variable may be null **fixed alerts:** * 2 for Dereferenced variable may be null -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements
THausherr commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1207141365 > Hi, @THausherr ! The testPDFBox5484. Don't you need to free the doc variable? Thanks, will fix this later. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] lgtm-com[bot] commented on pull request #107: potential memory leaks and small performance improvements
lgtm-com[bot] commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1206621146 This pull request **introduces 1 alert** and **fixes 2** when merging 4c6be6af9e7ed3f9b1543238bd20126e5822bff5 into bc996b06101f092db4236f3e87a007b787a4795d - [view on LGTM.com](https://lgtm.com/projects/g/apache/pdfbox/rev/pr-bf939699289aa3ae785886114d3d1f38ac9e7739) **new alerts:** * 1 for Dereferenced variable may be null **fixed alerts:** * 2 for Dereferenced variable may be null -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] lgtm-com[bot] commented on pull request #107: potential memory leaks and small performance improvements
lgtm-com[bot] commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1206522790 This pull request **introduces 1 alert** and **fixes 2** when merging a5c3022494d853bf22c1316aef9e1d016852bca6 into bc996b06101f092db4236f3e87a007b787a4795d - [view on LGTM.com](https://lgtm.com/projects/g/apache/pdfbox/rev/pr-6e1d539633acf40ba8120019bdedd0efb91e357e) **new alerts:** * 1 for Dereferenced variable may be null **fixed alerts:** * 2 for Dereferenced variable may be null -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements
valerybokov commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1206351676 Hi, @THausherr ! The testPDFBox5484. Don't you need to free the doc variable? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox-docs] asfgit closed pull request #9: PDFBOX-5475: remove period from "via. javadoc.io"
asfgit closed pull request #9: PDFBOX-5475: remove period from "via. javadoc.io" URL: https://github.com/apache/pdfbox-docs/pull/9 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox-docs] jamesdaily opened a new pull request, #9: PDFBOX-5475: remove period from "via. javadoc.io"
jamesdaily opened a new pull request, #9: URL: https://github.com/apache/pdfbox-docs/pull/9 PDFBOX-5475: remove period from "via. javadoc.io" to make it "via javadoc.io" -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements
THausherr commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1186577537 > As I understood, my commit has no bug and it is simpler then old version. Yes + yes and I've of course committed it. Sorry again for my mistake. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements
valerybokov commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1186576956 > Sorry, I completely missed that the original returns had been removed. I don't know how this happened because it's obvious. Your code example is fine too. As I understood, my commit has no bug and it is simpler then old version. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements
THausherr commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1186576212 Sorry, I completely missed that the original returns had been removed. I don't know how this happened because it's obvious. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements
valerybokov commented on PR #107:
URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1186575091
>
Heh! I believe, the De Morgan's law works. I tried to interpolate this code
to a little test. Tell me, please, where is bug?
import java.util.ArrayList;
public class HelloWorld{
public static void main(String []args){
System.out.println(v1() + " " + v2());
}
static boolean v1(){
ArrayListlist = new ArrayList();
// list.add(2);
list.add(4);
//list.add(5);
if (!list.contains(2) && !list.contains(4) && !list.contains(5))
return false;
return true;
}
static boolean v2(){
ArrayListlist = new ArrayList();
//list.add(2);
list.add(4);
//list.add(5);
return list.contains(2) || list.contains(4) || list.contains(5);
}
}
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org
For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] lehmi commented on pull request #107: potential memory leaks and small performance improvements
lehmi commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1186565205 > > > > > re [85368a5](https://github.com/apache/pdfbox/pull/107/commits/85368a5312e69b3a863e9163f19d63c8945b6c3f) - this is good but shouldn't the returns be changed as well? > > re those 2 LGTM alerts - can you please revert the `getDelta()` change in your huge PR (I've already mentioned that I don't want to introduce that change because it makes the code less readable, but LGTM keeps mentioning it) and fix the `getSignatureFields()` change (it ignores "fields") > > Also, this optimization of `getSignatureFields()` isn't needed because this is just a null check against fuzzers etc, this shouldn't be null in production. > > "shouldn't the returns be changed as well?" I didn't get. What I see: > > Your version //if all not contains if (!true && !true && !true) return false //(then) one or more contains return true > > My version //if one or more contains else false return true || true || true > > So, logic is the same but the code has been simplified. This is De Morgan's law https://en.wikipedia.org/wiki/De_Morgan%27s_laws And, yes the return value has to be changed and it is changed. The origin returns "false" if the condition is true and the proposed solution returns "true". Everything is fine -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] lgtm-com[bot] commented on pull request #107: potential memory leaks and small performance improvements
lgtm-com[bot] commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1186563232 This pull request **introduces 1 alert** when merging ca13076cba7e517981704382443496eee56042e0 into 665021ad23690963313c6ce38c2e5e538e9fd85c - [view on LGTM.com](https://lgtm.com/projects/g/apache/pdfbox/rev/pr-e4aa441ddcce21e2cb881fb9e46aba97a9b212ec) **new alerts:** * 1 for Container contents are never accessed -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements
valerybokov commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1186562664 > > re [85368a5](https://github.com/apache/pdfbox/pull/107/commits/85368a5312e69b3a863e9163f19d63c8945b6c3f) - this is good but shouldn't the returns be changed as well? > > re those 2 LGTM alerts - can you please revert the `getDelta()` change in your huge PR (I've already mentioned that I don't want to introduce that change because it makes the code less readable, but LGTM keeps mentioning it) and fix the `getSignatureFields()` change (it ignores "fields") > > Also, this optimization of `getSignatureFields()` isn't needed because this is just a null check against fuzzers etc, this shouldn't be null in production. "shouldn't the returns be changed as well?" I didn't get. What I see: Your version //if all not contains if (!true && !true && !true) return false //(then) one or more contains return true My version //if one or more contains else false return true || true || true So, logic is the same but the code has been simplified. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements
THausherr commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1186512434 re [85368a5](https://github.com/apache/pdfbox/pull/107/commits/85368a5312e69b3a863e9163f19d63c8945b6c3f) - this is good but shouldn't the returns be changed as well? re those 2 LGTM alerts - can you please revert the "getDelta()" change in your huge PR (I've already mentioned that I don't want to introduce that change because it makes the code less readable, but LGTM keeps mentioning it) and fix the "getSignatureFields()" change (it ignores "fields") -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] lgtm-com[bot] commented on pull request #107: potential memory leaks and small performance improvements
lgtm-com[bot] commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1186498241 This pull request **introduces 2 alerts** when merging e3eb3de498644559f862aff391a9e74208a25e63 into bd328d11942e9e11937dda399ab285085f5458ce - [view on LGTM.com](https://lgtm.com/projects/g/apache/pdfbox/rev/pr-8506da95480eeaa807b77ca8e132a02267421235) **new alerts:** * 1 for Container contents are never accessed * 1 for Boxed variable is never null -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements
THausherr commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1186450214 > PDDocument.saveIncrementalForExternalSigning. > If the getSignatureDictionaries method returns an empty collection (string...), you throw an IllegalStateException. You break the loop if any signature needs to be updated. If the collection is not empty and there is no updatable signature, the foundSignature variable will not be null and you will also do this incremental save. Is this the correct behavior, I mean do you need to do this incremental save on a non-empty collection? See the comment above that starts with "PDFBOX-3978", yes this is wanted. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements
valerybokov commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1186438627  -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements
valerybokov commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1186435932 > > Hi, @THausherr! The few my observations: > > 1 AnnotationBorder.underline - unused field. > > 2 CCITTFactory.readshort - useless if-statement (wrong if body?). > > 3 PDCaretAppearanceHandler - line 73. Operation result is ignored (matrix.transformPoint(rd, rd);) > > 1. true, but I'll keep it. This is because PDLinkAppearanceHandler doesn't use AnnotationBorder for some reason. Maybe it should, but this isn't really that important. From what I see, this would require more changes, some of them breaking. > 2. I don't understand. TIFF files start with "II" or "MM". > 3. thanks, removed. I have no idea why I did this. About second item. As I understood, bitwise OR operation doesn't care about arguments order:) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements
THausherr commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1186433593 > Hi, @THausherr! The few my observations: > 1 AnnotationBorder.underline - unused field. > 2 CCITTFactory.readshort - useless if-statement (wrong if body?). > 3 PDCaretAppearanceHandler - line 73. Operation result is ignored (matrix.transformPoint(rd, rd);) 1) true, but I'll keep it. This is because PDLinkAppearanceHandler doesn't use AnnotationBorder for some reason. Maybe it should, but this isn't really that important. From what I see, this would require more changes, some of them breaking. 2) I don't understand. TIFF files start with "II" or "MM". 3) thanks, removed. I have no idea why I did this. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements
valerybokov commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1186432401 COSDocument.saveIncrementalForExternalSigning. If the getSignatureDictionaries method returns an empty collection (string...), you throw an IllegalStateException. You break the loop if any signature needs to be updated. If the collection is not empty and there is no updatable signature, the foundSignature variable will not be null and you will also do this incremental save. Is this the correct behavior, I mean do you need to do this incremental save on a non-empty collection? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements
valerybokov commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1186421868 Hi, @THausherr! The few my observations: 1 AnnotationBorder.underline - unused field. 2 CCITTFactory.readshort - useless if-statement (wrong if body?). 3 PDCaretAppearanceHandler - line 73. Operation result is ignored (matrix.transformPoint(rd, rd);) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] lgtm-com[bot] commented on pull request #107: potential memory leaks and small performance improvements
lgtm-com[bot] commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1186240965 This pull request **introduces 1 alert** when merging fabcb363304653a4abfc15060df99a69baf3fbfe into 7e78974d59db8f6634e08bbc6dcb79530eb26570 - [view on LGTM.com](https://lgtm.com/projects/g/apache/pdfbox/rev/pr-7d931f0f0c4984e0b1b2f073c91c7ab1df3853ae) **new alerts:** * 1 for Boxed variable is never null -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] lgtm-com[bot] commented on pull request #107: potential memory leaks and small performance improvements
lgtm-com[bot] commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1186174760 This pull request **introduces 1 alert** when merging 57904b1a3e5b8353ac9477c9017387d36b5df35a into 7e78974d59db8f6634e08bbc6dcb79530eb26570 - [view on LGTM.com](https://lgtm.com/projects/g/apache/pdfbox/rev/pr-4318ebb933fe275e683bcb37f203d305dce6a344) **new alerts:** * 1 for Boxed variable is never null -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements
valerybokov commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1180247890 > > > existing code is better IMHO. And whoever wrote this, intended to use an array instead of a list. > > > > > > You are converting an array to a list in the getXRefRanges method. You use more memory and time to do this. I just removed the redundant operation. You are used to using lists. Only on this occasion you say that the existing code is better. > > getXRefRanges() starts with a list when it calls `new ArrayList<>()`. The current code converts this to an array at the end with `return list.toArray(new Long[list.size()]);` and loses the list. Yes. Just I mean, IMHO, it is redurant convertion. I found only one reason to no do that is backward compatibility (getXRefRanges is protected). But you often use lists and rarely arrays. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements
THausherr commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1179741986 > > existing code is better IMHO. And whoever wrote this, intended to use an array instead of a list. > > You are converting an array to a list in the getXRefRanges method. You use more memory and time to do this. I just removed the redundant operation. You are used to using lists. Only on this occasion you say that the existing code is better. getXRefRanges() starts with a list when it calls `new ArrayList<>()`. The current code converts this to an array at the end with `return list.toArray(new Long[list.size()]);` and loses the list. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements
valerybokov commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1179682790 > existing code is better IMHO. And whoever wrote this, intended to use an array instead of a list. You are converting an array to a list in the getXRefRanges method. You use more memory and time to do this. I just removed the redundant operation. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] asfgit closed pull request #145: [PDFBOX-5471] Fall back to an empty `PDRectangle` when Transparency Group `BBox` entry is missing
asfgit closed pull request #145: [PDFBOX-5471] Fall back to an empty `PDRectangle` when Transparency Group `BBox` entry is missing URL: https://github.com/apache/pdfbox/pull/145 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] higuaro opened a new pull request, #145: [PDFBOX-5471] Fall back to an empty `PDRectangle` when Transparency Group `BBox` entry is missing
higuaro opened a new pull request, #145: URL: https://github.com/apache/pdfbox/pull/145 This PR is a potential fix for https://issues.apache.org/jira/browse/PDFBOX-5471 The change consist of falling back to an empty `PDRectangle` if the Transparency Group `BBox` entry is missing. After some testing with some PDF viewer native clients, we couldn't find any visual artifacts or differences. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] higuaro closed pull request #144: [feature-request] Add a setter for `PageDrawer.graphics`
higuaro closed pull request #144: [feature-request] Add a setter for `PageDrawer.graphics` URL: https://github.com/apache/pdfbox/pull/144 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] lgtm-com[bot] commented on pull request #107: potential memory leaks and small performance improvements
lgtm-com[bot] commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1158855880 This pull request **introduces 1 alert** when merging ad9d96895a3998a78ee40c597ff0daa688b20101 into 742f8f3a6a567c8f779bf2f55159c34f57e0d62c - [view on LGTM.com](https://lgtm.com/projects/g/apache/pdfbox/rev/pr-0211006b41e68c37744406c57b241be75c745815) **new alerts:** * 1 for Boxed variable is never null -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] lehmi commented on pull request #107: potential memory leaks and small performance improvements
lehmi commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1158853322 >  I've fixed that, thanks for the pointer -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] lgtm-com[bot] commented on pull request #107: potential memory leaks and small performance improvements
lgtm-com[bot] commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1158758453 This pull request **introduces 1 alert** when merging 1858424b8a9f756a0c1d7f3e028698294b88d627 into d457f1c1dac8ef5baef540d34b1cdbf0fd09beb3 - [view on LGTM.com](https://lgtm.com/projects/g/apache/pdfbox/rev/pr-31232eb8e424703d075b82abde9b7d785fb07996) **new alerts:** * 1 for Boxed variable is never null -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements
valerybokov commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1158729508  -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements
THausherr commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1155945323 > Hi, @THausherr ! You did commit [53bd9bb](https://github.com/apache/pdfbox/commit/53bd9bb6cdaf01300aad13fa5632d830444a). How about line 249? fixed, thanks -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements
valerybokov commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1155610797 Hi, @THausherr ! You did commit https://github.com/apache/pdfbox/commit/53bd9bb6cdaf01300aad13fa5632d830444a. How about line 249? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] higuaro opened a new pull request, #144: [feature-request] Add a setter for `PageDrawer.graphics`
higuaro opened a new pull request, #144: URL: https://github.com/apache/pdfbox/pull/144 **Feature request** Hey folks, the following is to request a small but useful change in `PageDrawer`, consisting of adding a `protected` setter for the the `graphics` member variable. The idea behind this change is to be able to draw the entire content of a `TransparencyGroup` into a separate image. We already tried using `showTransparencyGroupOnGraphics()` but this doesn't work well for deeply nested and complicated transparency groups. We set up a proof of concept that sets the parent graphics to the target image through reflection and it worked, but our team would prefer to this through a less hacky way if possible. Keen to hear your thoughts and open to better ideas. **JIRA ticket** https://issues.apache.org/jira/browse/PDFBOX-5454 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] vk-github18 opened a new pull request, #143: Proof of concept for advanced glyph layout (2nd try)
vk-github18 opened a new pull request, #143: URL: https://github.com/apache/pdfbox/pull/143 Based on https://github.com/danfickle/pdfbox See examples/src/main/java/org/apache/pdfbox/examples/pdmodel/AdvancedTextLayoutSequencesDin91379.java Glyph layout looks good with NotoSans-Regular. Combinations with two diacritics work after reordering glyphs, using the patch for FOP-2969 and setting gdef in GlyphPositioningTable. Output of test is now: [TestDin91379AdvancedLayout-NotoSans-Regular.ttf-20.0.pdf](https://github.com/danfickle/pdfbox/files/8808716/TestDin91379AdvancedLayout-NotoSans-Regular.ttf-20.0.pdf) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] vk-github18 closed pull request #142: Proof of concept for advanced glyph layout
vk-github18 closed pull request #142: Proof of concept for advanced glyph layout URL: https://github.com/apache/pdfbox/pull/142 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] vk-github18 opened a new pull request, #142: Proof of concept for advanced glyph layout
vk-github18 opened a new pull request, #142: URL: https://github.com/apache/pdfbox/pull/142 Based on https://github.com/danfickle/pdfbox See examples/src/main/java/org/apache/pdfbox/examples/pdmodel/AdvancedTextLayoutSequencesDin91379.java Glyph layout looks good with NotoSans-Regular. Combinations with two diacritics work after reordering glyphs, using the patch for FOP-2969 and setting gdef in GlyphPositioningTable. Output of test is now: [TestDin91379AdvancedLayout-NotoSans-Regular.ttf-20.0.pdf](https://github.com/danfickle/pdfbox/files/8808716/TestDin91379AdvancedLayout-NotoSans-Regular.ttf-20.0.pdf) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] lgtm-com[bot] commented on pull request #107: potential memory leaks and small performance improvements
lgtm-com[bot] commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1145275849 This pull request **introduces 1 alert** when merging 0a44c64964776eed8e50a4671183b43535e330d9 into b56cfb9844e284bf60c5b7ffb0c6443e066ea120 - [view on LGTM.com](https://lgtm.com/projects/g/apache/pdfbox/rev/pr-f7407ac313610116ab55658b51761c554771d0b3) **new alerts:** * 1 for Boxed variable is never null -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] lgtm-com[bot] commented on pull request #107: potential memory leaks and small performance improvements
lgtm-com[bot] commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1145228857 This pull request **introduces 1 alert** when merging 9b273b181a96fd913f59643cb539a04481d7c50d into b56cfb9844e284bf60c5b7ffb0c6443e066ea120 - [view on LGTM.com](https://lgtm.com/projects/g/apache/pdfbox/rev/pr-11589ac2e6d6057ea0d090e3398a3a7165800ec3) **new alerts:** * 1 for Boxed variable is never null -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements
valerybokov commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1121512722 > > > > Hi, @THausherr ! 1 The method PDFPrintable.print has affine transform clonning (260). Is this operation really needed or is it old code? 2 the classes from org.apache.pdfbox.pdmodel.font.encoding packege have public constructors but they have field with comment "Singleton instance of this class". The same for a packege org.apache.fontbox.encoding. 3 The packages org.apache.fontbox.encoding and org.apache.pdfbox.pdmodel.font.encoding contain the MacRomanEncoding class. The encoding tables of these classes are not much different. Perhaps they were separated due to different encoding tables. > > > > > > > > > Thanks, I made two of the three proposed changes. I prefer not to tough the third one due to the difference. > > > > > > Hi, @THausherr! About PDFPrintable, line 260. I thought you would remove the AffineTransform instance from the code (remove those lines). I can't understand the whole logic of this code, but why do you need to set a transformation clone (check the graphics2D.getTransform java doc) on line 269? > > I have removed the clone, maybe you have an older version of the code. I kept the rest of the code. The purpose of this part is to restore the original transform when painting the border. Thank you for respond. I understood. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements
THausherr commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1121395542 > > > Hi, @THausherr ! 1 The method PDFPrintable.print has affine transform clonning (260). Is this operation really needed or is it old code? 2 the classes from org.apache.pdfbox.pdmodel.font.encoding packege have public constructors but they have field with comment "Singleton instance of this class". The same for a packege org.apache.fontbox.encoding. 3 The packages org.apache.fontbox.encoding and org.apache.pdfbox.pdmodel.font.encoding contain the MacRomanEncoding class. The encoding tables of these classes are not much different. Perhaps they were separated due to different encoding tables. > > > > > > Thanks, I made two of the three proposed changes. I prefer not to tough the third one due to the difference. > > Hi, @THausherr! About PDFPrintable, line 260. I thought you would remove the AffineTransform instance from the code (remove those lines). I can't understand the whole logic of this code, but why do you need to set a transformation clone (check the graphics2D.getTransform java doc) on line 269? I have removed the clone, maybe you have an older version of the code. I kept the rest of the code. The purpose of this part is to restore the original transform when painting the border. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] lgtm-com[bot] commented on pull request #107: potential memory leaks and small performance improvements
lgtm-com[bot] commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1121170749 This pull request **introduces 1 alert** when merging 666c8b9b7d194b2a97768dd88c057997da3d6cce into 890c0b48dc954831e5279b9ac4abdba427e684b9 - [view on LGTM.com](https://lgtm.com/projects/g/apache/pdfbox/rev/pr-5d1edc93beeeb994b386afd746d53db78e95b5fd) **new alerts:** * 1 for Boxed variable is never null -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] lgtm-com[bot] commented on pull request #107: potential memory leaks and small performance improvements
lgtm-com[bot] commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1120934637 This pull request **introduces 1 alert** when merging eff811931a081b2250de973bdb4fd5204f5e5b96 into 890c0b48dc954831e5279b9ac4abdba427e684b9 - [view on LGTM.com](https://lgtm.com/projects/g/apache/pdfbox/rev/pr-5af397c1784dfbf47994fe9ec219d74b3c6d2a9f) **new alerts:** * 1 for Boxed variable is never null -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] lgtm-com[bot] commented on pull request #107: potential memory leaks and small performance improvements
lgtm-com[bot] commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1120911005 This pull request **introduces 1 alert** when merging c3fd0b036697f595d9f9859a54ac8c6e94093532 into 890c0b48dc954831e5279b9ac4abdba427e684b9 - [view on LGTM.com](https://lgtm.com/projects/g/apache/pdfbox/rev/pr-84448505eeff5779bbcc8e8b9e1563b61c62dc58) **new alerts:** * 1 for Boxed variable is never null -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] lgtm-com[bot] commented on pull request #107: potential memory leaks and small performance improvements
lgtm-com[bot] commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1120853328 This pull request **introduces 1 alert** when merging 99a9c8f03a4d5bf987784817dee90a676d6e0100 into 890c0b48dc954831e5279b9ac4abdba427e684b9 - [view on LGTM.com](https://lgtm.com/projects/g/apache/pdfbox/rev/pr-722e3d0681f63b487e7c6ae873a14dd498f5b7f9) **new alerts:** * 1 for Boxed variable is never null -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] lgtm-com[bot] commented on pull request #107: potential memory leaks and small performance improvements
lgtm-com[bot] commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1120477909 This pull request **introduces 1 alert** when merging 8b6b62ad50175350bfe934e24b9b2a3ab8354b80 into 72c750a4cf850c7c5ef5d3ad05956910a970ccef - [view on LGTM.com](https://lgtm.com/projects/g/apache/pdfbox/rev/pr-d05a50f1f304b4fc7922029c9dd5d9f660469f70) **new alerts:** * 1 for Boxed variable is never null -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements
valerybokov commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1120470783 > > Hi, @THausherr ! 1 The method PDFPrintable.print has affine transform clonning (260). Is this operation really needed or is it old code? 2 the classes from org.apache.pdfbox.pdmodel.font.encoding packege have public constructors but they have field with comment "Singleton instance of this class". The same for a packege org.apache.fontbox.encoding. 3 The packages org.apache.fontbox.encoding and org.apache.pdfbox.pdmodel.font.encoding contain the MacRomanEncoding class. The encoding tables of these classes are not much different. Perhaps they were separated due to different encoding tables. > > Thanks, I made two of the three proposed changes. I prefer not to tough the third one due to the difference. Hi, @THausherr! About PDFPrintable, line 260. I thought you would remove the AffineTransform instance from the code (remove those lines). I can't understand the whole logic of this code, but why do you need to set a transformation clone (check the graphics2D.getTransform java doc) on line 269? About encoding. You have changed classes from pdfbox but not from fontbox. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements
valerybokov commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1120468091 > This makes the code less readable IMHO, and the possible performance gain doesn't matter, LZW compression isn't used much (mostly on very old PDFs, because it's not efficient). Re your other question "it is possible": yes, it is, that part is green in the jacoco report, see .../pdfbox/target/site/jacoco/org.apache.pdfbox.filter/LZWFilter.java.html okay -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements
valerybokov commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1119851991 > PDFCloneUtility.checkForRecursion() check breaks support for a some existing PDFs. I've seen a lot of PDFs where the /Prev and /Next entries point to the parent element if there is no prev or next element. So this breaks support for a a lot of PDFs which could be processed before. Will be better if you will write it here: https://issues.apache.org/jira/projects/PDFBOX/issues/PDFBOX-5426?filter=allopenissues -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] sz5000 commented on pull request #107: potential memory leaks and small performance improvements
sz5000 commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1119675474 PDFCloneUtility.checkForRecursion() check breaks support for a some existing PDFs. I've seen a lot of PDFs where the /Prev and /Next entries point to the parent element if there is no prev or next element. So this breaks support for a a lot of PDFs which could be processed before. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements
THausherr commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1114219783 > generateAppearanceStreams > Proposition: move the method generateAppearanceStreams to PDAbstractAppearanceHandler Thanks, done. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements
THausherr commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1114028675 > Proposition: use java.util.concurrent.ConcurrentHashMap for CMapManager .cMapCache thanks, done and found and removed unneeded related code -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements
THausherr commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1113574945 > Okay. Third is warning. Just a strange code. How about my propositions? I assume you mean the other comments. I'll read them when I have the time. I have about 50 github mails from you that are kept unread so that I can work on them some day. Btw each new comment reduces the probability that I read your older comments because when I have some time, I always read the latest comment first. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements
valerybokov commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1113568288 > > Hi, @THausherr ! 1 The method PDFPrintable.print has affine transform clonning (260). Is this operation really needed or is it old code? 2 the classes from org.apache.pdfbox.pdmodel.font.encoding packege have public constructors but they have field with comment "Singleton instance of this class". The same for a packege org.apache.fontbox.encoding. 3 The packages org.apache.fontbox.encoding and org.apache.pdfbox.pdmodel.font.encoding contain the MacRomanEncoding class. The encoding tables of these classes are not much different. Perhaps they were separated due to different encoding tables. > > Thanks, I made two of the three proposed changes. I prefer not to tough the third one due to the difference. Okay. Third is warning. Just a strange code. How about my propositions? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] THausherr commented on pull request #107: potential memory leaks and small performance improvements
THausherr commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1108901829 > Hi, @THausherr ! 1 The method PDFPrintable.print has affine transform clonning (260). Is this operation really needed or is it old code? 2 the classes from org.apache.pdfbox.pdmodel.font.encoding packege have public constructors but they have field with comment "Singleton instance of this class". The same for a packege org.apache.fontbox.encoding. 3 The packages org.apache.fontbox.encoding and org.apache.pdfbox.pdmodel.font.encoding contain the MacRomanEncoding class. The encoding tables of these classes are not much different. Perhaps they were separated due to different encoding tables. Thanks, I made two of the three proposed changes. I prefer not to tough the third one due to the difference. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] lgtm-com[bot] commented on pull request #107: potential memory leaks and small performance improvements
lgtm-com[bot] commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1108332233 This pull request **introduces 1 alert** when merging e28826e9c1d38ce1129ede2a568b94285030370e into 19a3c1837d6fb20734e2ee7856128c8995394459 - [view on LGTM.com](https://lgtm.com/projects/g/apache/pdfbox/rev/pr-b299addd14aecd42c031a73c17973eb41003a155) **new alerts:** * 1 for Boxed variable is never null -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements
valerybokov commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1108301964 Proposition: use java.util.concurrent.ConcurrentHashMap for CMapManager .cMapCache -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements
valerybokov commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1108294910 Proposition: move the method generateAppearanceStreams to PDAbstractAppearanceHandler -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
[GitHub] [pdfbox] valerybokov commented on pull request #107: potential memory leaks and small performance improvements
valerybokov commented on PR #107: URL: https://github.com/apache/pdfbox/pull/107#issuecomment-1108249415 Hi, @THausherr ! 1 The method PDFPrintable.print has affine transform clonning (260). Is this operation really needed or is it old code? 2 the classes from org.apache.pdfbox.pdmodel.font.encoding packege have public constructors but they have field with comment "Singleton instance of this class". The same for a packege org.apache.fontbox.encoding. 3 The packages org.apache.fontbox.encoding and org.apache.pdfbox.pdmodel.font.encoding contain the MacRomanEncoding class. The encoding tables of these classes are not much different. Perhaps they were separated due to different encoding tables. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org For additional commands, e-mail: dev-h...@pdfbox.apache.org
