rototor commented on code in PR #210:
URL: https://github.com/apache/pdfbox/pull/210#discussion_r2203227340
##########
pdfbox/src/main/java/org/apache/pdfbox/pdmodel/graphics/image/PDImageXObject.java:
##########
@@ -383,6 +383,69 @@ public static PDImageXObject
createFromByteArray(PDDocument document, byte[] byt
throw new IllegalArgumentException("Image type " + fileType + " not
supported: " + name);
}
+ /**
+ * Create a PDImageXObject from an image byte array. This overloaded
version allows providing
+ * a custom factory to handle specific image formats, such as BMP and GIF,
or to act as a
+ * fallback strategy when the default converters (e.g., for PNG or TIFF)
fail.
+ *
+ * @param document the document that shall use this PDImageXObject.
+ * @param byteArray bytes from an image file.
+ * @param name name of image file for exception messages, can be null.
+ * @param defaultFactory optional factory used to handle BMP, GIF, or
fallback cases
+ * (e.g., for PNG or TIFF). If {@code null}, this
method delegates to
+ * {@link #createFromByteArray(PDDocument, byte[],
String)}.
+ * @return a PDImageXObject.
+ * @throws IOException if there is an error when reading the file or
creating the
+ * PDImageXObject.
+ * @throws IllegalArgumentException if the image type is not supported.
+ */
+ public static PDImageXObject createFromByteArray(PDDocument document,
byte[] byteArray, String name, DefaultFactory defaultFactory) throws IOException
Review Comment:
Here I'm a bit confused, also by the naming stuff.
I would rather extend the existing method with a parameter. I think
"DefaultFactory" is a bad name. I.e., the way I would implement this would be
```
// Method with signature of the existing method is just forwarding to the
other new overload
public static PDImageXObject createFromByteArray(PDDocument document, byte[]
byteArray, String name)
{
return createFromByteArray(document,byteArray,name, null);
}
// Extend the existing method with the one parameter
public static PDImageXObject createFromByteArray(PDDocument document, byte[]
byteArray, String name, DefaultFactory defaultFactory) throws {
// .. logic like it is now BUT instead of just encoding this lossless we
first check if we have a *fallback*
if( defaultFactory != null )
return defaultFactory.create...(...)
// otherwise just do the PNGEncoder Lossless stuff like it is now.
}
```
And therefor I think defaultFactory is bad name. It should be a fallback.
--
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
