You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2021/01/23 12:07:46 UTC
[GitHub] [commons-imaging] kinow opened a new pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
kinow opened a new pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116
Uses the approach with a POJO discussed in JIRA. It is similar to what was designed by @, https://github.com/mgmechanics/commons-imaging/blob/IMAGING-159-POJO/src/main/java/org/apache/commons/imaging/ImagingParameters.java.
Some values were removed (no more verbose, that's using JUL API now), and a few more changes to how the classes and interface are organized.
Still work-in-progress, aiming to discuss with @gwlucastrig and bring it to the mailing list for wider discussion.
@gwlucastrig some interesting issues:
- [ ] do we need a marker interface, or should `ImagingParameters` be the class with common parameters?
- [ ] initially I put the classes in the `.common` package (which says in the docs that stores classes common to multiple formats), but maybe it should go to `.imaging`, to avoid circular reference between `.imaging` and `.imaging.common`
- [ ] in many cases, `params` is optional, then later on the code might do a `if (params == null) params = new HashMap<>();`. But I think since we have an object now, we can have:
* some cases where params are optional; so we overload the method, defining `parseAbc(InputStream is) {...}` and also `parseAbc(Parameters params, InputStream is)` (i.e. if you want to, you can define the parameters, otherwise just call the method without params, and the method will call the other overloaded method with `this.parseAbc(new AbcParameters(), is)`.
* other cases it may be necessary to always have the parameters, so for this case we simply have a method with `parseAbc(Parameters params) { Objects.requireNotNull(params, ...) }`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] gwlucastrig commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
gwlucastrig commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-841791184
It looks like providing compatibility with Android OS may not be practical.
Android has its own graphics and media classes. So anything with java.awt
in it will not port. Common Imaging using BufferedImage extensively. Most
other AWT elements are used only a little. Color shows up the most. The
ICC_ColorSpace also shows up. A couple of the test applications use
Graphics to draw some test images as input.
I think I might post a question on Java Coderanch or Stackoverflow to ask
about compatible solutions.
On Sat, May 15, 2021 at 8:18 AM Bruno P. Kinoshita ***@***.***>
wrote:
> If those classes are from awt, the issue is that we have a dependency to
> java.desktop (I think that's the java module name) and also cause an issue
> to users of the API that are using Android (as IIUC, there's no awt in
> Android's Java SDK?).
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/commons-imaging/pull/116#issuecomment-841649340>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AEWJDYNES4M3XQCOJIEZN6TTNZRAXANCNFSM4WPUJSLQ>
> .
>
Search "java.awt" (229 hits in 121 files of 557 searched)
commons-imaging\src\main\java\org\apache\commons\imaging\ColorTools.java (11 hits)
Line 20: import java.awt.RenderingHints;
Line 21: import java.awt.Transparency;
Line 22: import java.awt.color.ColorSpace;
Line 23: import java.awt.color.ICC_ColorSpace;
Line 24: import java.awt.color.ICC_Profile;
Line 25: import java.awt.image.BufferedImage;
Line 26: import java.awt.image.ColorConvertOp;
Line 27: import java.awt.image.ColorModel;
Line 28: import java.awt.image.ComponentColorModel;
Line 29: import java.awt.image.DirectColorModel;
Line 30: import java.awt.image.ImagingOpException;
commons-imaging\src\main\java\org\apache\commons\imaging\common\BufferedImageFactory.java (1 hit)
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\common\ImageBuilder.java (9 hits)
Line 43: import java.awt.color.ColorSpace;
Line 44: import java.awt.image.BufferedImage;
Line 45: import java.awt.image.ColorModel;
Line 46: import java.awt.image.DataBuffer;
Line 47: import java.awt.image.DataBufferInt;
Line 48: import java.awt.image.DirectColorModel;
Line 49: import java.awt.image.Raster;
Line 50: import java.awt.image.RasterFormatException;
Line 51: import java.awt.image.WritableRaster;
commons-imaging\src\main\java\org\apache\commons\imaging\common\RgbBufferedImageFactory.java (1 hit)
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\common\SimpleBufferedImageFactory.java (1 hit)
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\bmp\BmpImageParser.java (2 hits)
Line 27: import java.awt.Dimension;
Line 28: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\bmp\BmpWriter.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\bmp\BmpWriterPalette.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\bmp\BmpWriterRgb.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\dcx\DcxImageParser.java (2 hits)
Line 23: import java.awt.Dimension;
Line 24: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\gif\GifImageParser.java (2 hits)
Line 28: import java.awt.Dimension;
Line 29: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\icns\IcnsDecoder.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\icns\IcnsImageParser.java (2 hits)
Line 23: import java.awt.Dimension;
Line 24: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\ico\IcoImageParser.java (2 hits)
Line 26: import java.awt.Dimension;
Line 27: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\jpeg\decoder\JpegDecoder.java (6 hits)
Line 21: import java.awt.image.BufferedImage;
Line 22: import java.awt.image.ColorModel;
Line 23: import java.awt.image.DataBuffer;
Line 24: import java.awt.image.DirectColorModel;
Line 25: import java.awt.image.Raster;
Line 26: import java.awt.image.WritableRaster;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\jpeg\JpegImageMetadata.java (2 hits)
Line 19: import java.awt.Dimension;
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\jpeg\JpegImageParser.java (2 hits)
Line 23: import java.awt.Dimension;
Line 24: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\pcx\PcxImageParser.java (11 hits)
Line 24: import java.awt.Dimension;
Line 25: import java.awt.Transparency;
Line 26: import java.awt.color.ColorSpace;
Line 27: import java.awt.image.BufferedImage;
Line 28: import java.awt.image.ColorModel;
Line 29: import java.awt.image.ComponentColorModel;
Line 30: import java.awt.image.DataBuffer;
Line 31: import java.awt.image.DataBufferByte;
Line 32: import java.awt.image.IndexColorModel;
Line 33: import java.awt.image.Raster;
Line 34: import java.awt.image.WritableRaster;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\pcx\PcxWriter.java (1 hit)
Line 18: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\png\PngImageParser.java (6 hits)
Line 25: import java.awt.Dimension;
Line 26: import java.awt.color.ColorSpace;
Line 27: import java.awt.color.ICC_ColorSpace;
Line 28: import java.awt.color.ICC_Profile;
Line 29: import java.awt.image.BufferedImage;
Line 30: import java.awt.image.ColorModel;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\png\PngWriter.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\png\ScanExpediter.java (1 hit)
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\png\ScanExpediterInterlaced.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\png\ScanExpediterSimple.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\pnm\PamWriter.java (1 hit)
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\pnm\PbmWriter.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\pnm\PgmWriter.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\pnm\PnmImageParser.java (2 hits)
Line 22: import java.awt.Dimension;
Line 23: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\pnm\PnmWriter.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\pnm\PpmWriter.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\psd\dataparsers\DataParser.java (2 hits)
Line 19: import java.awt.image.BufferedImage;
Line 20: import java.awt.image.DataBuffer;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\psd\datareaders\CompressedDataReader.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\psd\datareaders\DataReader.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\psd\datareaders\UncompressedDataReader.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\psd\PsdImageParser.java (2 hits)
Line 26: import java.awt.Dimension;
Line 27: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\rgbe\RgbeImageParser.java (10 hits)
Line 19: import java.awt.Dimension;
Line 20: import java.awt.Point;
Line 21: import java.awt.Transparency;
Line 22: import java.awt.color.ColorSpace;
Line 23: import java.awt.image.BandedSampleModel;
Line 24: import java.awt.image.BufferedImage;
Line 25: import java.awt.image.ComponentColorModel;
Line 26: import java.awt.image.DataBuffer;
Line 27: import java.awt.image.DataBufferFloat;
Line 28: import java.awt.image.Raster;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\tiff\datareaders\DataReaderStrips.java (1 hit)
Line 20: import java.awt.Rectangle;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\tiff\datareaders\DataReaderTiled.java (1 hit)
Line 26: import java.awt.Rectangle;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\tiff\datareaders\ImageDataReader.java (1 hit)
Line 32: import java.awt.Rectangle;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\tiff\photometricinterpreters\floatingpoint\PaletteEntry.java (1 hit)
Line 19: import java.awt.Color;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\tiff\photometricinterpreters\floatingpoint\PaletteEntryForRange.java (1 hit)
Line 19: import java.awt.Color;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\tiff\photometricinterpreters\floatingpoint\PaletteEntryForValue.java (1 hit)
Line 19: import java.awt.Color;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\tiff\photometricinterpreters\floatingpoint\PhotometricInterpreterFloat.java (1 hit)
Line 19: import java.awt.Color;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\tiff\TiffDirectory.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\tiff\TiffImageMetadata.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\tiff\TiffImageParser.java (3 hits)
Line 28: import java.awt.Dimension;
Line 29: import java.awt.Rectangle;
Line 30: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\tiff\write\TiffImageWriterBase.java (2 hits)
Line 33: import java.awt.image.BufferedImage;
Line 34: import java.awt.image.ColorModel;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\wbmp\WbmpImageParser.java (7 hits)
Line 21: import java.awt.Dimension;
Line 22: import java.awt.image.BufferedImage;
Line 23: import java.awt.image.DataBuffer;
Line 24: import java.awt.image.DataBufferByte;
Line 25: import java.awt.image.IndexColorModel;
Line 26: import java.awt.image.Raster;
Line 27: import java.awt.image.WritableRaster;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\xbm\XbmImageParser.java (8 hits)
Line 19: import java.awt.Dimension;
Line 20: import java.awt.image.BufferedImage;
Line 21: import java.awt.image.ColorModel;
Line 22: import java.awt.image.DataBuffer;
Line 23: import java.awt.image.DataBufferByte;
Line 24: import java.awt.image.IndexColorModel;
Line 25: import java.awt.image.Raster;
Line 26: import java.awt.image.WritableRaster;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\xpm\XpmImageParser.java (8 hits)
Line 18: import java.awt.Dimension;
Line 19: import java.awt.image.BufferedImage;
Line 20: import java.awt.image.ColorModel;
Line 21: import java.awt.image.DataBuffer;
Line 22: import java.awt.image.DirectColorModel;
Line 23: import java.awt.image.IndexColorModel;
Line 24: import java.awt.image.Raster;
Line 25: import java.awt.image.WritableRaster;
commons-imaging\src\main\java\org\apache\commons\imaging\icc\IccProfileParser.java (1 hit)
Line 23: import java.awt.color.ICC_Profile;
commons-imaging\src\main\java\org\apache\commons\imaging\ImageDump.java (4 hits)
Line 19: import java.awt.color.ColorSpace;
Line 20: import java.awt.color.ICC_ColorSpace;
Line 21: import java.awt.color.ICC_Profile;
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\ImageParser.java (2 hits)
Line 19: import java.awt.Dimension;
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\Imaging.java (3 hits)
Line 22: import java.awt.Dimension;
Line 23: import java.awt.color.ICC_Profile;
Line 24: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\internal\Debug.java (1 hit)
Line 19: import java.awt.color.ICC_Profile;
commons-imaging\src\main\java\org\apache\commons\imaging\palette\Dithering.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\palette\MedianCutQuantizer.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\palette\PaletteFactory.java (3 hits)
Line 19: import java.awt.color.ColorSpace;
Line 20: import java.awt.image.BufferedImage;
Line 21: import java.awt.image.ColorModel;
commons-imaging\src\test\java\org\apache\commons\imaging\common\bytesource\ByteSourceImageTest.java (2 hits)
Line 24: import java.awt.Dimension;
Line 25: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\common\bytesource\ByteSourceInputStreamTest.java (1 hit)
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\common\ImageBuilderTest.java (3 hits)
Line 19: import java.awt.image.BufferedImage;
Line 20: import java.awt.image.ColorModel;
Line 21: import java.awt.image.RasterFormatException;
commons-imaging\src\test\java\org\apache\commons\imaging\common\RgbBufferedImageFactoryTest.java (1 hit)
Line 22: import java.awt.image.DirectColorModel;
commons-imaging\src\test\java\org\apache\commons\imaging\examples\ApacheImagingSpeedAndMemoryTest.java (1 hit)
Line 156: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\examples\ImageReadExample.java (5 hits)
Line 19: import java.awt.GraphicsConfiguration;
Line 20: import java.awt.GraphicsDevice;
Line 21: import java.awt.GraphicsEnvironment;
Line 22: import java.awt.Transparency;
Line 23: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\examples\ImageWriteExample.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\examples\SampleUsage.java (3 hits)
Line 19: import java.awt.Dimension;
Line 20: import java.awt.color.ICC_Profile;
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\examples\tiff\ExampleReadFloatingPointData.java (2 hits)
Line 19: import java.awt.Color;
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\examples\tiff\ReadAndRenderFloatingPoint.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\examples\tiff\ReadTagsAndImages.java (1 hit)
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\bmp\BmpReadTest.java (1 hit)
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\bmp\BmpRoundtripTest.java (1 hit)
Line 23: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\bmp\BmpWriterRgbTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\dcx\DcxReadTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\gif\GifReadTest.java (1 hit)
Line 24: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\icns\IcnsReadTest.java (1 hit)
Line 23: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\icns\IcnsRoundTripTest.java (1 hit)
Line 23: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\ico\IcoReadTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\ico\IcoRoundtripTest.java (1 hit)
Line 23: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\jpeg\exif\MicrosoftTagTest.java (1 hit)
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\jpeg\iptc\IptcFullDiscardTest.java (2 hits)
Line 20: import java.awt.Graphics2D;
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\jpeg\JpegReadTest.java (1 hit)
Line 23: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\jpeg\JpegWithJpegThumbnailTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\jpeg\specific\JpegImageParserTest.java (1 hit)
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\pam\PamReadTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\pcx\PcxReadTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\png\ConvertPngToGifTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\png\PngMultipleRoundtripTest.java (1 hit)
Line 23: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\png\PngReadTest.java (1 hit)
Line 23: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\png\PngTextTest.java (2 hits)
Line 24: import java.awt.Color;
Line 25: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\png\PngWriteForceTrueColorText.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\png\PngWriteReadTest.java (1 hit)
Line 25: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\pnm\PnmImageParserTest.java (2 hits)
Line 23: import java.awt.image.BufferedImage;
Line 24: import java.awt.image.DataBufferInt;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\psd\PsdReadTest.java (1 hit)
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\rgbe\RgbeReadTest.java (1 hit)
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\tiff\photometricinterpreters\floatingpoint\PaletteEntryForRangeTest.java (1 hit)
Line 24: import java.awt.Color;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\tiff\photometricinterpreters\floatingpoint\PaletteEntryForValueTest.java (1 hit)
Line 24: import java.awt.Color;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\tiff\photometricinterpreters\floatingpoint\PhotometricInterpreterFloatTest.java (1 hit)
Line 23: import java.awt.Color;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\tiff\TiffAlphaRoundTripTest.java (4 hits)
Line 19: import java.awt.AlphaComposite;
Line 20: import java.awt.Color;
Line 21: import java.awt.Graphics2D;
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\tiff\TiffCcittTest.java (1 hit)
Line 23: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\tiff\TiffFloatingPointReadTest.java (2 hits)
Line 23: import java.awt.Color;
Line 24: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\tiff\TiffFloatingPointRoundTripTest.java (1 hit)
Line 23: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\tiff\TiffReadAlphaTest.java (3 hits)
Line 19: import java.awt.Color;
Line 20: import java.awt.Graphics2D;
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\tiff\TiffReadTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\tiff\TiffRoundtripTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\tiff\TiffSubImageTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\wbmp\WbmpReadTest.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\xbm\XbmReadTest.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\xmp\XmpUpdateTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\xpm\XpmReadTest.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\ImageDumpTest.java (2 hits)
Line 21: import java.awt.color.ColorSpace;
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\palette\DitheringTest.java (1 hit)
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\palette\PaletteQuantizationTest.java (1 hit)
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\roundtrip\BitmapRoundtripTest.java (1 hit)
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\roundtrip\FullColorRoundtrip.java (1 hit)
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\roundtrip\GrayscaleRountripTest.java (1 hit)
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\roundtrip\ImageAsserts.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\roundtrip\LimitedColorRoundtripTest.java (1 hit)
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\roundtrip\NullParametersRoundtripTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\roundtrip\PixelDensityRoundtrip.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\roundtrip\RoundtripBase.java (1 hit)
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\roundtrip\TestImages.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
Search "java.awt" (0 hits in 0 files of 1 searched)
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a change in pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on a change in pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#discussion_r673491489
##########
File path: src/main/java/org/apache/commons/imaging/ImageParser.java
##########
@@ -79,25 +78,18 @@
* that the documentation is perfect, especially in the more obscure
* and specialized areas of implementation.
*
- * <h3>The "Map params" argument</h3>
+ * <h3>The "params" argument</h3>
*
- * Many of the methods specified by this class accept an argument of
- * type Map giving a list of parameters to be used when processing an
- * image. For example, some of the output formats permit the specification
+ * <p>Many of the methods specified by this class accept an argument of
+ * type {@code T} defining the parameters to be used when
+ * processing an image. For example, some of the output formats permit
* of different kinds of image compression or color models. Some of the
* reading methods permit the calling application to require strict
- * format compliance. In many cases, however, an application will not
- * require the use of this argument. While some of the ImageParser
- * implementations check for (and ignore) null arguments for this parameter,
- * not all of them do (at least not at the time these notes were written).
- * Therefore, a prudent programmer will always supply an valid, though
- * empty instance of a Map implementation when calling such methods.
- * Generally, the java HashMap class is useful for this purpose.
+ * format compliance.</p>
*
- * <p>Additionally, developers creating or enhancing classes derived
- * from ImageParser are encouraged to include such checks in their code.
+ * @param <T> type of parameters used by this image parser
*/
-public abstract class ImageParser extends BinaryFileParser {
+public abstract class ImageParser<T extends ImagingParameters> extends BinaryFileParser {
Review comment:
Hi @gwlucastrig
An approach similar to what we are trying here, thanks for sharing!
>This would compile just fine. However, betaParameters would contain information that was irrelevant to the alpha parser. It would probably be a case where the programmer made a mistake. The program would probably run, but it might not do quite what the developer hoped. Some debugging would be necessary. How could we help this case?
I think we have reached the same point here. While both our approaches lead to no generics, the cost is accepting compilation and having to do some runtime checking, though not perfect.
>Maybe forget the whole bit about the copy constructor for AlphaParameters. I think most classes would just do the branching approach and only the guys who needed to do something special would use the copy-constructor approach.
My current experiment is not going anywhere, so I'll `git reset --hard` and start testing yours with the copy constructor.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a change in pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on a change in pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#discussion_r670141981
##########
File path: src/main/java/org/apache/commons/imaging/Imaging.java
##########
@@ -735,32 +737,35 @@ public static ImageInfo getImageInfo(final File file) throws ImageReadException,
return getImageInfo(file, null);
}
- private static ImageInfo getImageInfo(final ByteSource byteSource, final Map<String, Object> params)
- throws ImageReadException, IOException {
- return getImageParser(byteSource).getImageInfo(byteSource, params);
+ // See getImageParser
+ @SuppressWarnings("unchecked")
+ private static ImageInfo getImageInfo(final ByteSource byteSource, final ImagingParameters params) throws ImageReadException, IOException {
+ return Imaging.getImageParser(byteSource).getImageInfo(byteSource, params);
}
- private static ImageParser getImageParser(final ByteSource byteSource)
- throws ImageReadException, IOException {
+ // TODO: We have no way of knowing whether the returned ImageParser will accept the ImagingParameters,
+ // even if we specified generic types for the static methods.
Review comment:
Not sure if there's a better way of changing this 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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-961461586
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] darkma773r commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
darkma773r commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-985507050
Cool! I'll try to take a look at this over the weekend.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-1008428517
Not a problem @darkma773r , I'll work on other things, including trying to look at that fuzzy-search issue GEOMETRY-142. 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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a change in pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on a change in pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#discussion_r785353218
##########
File path: src/main/java/org/apache/commons/imaging/internal/Util.java
##########
@@ -0,0 +1,73 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.imaging.internal;
+
+import org.apache.commons.imaging.ImageFormat;
+import org.apache.commons.imaging.ImageFormats;
+import org.apache.commons.imaging.ImageParser;
+import org.apache.commons.imaging.Imaging;
+import org.apache.commons.imaging.ImagingParameters;
+import org.apache.commons.imaging.common.bytesource.ByteSource;
+
+import java.io.IOException;
+import java.util.function.Predicate;
+import java.util.function.Supplier;
+
+/**
+ * Internal utilities.
+ *
+ * @since 1.0-alpha3
+ */
+public class Util {
+
+ private Util() {}
+
+ public static <T extends ImagingParameters> ImageParser<T> getImageParser(ImageFormat format) {
+ return getImageParser((parser) -> parser.canAcceptType(format), () -> new RuntimeException("Unknown Format: " + format));
+ }
+
+ public static <T extends ImagingParameters> ImageParser<T> getImageParser(String fileExtension) {
+ return getImageParser((parser) -> parser.canAcceptExtension(fileExtension), () -> new RuntimeException("Unknown Extension: " + fileExtension));
Review comment:
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a change in pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on a change in pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#discussion_r785353011
##########
File path: src/main/java/org/apache/commons/imaging/ImageParser.java
##########
@@ -955,18 +944,15 @@ protected BufferedImageFactory getBufferedImageFactory(final Map<String, Object>
/**
* A utility method to search a params specification and determine
- * whether it contains the ImagingConstants.PARAM_KEY_STRICT
- * specification. Intended
+ * whether it contains the parameters contain the strict flag. Intended
* for internal use by ImageParser implementations.
*
- * @param params A valid Map object (or a null).
+ * @param params optional parameters.
* @return If the params specify strict format compliance, true;
* otherwise, false.
+ * @param <T> type of parameters used by this image parser
*/
- public static boolean isStrict(final Map<String, Object> params) {
- if (params == null || !params.containsKey(ImagingConstants.PARAM_KEY_STRICT)) {
- return false;
- }
- return ((Boolean) params.get(ImagingConstants.PARAM_KEY_STRICT)).booleanValue();
+ public static <T extends ImagingParameters> boolean isStrict(final T params) {
+ return params.isStrict();
Review comment:
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a change in pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on a change in pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#discussion_r777299737
##########
File path: src/main/java/org/apache/commons/imaging/formats/jpeg/JpegImagingParameters.java
##########
@@ -0,0 +1,24 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ * under the License.
+ */
+
+package org.apache.commons.imaging.formats.jpeg;
+
+import org.apache.commons.imaging.formats.tiff.TiffImagingParameters;
+
+/**
+ * Jpeg format parameters.
+ * @since 1.0-alpha3
+ */
+public class JpegImagingParameters extends TiffImagingParameters {}
Review comment:
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] darkma773r commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
darkma773r commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-955197158
I just made a merge request on this branch in your repo, @kinow: https://github.com/kinow/commons-imaging/pull/36. Let me know what you think.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-955198434
Thanks a lot @darkma773r !! Will take a look on 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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-956081611
@darkma773r
Thanks a lot for creating that pull request. Really clever the intermediate `GenericImageParser`. I liked it and might use it later! :+1:
A few questions/notes, feel free to point if any of what I say is off the mark.
1. If the `JpegImageParser` requires a parameter different than the `TiffImageParser`, I assume we would have to create a new `JpegImagingParameter`. In that case, wouldn't we have to keep the old constructor for backward compatibility until a new major release?
2. I think it would be best if cases like this were not allowed `PcxImagingParameters parser = new PcxImagingParameters(new TiffImagingParameters()); // no error in compile or runtime`
3. I think you removed the option to allow `null` parameters in the `Imaging#writeImage` method (and other methods). I thought about doing that too, that would remove one generic suppress I think, but there are unit tests and examples. The API has allowed it since Sanselan, so not too sure about enforcing parameters to be non-null now.
4. I liked the error in this one `new PngImageParser().writeImage(null, null, new TiffImagingParameters());`, which gives `java.lang.IllegalArgumentException: Invalid imaging parameters: expected type...`
Your design really solves the generics problems, so I will take another look later and see if I find a middle way. See what you think about the `null` parameters, and allowing the empty parameters types for `Jpeg` and other parsers, or about the other notes @darkma773r :+1:
Thanks heaps!
```java
Imaging.writeImage((BufferedImage) null, (OutputStream) null, ImageFormats.GIF, new TiffImagingParameters());
// java.lang.IllegalArgumentException: Invalid imaging parameters: expected type org.apache.commons.imaging.common.XmpImagingParameters or a super class but was org.apache.commons.imaging.formats.tiff.TiffImagingParameters :+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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-958618064
>> If the JpegImageParser requires a parameter different than the TiffImageParser, I assume we would have to create a new JpegImagingParameter. In that case, wouldn't we have to keep the old constructor for backward compatibility until a new major release?
>
>I'm not totally sure what you mean here. Can you give an example?
Sorry, I wasn't very clear. At the moment we have `JpegImageParser parser = new JpegImageParser();`. Internally, the `JpegImageParser` is using a `TiffImagingParameters`.
But if we need to add a parameter that is used only by the `JpegImageParser`, I think we would want to have a `JpegImagingParameters`. In which case I am not sure if we would be able to maintain backward compatibility by replacing the `TiffImagingParameters` in the `JpegImageParser`.
>since it makes it it kind of hard to tell what parameters to use when creating images of a certain type.
:point_up: this is my main concern. Avoid users having to guess what's the parameter that they want to use when parsing a Jpeg, Tiff, Gif, etc. Preferably something that their IDE's can assist with, using auto-complete for example.
>One idea for making this readily available in the API would be to add convenience factory methods in Imaging for creating instances of each parameters type. Ex:
Possibly another design to consider/experiment :-)
>Side note: Is there a reason the JpegImageParser uses TiffImageParameters? The two formats are not related, correct? Perhaps we could have a common base class for them?
We can probably avoid that. I agree it doesn't make much sense for the parameters to have a relationship like that. Currently, [`JpegImageParser`](https://github.com/apache/commons-imaging/blob/0ccabc3416cecf25cdec90a522f0912baf19bc58/src/main/java/org/apache/commons/imaging/formats/jpeg/JpegImageParser.java#L751) has some code in common with the `TiffImageParser` for parsing EXIF metadata. I don't recall 100%, but I believe that's why I implemented everything in the `TiffImagingParameters` and, realizing the Jpeg parameters were the same, made the parent-child class relantionship.
>Is the idea that it is confusing on what properties are copied over?
Not at all, that part is clear and looks OK. It's with users being able to use the wrong parameter class by accident, and realizing only in runtime iff a runtime exception is thrown (or in the worst case, no exception is thrown and the program behaves in a wrong way; debugging an issue like that can be annoying I think).
>I attempted to retain the null parameter functionality. In the normalizeParameters method, if a null parameter is passed, a default instance is created and returned, allowing downstream code to bypass null checks. mvn clean install passes with my current setup. Is there a unit test I missed?
Ah, my bad! Your solution works with the method I was looking at. I wrote this one before looking at the method that creates the default parameters :+1:
I need to find some time to “get in the zone” and try a few things. But it would be, mainly, to prevent users being able to use the wrong parameter types by accident, and making it so that we can add/remove parameters (deprecating what was removed) in the 1.x release series without worrying about backward compatibility. If we are able to modify your PR again to accommodate these two requirements, I think we would have the perfect solution :-)
Thanks a lot!
Bruno
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-961461586
>I just had another thought on this: it strikes me that the main issue with the generic ImagerParser<P extends ImagingParameters> format is how to deal with format-agnostic code, mainly the utility methods in Imaging.
Exactly. Were it not for the `Imaging` public methods, we could use the generics or any other design without much impact to users. But the current code base uses `Imaging` as the main entry point for users in the API (unless users want something more complicated, in which case they can use parsers directly.)
>In other words, how do users specify parameters when they don't even know what the format is? The current approach is to have users pass in their own parameters objects to these utility methods but this gets into the problem of what happens when they pass the wrong parameters type.
:+1: you described the problem really well!
>What if we flipped this? Instead of having the users create the parameters object, we have the parser create one and then have the user modify it as needed in a callback. So, instead of this
Ah, it sounds like a good thing to try! I think this approach would apply some inversion of control, or algebraic effects, and could help us solve the current impasse.
>Thoughts?
I think it would need a few more changes to allow users to configure the correct parameter type? The `Consumer<ImagingParameters>` uses the base type, so in this closure the user has only an instance of `ImagingParameters`, and would have to cast to the correct type.
I used the last method in `Imaging.java` instead of `getIccProfileBytes` (for convenience, to have the `main` method nearby in the IDE :grimacing: ):
```java
public static void writeImage(final BufferedImage src, final OutputStream os,
final ImageFormat format, final Consumer<ImagingParameters> configurer) throws ImageWriteException,
IOException {
final List<ImageParser> imageParsers = ImageParser.getAllImageParsers();
ImageParser imageParser = null;
for (final ImageParser imageParser2 : imageParsers) {
if (imageParser2.canAcceptType(format)) {
imageParser = imageParser2;
break;
}
}
if (imageParser == null) {
throw new ImageWriteException("Unknown Format: " + format);
}
final ImagingParameters params = imageParser.getDefaultParameters();
configurer.accept(params);
imageParser.writeImage(src, os, params);
}
```
But the caller code still has issues with the parameter type, I think?
```java
public static void main(String[] args) throws Exception {
Imaging.writeImage(
(BufferedImage) null,
(OutputStream) null,
ImageFormats.TIFF,
(ImagingParameters p) -> { // if `TiffImagingParameters`: Incompatible parameter types in lambda expression: expected ImagingParameters but found TiffImagingParameters
// p is still ImagingParameters, not TiffImagingParameters
p.setFileName("bla.tif");
});
}
```
Or would you re-write `writeImage` differently, @darkma773r ?
Thanks!
Bruno
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-961799086
@darkma773r I've been thinking the types we use in that class, like `ImageParser`, `ImagingParameter`, and `ImageFormat`. When we pass the arguments to `writeImage`, for example, it includes a `ImageFormat`, and also an `ImagingParameters`.
However, when you pass an `ImageFormats.GIF` it's obvious the parameter will be of type `GifImagingParameters`. It is also obvious the parser will be the `GifImageParser`.
I'm thinking that maybe we could use something like `ImagingParameters<Gif>` and `ImageParser<Gif>`, eliminating the `ImageFormat`. I think it would be possible to have a map of format=>parser, and a method that retrieves the parser with the right format. Sorry if it's vague, I will think more about it this weekend and see if I can suggest some code to compare with what we have 👍
Bruno
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a change in pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on a change in pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#discussion_r780552113
##########
File path: src/changes/changes.xml
##########
@@ -45,6 +45,21 @@ The <action> type attribute can be add,update,fix,remove.
</properties>
<body>
<release version="1.0-alpha3" date="2020-??-??" description="Third 1.0 alpha release">
+ <action issue="IMAGING-159" dev="kinow" type="fix" due-to="Bruno P. Kinoshita, Gary Lucas, Matt Juntunen">
Review comment:
Thanks for the help!!! :point_up: :bowing_man:
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] darkma773r commented on a change in pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
darkma773r commented on a change in pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#discussion_r762471491
##########
File path: src/main/java/org/apache/commons/imaging/ImageFormats.java
##########
@@ -16,40 +16,68 @@
*/
package org.apache.commons.imaging;
+import org.apache.commons.imaging.formats.bmp.BmpImagingParameters;
+import org.apache.commons.imaging.formats.gif.GifImagingParameters;
+import org.apache.commons.imaging.formats.icns.IcnsImagingParameters;
+import org.apache.commons.imaging.formats.ico.IcoImagingParameters;
+import org.apache.commons.imaging.formats.jpeg.JpegImagingParameters;
+import org.apache.commons.imaging.formats.pcx.PcxImagingParameters;
+import org.apache.commons.imaging.formats.png.PngImagingParameters;
+import org.apache.commons.imaging.formats.pnm.PnmImagingParameters;
+import org.apache.commons.imaging.formats.psd.PsdImagingParameters;
+import org.apache.commons.imaging.formats.rgbe.RgbeImagingParameters;
+import org.apache.commons.imaging.formats.tiff.TiffImagingParameters;
+import org.apache.commons.imaging.formats.wbmp.WbmpImagingParameters;
+import org.apache.commons.imaging.formats.xbm.XbmImagingParameters;
+import org.apache.commons.imaging.formats.xpm.XpmImagingParameters;
+
+import java.util.function.Supplier;
+
/**
* Enum of known image formats.
*/
public enum ImageFormats implements ImageFormat {
- UNKNOWN,
- BMP,
- DCX,
- GIF,
- ICNS,
- ICO,
- JBIG2,
- JPEG,
- PAM,
- PSD,
- PBM,
- PGM,
- PNM,
- PPM,
- PCX,
- PNG,
- RGBE,
- TGA,
- TIFF,
- WBMP,
- XBM,
- XPM;
+ UNKNOWN(null),
+ BMP(BmpImagingParameters::new, "bmp", "dib"),
+ DCX(PcxImagingParameters::new, "dcx"),
+ GIF(GifImagingParameters::new, "gif"),
+ ICNS(IcnsImagingParameters::new, "icns"),
+ ICO(IcoImagingParameters::new, "ico"),
+ JBIG2(null),
+ JPEG(JpegImagingParameters::new, "jpg", "jpeg"),
+ PAM(PnmImagingParameters::new, "pam"),
+ PSD(PsdImagingParameters::new, "psd"),
+ PBM(PnmImagingParameters::new, "pbm"),
+ PGM(PnmImagingParameters::new, "pgm"),
+ PNM(PnmImagingParameters::new, "pnm"),
+ PPM(PnmImagingParameters::new, "ppm"),
+ PCX(PcxImagingParameters::new, "pcx", "pcc"),
+ PNG(PngImagingParameters::new, "png"),
+ RGBE(RgbeImagingParameters::new, "rgbe"),
+ TGA(null),
+ TIFF(TiffImagingParameters::new, "tif", "tiff"),
+ WBMP(WbmpImagingParameters::new, "wbmp"),
+ XBM(XbmImagingParameters::new, "xbm"),
+ XPM(XpmImagingParameters::new, "xpm");
+
+ private final String[] extensions;
+
+ ImageFormats(Supplier<? extends ImagingParameters> factory, String ...extensions) {
+ this.extensions = extensions;
+ }
@Override
public String getName() {
return name();
}
@Override
- public String getExtension() {
- return name();
+ public String[] getExtensions() {
+ return this.extensions.clone();
+ }
+
+ @Override
+ public String getDefaultExtension() {
+ return this.extensions[0];
Review comment:
`ImageFormats.UNKNOWN.getDefaultExtension()` throws index out of bounds.
##########
File path: src/main/java/org/apache/commons/imaging/Imaging.java
##########
@@ -16,9 +16,6 @@
*/
package org.apache.commons.imaging;
-import static org.apache.commons.imaging.ImagingConstants.PARAM_KEY_FILENAME;
-import static org.apache.commons.imaging.ImagingConstants.PARAM_KEY_FORMAT;
Review comment:
Woo hoo!
##########
File path: src/main/java/org/apache/commons/imaging/ImageFormat.java
##########
@@ -33,6 +34,7 @@
*
* @return String extension
*/
- String getExtension();
+ String[] getExtensions();
Review comment:
Use `List<String>` instead of `String[]`? I generally find lists easier to work with, especially for streams.
##########
File path: src/main/java/org/apache/commons/imaging/ImageFormats.java
##########
@@ -16,40 +16,68 @@
*/
package org.apache.commons.imaging;
+import org.apache.commons.imaging.formats.bmp.BmpImagingParameters;
+import org.apache.commons.imaging.formats.gif.GifImagingParameters;
+import org.apache.commons.imaging.formats.icns.IcnsImagingParameters;
+import org.apache.commons.imaging.formats.ico.IcoImagingParameters;
+import org.apache.commons.imaging.formats.jpeg.JpegImagingParameters;
+import org.apache.commons.imaging.formats.pcx.PcxImagingParameters;
+import org.apache.commons.imaging.formats.png.PngImagingParameters;
+import org.apache.commons.imaging.formats.pnm.PnmImagingParameters;
+import org.apache.commons.imaging.formats.psd.PsdImagingParameters;
+import org.apache.commons.imaging.formats.rgbe.RgbeImagingParameters;
+import org.apache.commons.imaging.formats.tiff.TiffImagingParameters;
+import org.apache.commons.imaging.formats.wbmp.WbmpImagingParameters;
+import org.apache.commons.imaging.formats.xbm.XbmImagingParameters;
+import org.apache.commons.imaging.formats.xpm.XpmImagingParameters;
+
+import java.util.function.Supplier;
+
/**
* Enum of known image formats.
*/
public enum ImageFormats implements ImageFormat {
- UNKNOWN,
- BMP,
- DCX,
- GIF,
- ICNS,
- ICO,
- JBIG2,
- JPEG,
- PAM,
- PSD,
- PBM,
- PGM,
- PNM,
- PPM,
- PCX,
- PNG,
- RGBE,
- TGA,
- TIFF,
- WBMP,
- XBM,
- XPM;
+ UNKNOWN(null),
+ BMP(BmpImagingParameters::new, "bmp", "dib"),
Review comment:
These constructor references don't seem to be used anymore. Remove?
##########
File path: src/main/java/org/apache/commons/imaging/Imaging.java
##########
@@ -857,20 +837,19 @@ public static Dimension getImageSize(final File file) throws ImageReadException,
*
* @param file
* File containing image data.
- * @param params
- * Map of optional parameters, defined in ImagingConstants.
+ * @param params optional parameters.
* @return The width and height of the image.
* @throws ImageReadException if it fails to parse the image
* @throws IOException if it fails to read the image data
*/
- public static Dimension getImageSize(final File file, final Map<String, Object> params)
+ public static Dimension getImageSize(final File file, final ImagingParameters params)
Review comment:
Not sure about this API. Callers need to know the type of the image being read in order to avoid a `ClassCastException`. Possible options:
1. Make it possible to "upgrade" the parameters object internally and copy the values into an instance of the correct type.
2. Remove this method (and others that accept `ImagingParameters`) altogether and only allow parameters to be passed using the methods on the parser. We could make the parsers more easily accessible by making the `getImageParser` method public.
##########
File path: src/main/java/org/apache/commons/imaging/formats/jpeg/JpegImagingParameters.java
##########
@@ -0,0 +1,24 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ * under the License.
+ */
+
+package org.apache.commons.imaging.formats.jpeg;
+
+import org.apache.commons.imaging.formats.tiff.TiffImagingParameters;
+
+/**
+ * Jpeg format parameters.
+ * @since 1.0-alpha3
+ */
+public class JpegImagingParameters extends TiffImagingParameters {}
Review comment:
Do all the properties on `TiffImagingParameters` apply to JPEGs? (ex, `setExif(TiffOutputSet exif)`)
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] coveralls edited a comment on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-841637226
[](https://coveralls.io/builds/42656656)
Coverage decreased (-0.2%) to 76.512% when pulling **f136ca85bf930652f7677646548284f871dd7be8 on kinow:IMAGING-159-1** into **32e1a3b839eb75b38c47531cb2dd6e0012bb3bf5 on apache:master**.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a change in pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on a change in pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#discussion_r679442463
##########
File path: src/main/java/org/apache/commons/imaging/ImageParser.java
##########
@@ -79,25 +78,18 @@
* that the documentation is perfect, especially in the more obscure
* and specialized areas of implementation.
*
- * <h3>The "Map params" argument</h3>
+ * <h3>The "params" argument</h3>
*
- * Many of the methods specified by this class accept an argument of
- * type Map giving a list of parameters to be used when processing an
- * image. For example, some of the output formats permit the specification
+ * <p>Many of the methods specified by this class accept an argument of
+ * type {@code T} defining the parameters to be used when
+ * processing an image. For example, some of the output formats permit
* of different kinds of image compression or color models. Some of the
* reading methods permit the calling application to require strict
- * format compliance. In many cases, however, an application will not
- * require the use of this argument. While some of the ImageParser
- * implementations check for (and ignore) null arguments for this parameter,
- * not all of them do (at least not at the time these notes were written).
- * Therefore, a prudent programmer will always supply an valid, though
- * empty instance of a Map implementation when calling such methods.
- * Generally, the java HashMap class is useful for this purpose.
+ * format compliance.</p>
*
- * <p>Additionally, developers creating or enhancing classes derived
- * from ImageParser are encouraged to include such checks in their code.
+ * @param <T> type of parameters used by this image parser
*/
-public abstract class ImageParser extends BinaryFileParser {
+public abstract class ImageParser<T extends ImagingParameters> extends BinaryFileParser {
Review comment:
Good point @gwlucastrig . I will have a look to remember why I extended the tiff class.
As for the generics, maybe we can find a compromise between our two options, even if it requires further changes (e.g. rethink the Imaging class for example). Or maybe after reviewing this jpeg - tiff parent relationship we may simplify this code more and have some other ideas.
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow edited a comment on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow edited a comment on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-841649340
If those classes are from awt, the issue is that we have a dependency to java.desktop (I think that's the java module name) and also cause an issue to users of the API that are using Android (as IIUC, there's no awt in Android's Java SDK?).
Let's see if we have any alternatives. If not, then we might as well just use it (we are using it elsewhere), and fix this issue 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] coveralls edited a comment on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-841637226
[](https://coveralls.io/builds/41452544)
Coverage decreased (-0.2%) to 76.565% when pulling **4b7f97519ea2712d89100ad0e28138f1a1e94acc on kinow:IMAGING-159-1** into **a87b545e1019b43bfbe31fa5df3a4f3095a8b438 on apache:master**.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-783729644
> Hey Bruno,
>
> Anything I can do to help with this?
>
> Gary
I thought it would be a simple task, but replacing the hashmaps by the new object would be quick. I have a few more files o update. Once the build is green here, no more errors, then I would like you to take a look and see how if it looks OK, and if we can do one more iteration to get the TIFF parameters you mentioned the other day, please :grimacing: 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] coveralls edited a comment on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-841637226
[](https://coveralls.io/builds/41387442)
Coverage decreased (-0.2%) to 76.575% when pulling **d3ffa1e93188948d4b9f721318a382a7819475cf on kinow:IMAGING-159-1** into **a87b545e1019b43bfbe31fa5df3a4f3095a8b438 on apache:master**.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-841649340
If those classes are from awt, the issue is that we have a dependency to java.desktop (I think that's the java module name) and also cause an issue to users of the API that are using Android (as IIUC, there's no awt in Android's Java SDK?).
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-826703877
Note to self: rebase again, and check Tabs vs. Spaces.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] darkma773r commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
darkma773r commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-958597172
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] gwlucastrig commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
gwlucastrig commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-961784862
My perspective is limited because I am so focused on TIFFs. And I seldom use the basic Imaging class (preferring the TIFF-specific parser and write classes). So I am sure I am unaware of a number of valid use cases.
That being said, I might think that simpler is better. The Imaging class API could be cut back to just include only limited support for parameters. The read-related operations might not even take parameters arguments. The write-related would only take a very basic parameters. In fact, except for a "dots per inch" or "mm per pixel" setting, I can't think of anything that would be supported across the majority of write operations.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow edited a comment on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow edited a comment on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-961796289
That is a good point @gwlucastrig , and I have no objections to massaging the code of the `Imaging` class. I will spend some time thinking about that, and seeing if/when the parameters would be useful or not. Thanks!!! (and you have a perspective of user + dev, which is really valuable @gwlucastrig !)
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-950136340
Triaged issues today, and I think this is the last issue pending for 1.0 release. The others can still be fixed in 1.0.1, or 1.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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] gwlucastrig commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
gwlucastrig commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-783770464
I'll be glad to do the TIFF integration. Just let me know when you're ready.
É isso aí
Gary
On Mon, Feb 22, 2021 at 5:43 PM Bruno P. Kinoshita <no...@github.com>
wrote:
> Hey Bruno,
>
> Anything I can do to help with this?
>
> Gary
>
> I thought it would be a simple task, but replacing the hashmaps by the new
> object would be quick. I have a few more files o update. Once the build is
> green here, no more errors, then I would like you to take a look and see
> how if it looks OK, and if we can do one more iteration to get the TIFF
> parameters you mentioned the other day, please 😬 Thanks!
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/commons-imaging/pull/116#issuecomment-783729644>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AEWJDYPHHKHLK62VTFXYMSDTALMZZANCNFSM4WPUJSLQ>
> .
>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow merged pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow merged pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a change in pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on a change in pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#discussion_r777276289
##########
File path: src/main/java/org/apache/commons/imaging/formats/jpeg/JpegImagingParameters.java
##########
@@ -0,0 +1,24 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ * under the License.
+ */
+
+package org.apache.commons.imaging.formats.jpeg;
+
+import org.apache.commons.imaging.formats.tiff.TiffImagingParameters;
+
+/**
+ * Jpeg format parameters.
+ * @since 1.0-alpha3
+ */
+public class JpegImagingParameters extends TiffImagingParameters {}
Review comment:
At the moment, due to how the classes are structured, I believe they do. But I've been thinking about it since you first noticed that it was strange the TIFF & JPEG parsers' relationship. Let me see how the code would look like separating the two (with either an intermediary class playing the role of a mixin, or simply duplicating the 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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-881833791
Excellent review @darkma773r! I've incorporated one of your suggestions already, but will need a little more time to go over the other items. However, having read your suggestions, I think they will fix other issues I had with the current implementation.
Thanks heaps! :champagne: :1st_place_medal:
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] darkma773r commented on a change in pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
darkma773r commented on a change in pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#discussion_r671470893
##########
File path: src/main/java/org/apache/commons/imaging/ImageFormats.java
##########
@@ -16,40 +16,92 @@
*/
package org.apache.commons.imaging;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+
+import org.apache.commons.imaging.formats.bmp.BmpImagingParameters;
+import org.apache.commons.imaging.formats.gif.GifImagingParameters;
+import org.apache.commons.imaging.formats.icns.IcnsImagingParameters;
+import org.apache.commons.imaging.formats.ico.IcoImagingParameters;
+import org.apache.commons.imaging.formats.jpeg.JpegImagingParameters;
+import org.apache.commons.imaging.formats.pcx.PcxImagingParameters;
+import org.apache.commons.imaging.formats.png.PngImagingParameters;
+import org.apache.commons.imaging.formats.pnm.PnmImagingParameters;
+import org.apache.commons.imaging.formats.psd.PsdImagingParameters;
+import org.apache.commons.imaging.formats.rgbe.RgbeImagingParameters;
+import org.apache.commons.imaging.formats.tiff.TiffImagingParameters;
+import org.apache.commons.imaging.formats.wbmp.WbmpImagingParameters;
+import org.apache.commons.imaging.formats.xbm.XbmImagingParameters;
+import org.apache.commons.imaging.formats.xpm.XpmImagingParameters;
+
/**
* Enum of known image formats.
*/
public enum ImageFormats implements ImageFormat {
- UNKNOWN,
- BMP,
- DCX,
- GIF,
- ICNS,
- ICO,
- JBIG2,
- JPEG,
- PAM,
- PSD,
- PBM,
- PGM,
- PNM,
- PPM,
- PCX,
- PNG,
- RGBE,
- TGA,
- TIFF,
- WBMP,
- XBM,
- XPM;
+ UNKNOWN(ImagingParameters.class),
+ BMP(BmpImagingParameters.class, "bmp", "dib"),
+ DCX(PcxImagingParameters.class, "dcx"),
+ GIF(GifImagingParameters.class, "gif"),
+ ICNS(IcnsImagingParameters.class, "icns"),
+ ICO(IcoImagingParameters.class, "ico"),
+ JBIG2(ImagingParameters.class),
+ JPEG(JpegImagingParameters.class, "jpg", "jpeg"),
+ PAM(PnmImagingParameters.class, "pam"),
+ PSD(PsdImagingParameters.class, "psd"),
+ PBM(PnmImagingParameters.class, "pbm"),
+ PGM(PnmImagingParameters.class, "pgm"),
+ PNM(PnmImagingParameters.class, "pnm"),
+ PPM(PnmImagingParameters.class, "ppm"),
+ PCX(PcxImagingParameters.class, "pcx", "pcc"),
+ PNG(PngImagingParameters.class, "png"),
+ RGBE(RgbeImagingParameters.class, "rgbe"),
+ TGA(ImagingParameters.class),
+ TIFF(TiffImagingParameters.class, "tif", "tiff"),
+ WBMP(WbmpImagingParameters.class, "wbmp"),
+ XBM(XbmImagingParameters.class, "xbm"),
+ XPM(XpmImagingParameters.class, "xpm");
+
+ private static final Logger LOGGER = Logger.getLogger(ImageFormats.class.getName());
+
+ private String[] extensions;
+ private Class<? extends ImagingParameters> parametersClass;
+
+ ImageFormats(Class<? extends ImagingParameters> parametersClass, String ...extensions) {
+ this.extensions = extensions;
+ this.parametersClass = parametersClass;
+ }
@Override
public String getName() {
return name();
}
@Override
- public String getExtension() {
- return name();
+ public String[] getExtensions() {
+ return this.extensions.clone();
+ }
+
+ @Override
+ public String getDefaultExtension() {
+ return this.extensions[0];
+ }
+
+ @Override
+ public ImagingParameters createImagingParameters() {
+ ImagingParameters parameters = null;
+ if (this.parametersClass != null) {
+ Constructor<?> ctor;
+ try {
+ ctor = this.parametersClass.getConstructor();
+ parameters = (ImagingParameters) ctor.newInstance();
+ } catch (NoSuchMethodException | SecurityException | InstantiationException | IllegalAccessException | IllegalArgumentException | InvocationTargetException e) {
+ LOGGER.log(Level.WARNING, "Failed to create imaging parameters: " + e.getMessage(), e);
+ parameters = new ImagingParameters();
+ }
+ parameters.setImageFormat(this);
+ }
+ return parameters;
Review comment:
Maybe use factory functions instead of reflection here. Ex:
```java
enum ImageFormat {
PNG(PngImagingParameter::new);
private final Supplier<? extends ImagingParameters> factory;
// ....
public ImagingParameters createImagingParameters() {
return factory.get();
}
}
```
##########
File path: src/main/java/org/apache/commons/imaging/ImagingParameters.java
##########
@@ -0,0 +1,108 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.imaging;
+
+import org.apache.commons.imaging.common.BufferedImageFactory;
+
+/**
+ * Imaging parameters.
+ *
+ * <p>Contains parameters that are common to all formats. Implementations must include
+ * the specific parameters for each image format.</p>
+ *
+ * @since 1.0-alpha3
+ */
+public class ImagingParameters {
+
+ /**
+ * Whether to throw an exception when any issue occurs during reading
+ * or writing a file format. Default is {@code false}.
+ */
+ private boolean strict = false;
+
+ /**
+ * An optional file name, used for the description of input streams
+ * where a file name would be hard (or not possible) to be identified.
+ * Default is {@code null}.
+ */
+ private String fileName = null;
+
+ /**
+ * Factory to create {@code BufferedImage}s. Default is {@code null}.
+ */
+ private BufferedImageFactory bufferedImageFactory = null;
+
+ /**
+ * Image format used in write operations to indicate desired image format.
+ * Default is {@code null}.
+ *
+ * <p>Valid values: Any format defined in ImageFormat, such as
+ * ImageFormat.IMAGE_FORMAT_PNG.</p>
+ *
+ * @see org.apache.commons.imaging.ImageFormats
+ */
+ private ImageFormat imageFormat;
Review comment:
Should this be final? I'm wondering since otherwise it would be possible to create an instance with a class name that doesn't match the image format type. Ex:
```java
PngImagingParameters params = new PngImagingParameters();
params.setImageFormat(ImageFormat.BMP); // doesn't seem like a good thing
```
Perhaps something like this would work?
```java
public abstract class ImagingParameters {
private final ImageFormat imageFormat;
protected ImagingParameters(final ImageFormat imageFormat) {
this.imageFormat = imageFormat;
}
}
public class PngImageParameters {
public PngImageParameters() {
super(ImageFormat.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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a change in pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on a change in pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#discussion_r671623444
##########
File path: src/main/java/org/apache/commons/imaging/ImageParser.java
##########
@@ -106,7 +98,7 @@
*
* @return A valid array of image parsers
*/
- public static ImageParser[] getAllImageParsers() {
+ public static ImageParser<?>[] getAllImageParsers() {
Review comment:
Good idea, 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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] darkma773r commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
darkma773r commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-958597172
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] darkma773r commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
darkma773r commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-959156762
I just had another thought on this: it strikes me that the main issue with the generic `ImagerParser<P extends ImagingParameters>` format is how to deal with format-agnostic code, mainly the utility methods in `Imaging`. In other words, how do users specify parameters when they don't even know what the format is? The current approach is to have users pass in their own parameters objects to these utility methods but this gets into the problem of what happens when they pass the wrong parameters type. What if we flipped this? Instead of having the users create the parameters object, we have the parser create one and then have the user modify it as needed in a callback. So, instead of this
```
public static byte[] getICCProfileBytes(final File file, final ImagingParameters params) {
// hopefully params is of the correct type!
}
```
we do this
```
public static byte[] getICCProfileBytes(final File file, final Consumer<ImagingParameters> configurer) {
ImageParser<?> parser = ...; // get the parser
return getICCProfileBytes(parser, configurer);
}
private static <P extends ImagingParameters> byte[] getICCProfileBytes(final ImageParser<P> parser, final Consumer<ImagingParameters> configurer) {
P params = parser.getDefaultParameters();
if (configurer != null) {
// let the caller configure the parameters
configurer.accept(params);
}
// params is known to be the correct type
return parser.getICCProfileBytes(params);
}
```
Users would then call it like this:
```
Imaging.getICCProfileBytes(file, p -> p.setStrict(true));
```
We could also add methods to `ImageParser` that accept consumers like this for consistency. That would probably be best.
Thoughts?
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-950461037
Hi @gwlucastrig we just need to sort out if the API needs to be redesigned due to the generics suppressions or if that's something we can consider a 1.1 or 2.0 improvement.
I'm inclined to just go with this design - if no other options are presented with consensus - and then rethink later in a 2.0 version if needed.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] darkma773r commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
darkma773r commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-953743996
Hello again. Apologies for my silence here. I've been meaning to revisit this but just haven't gotten to it. I'll try to spend some time with this over the next several days and see if I can come up with something. I feel like we are very close.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] gwlucastrig commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
gwlucastrig commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-950406616
For what it's worth, I think that your proposed changes will be a
significant improvement to the API design.
Let me know if you need help.
On Sat, Oct 23, 2021 at 7:09 AM Bruno P. Kinoshita ***@***.***>
wrote:
> Triaged issues today, and I think this is the last issue pending for 1.0
> release. The others can still be fixed in 1.0.1, or 1.1.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/commons-imaging/pull/116#issuecomment-950136340>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AEWJDYJFABFIYWWPL4O4MYLUIKJYDANCNFSM4WPUJSLQ>
> .
> Triage notifications on the go with GitHub Mobile for iOS
> <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
> or Android
> <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
>
>
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a change in pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on a change in pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#discussion_r777276089
##########
File path: src/main/java/org/apache/commons/imaging/ImageFormats.java
##########
@@ -16,40 +16,68 @@
*/
package org.apache.commons.imaging;
+import org.apache.commons.imaging.formats.bmp.BmpImagingParameters;
+import org.apache.commons.imaging.formats.gif.GifImagingParameters;
+import org.apache.commons.imaging.formats.icns.IcnsImagingParameters;
+import org.apache.commons.imaging.formats.ico.IcoImagingParameters;
+import org.apache.commons.imaging.formats.jpeg.JpegImagingParameters;
+import org.apache.commons.imaging.formats.pcx.PcxImagingParameters;
+import org.apache.commons.imaging.formats.png.PngImagingParameters;
+import org.apache.commons.imaging.formats.pnm.PnmImagingParameters;
+import org.apache.commons.imaging.formats.psd.PsdImagingParameters;
+import org.apache.commons.imaging.formats.rgbe.RgbeImagingParameters;
+import org.apache.commons.imaging.formats.tiff.TiffImagingParameters;
+import org.apache.commons.imaging.formats.wbmp.WbmpImagingParameters;
+import org.apache.commons.imaging.formats.xbm.XbmImagingParameters;
+import org.apache.commons.imaging.formats.xpm.XpmImagingParameters;
+
+import java.util.function.Supplier;
+
/**
* Enum of known image formats.
*/
public enum ImageFormats implements ImageFormat {
- UNKNOWN,
- BMP,
- DCX,
- GIF,
- ICNS,
- ICO,
- JBIG2,
- JPEG,
- PAM,
- PSD,
- PBM,
- PGM,
- PNM,
- PPM,
- PCX,
- PNG,
- RGBE,
- TGA,
- TIFF,
- WBMP,
- XBM,
- XPM;
+ UNKNOWN(null),
+ BMP(BmpImagingParameters::new, "bmp", "dib"),
+ DCX(PcxImagingParameters::new, "dcx"),
+ GIF(GifImagingParameters::new, "gif"),
+ ICNS(IcnsImagingParameters::new, "icns"),
+ ICO(IcoImagingParameters::new, "ico"),
+ JBIG2(null),
+ JPEG(JpegImagingParameters::new, "jpg", "jpeg"),
+ PAM(PnmImagingParameters::new, "pam"),
+ PSD(PsdImagingParameters::new, "psd"),
+ PBM(PnmImagingParameters::new, "pbm"),
+ PGM(PnmImagingParameters::new, "pgm"),
+ PNM(PnmImagingParameters::new, "pnm"),
+ PPM(PnmImagingParameters::new, "ppm"),
+ PCX(PcxImagingParameters::new, "pcx", "pcc"),
+ PNG(PngImagingParameters::new, "png"),
+ RGBE(RgbeImagingParameters::new, "rgbe"),
+ TGA(null),
+ TIFF(TiffImagingParameters::new, "tif", "tiff"),
+ WBMP(WbmpImagingParameters::new, "wbmp"),
+ XBM(XbmImagingParameters::new, "xbm"),
+ XPM(XpmImagingParameters::new, "xpm");
+
+ private final String[] extensions;
+
+ ImageFormats(Supplier<? extends ImagingParameters> factory, String ...extensions) {
+ this.extensions = extensions;
+ }
@Override
public String getName() {
return name();
}
@Override
- public String getExtension() {
- return name();
+ public String[] getExtensions() {
+ return this.extensions.clone();
+ }
+
+ @Override
+ public String getDefaultExtension() {
+ return this.extensions[0];
Review comment:
Ouch, fixing it. 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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] darkma773r commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
darkma773r commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-986136369
@kinow, I have a couple of concerns about the API still. That being said, I don't want to single-handedly delay progress on the release, so please feel free to disagree with me. My main concerns are
1. the ease of `ClassCastExceptions` being thrown on `Imaging` methods that accept `ImagingParameters` as arguments, and
2. the way `JpegImagingParameters` extends`TiffImagingParameters`.
We've discussed both of these before. For the first item, the simplest case is to just remove the methods that accept `ImagingParameters`. If we want to keep them, there might be promise in using the configurer callback approach, where the user passes a callback that accepts a parameters instance of the correct type (created by `ImageParser.getDefaultParameters()`) and configures it. Ex:
```java
// ImagingParametersConfigurer.java
@FunctionalInterface
public ImagingParametersConfigurer {
void configure(ImagingParameters p);
}
// Imaging.java
public static ImageInfo getImageInfo(ByteSource bs, ImagingParametersConfigurer configurer) { ... }
// user code
ImageInfo info = Imaging.getImageInfo(bs, p -> p.setStrict(true));
```
Thanks for your tireless work on 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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow edited a comment on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow edited a comment on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-826703877
Note to self:
- [x] rebase again
- [x] check Tabs vs. Spaces
- [x] great suggestion from @gwlucastrig : "use one call for setSubImage, rather than 4 separate calls for setSubImageX, Y, Width, Height."
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] gwlucastrig commented on a change in pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
gwlucastrig commented on a change in pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#discussion_r673102927
##########
File path: src/main/java/org/apache/commons/imaging/ImageParser.java
##########
@@ -79,25 +78,18 @@
* that the documentation is perfect, especially in the more obscure
* and specialized areas of implementation.
*
- * <h3>The "Map params" argument</h3>
+ * <h3>The "params" argument</h3>
*
- * Many of the methods specified by this class accept an argument of
- * type Map giving a list of parameters to be used when processing an
- * image. For example, some of the output formats permit the specification
+ * <p>Many of the methods specified by this class accept an argument of
+ * type {@code T} defining the parameters to be used when
+ * processing an image. For example, some of the output formats permit
* of different kinds of image compression or color models. Some of the
* reading methods permit the calling application to require strict
- * format compliance. In many cases, however, an application will not
- * require the use of this argument. While some of the ImageParser
- * implementations check for (and ignore) null arguments for this parameter,
- * not all of them do (at least not at the time these notes were written).
- * Therefore, a prudent programmer will always supply an valid, though
- * empty instance of a Map implementation when calling such methods.
- * Generally, the java HashMap class is useful for this purpose.
+ * format compliance.</p>
*
- * <p>Additionally, developers creating or enhancing classes derived
- * from ImageParser are encouraged to include such checks in their code.
+ * @param <T> type of parameters used by this image parser
*/
-public abstract class ImageParser extends BinaryFileParser {
+public abstract class ImageParser<T extends ImagingParameters> extends BinaryFileParser {
Review comment:
Maybe forget the whole bit about the copy constructor for AlphaParameters. I think most classes would just do the branching approach and only the guys who needed to do something special would use the copy-constructor approach.
```
@Override
public void parse(ExampleParameters parameters) {
if (parameters instanceof AlphaParameters) {
processAlphaParameters((AlphaParameters) parameters);
} else {
processGeneralParameters(parameters);
}
```
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] darkma773r commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
darkma773r commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-958597172
@kinow
> If the JpegImageParser requires a parameter different than the TiffImageParser, I assume we would have to create a new JpegImagingParameter. In that case, wouldn't we have to keep the old constructor for backward compatibility until a new major release?
I'm not totally sure what you mean here. Can you give an example?
On a related note, in my merge request, I removed all of the parameter classes that did not actually have any of their own properties (such as JpegImageParameters). I'm not sure if this is a great idea, though, since it makes it it kind of hard to tell what parameters to use when creating images of a certain type. If might be better to add them back just for the convenience of not having to look in the documentation to find what parameters class to use. For example, if you are creating a JPEG, you automatically know that you need a JpegImageParameters; BMP implies BmpImageParameters, etc.
One idea for making this readily available in the API would be to add convenience factory methods in Imaging for creating instances of each parameters type. Ex:
```
public static BmpImageParameters bmpParameters() { return new BmpImageParameters(); }
public static TiffImageParameters tiffParameters() { return new TiffImageParameters(); }
```
This way, users would be able to find the parameters class they want by looking at a single class. The methods would return the specific ImageParameters subclass so users could immediately access all of the available properties.
Side note: Is there a reason the JpegImageParser uses TiffImageParameters? The two formats are not related, correct? Perhaps we could have a common base class for them?
> I think it would be best if cases like this were not allowed PcxImagingParameters parser = new PcxImagingParameters(new TiffImagingParameters()); // no error in compile or runtime
Is the idea that it is confusing on what properties are copied over? My thought is that the copy constructor copies over as many fields as it can and ignores the rest. For example, say you are writing an image out in multiple formats. You could create a very specific parameter instance and then use the copy constructors to copy relevant fields for the other formats.
```
TiffImageParameters tiffParams = new TiffImageParameters();
// populate params...
// create images
Imaging.writeImage(src, fileA, ImageFormats.TIFF, tiffParams);
Imaging.writeImage(src, fileB, ImageFormats.BMP, new BmpImageParameters(tiffParams));
Imaging.writeImage(src, fileC, ImageFormats.PNG, new PngImageParameters(tiffParams));
```
> I think you removed the option to allow null parameters in the Imaging#writeImage method (and other methods). I thought about doing that too, that would remove one generic suppress I think, but there are unit tests and examples. The API has allowed it since Sanselan, so not too sure about enforcing parameters to be non-null now.
I attempted to retain the null parameter functionality. In the `normalizeParameters` method, if a null parameter is passed, a default instance is created and returned, allowing downstream code to bypass null checks. `mvn clean install` passes with my current setup. Is there a unit test I missed?
> I liked the error in this one new PngImageParser().writeImage(null, null, new TiffImagingParameters());, which gives java.lang.IllegalArgumentException: Invalid imaging parameters: expected type...
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-958618064
>> If the JpegImageParser requires a parameter different than the TiffImageParser, I assume we would have to create a new JpegImagingParameter. In that case, wouldn't we have to keep the old constructor for backward compatibility until a new major release?
>
>I'm not totally sure what you mean here. Can you give an example?
Sorry, I wasn't very clear. At the moment we have `JpegImageParser parser = new JpegImageParser();`. Internally, the `JpegImageParser` is using a `TiffImagingParameters`.
But if we need to add a parameter that is used only by the `JpegImageParser`, I think we would want to have a `JpegImagingParameters`. In which case I am not sure if we would be able to maintain backward compatibility by replacing the `TiffImagingParameters` in the `JpegImageParser`.
>since it makes it it kind of hard to tell what parameters to use when creating images of a certain type.
:point_up: this is my main concern. Avoid users having to guess what's the parameter that they want to use when parsing a Jpeg, Tiff, Gif, etc. Preferably something that their IDE's can assist with, using auto-complete for example.
>One idea for making this readily available in the API would be to add convenience factory methods in Imaging for creating instances of each parameters type. Ex:
Possibly another design to consider/experiment :-)
>Side note: Is there a reason the JpegImageParser uses TiffImageParameters? The two formats are not related, correct? Perhaps we could have a common base class for them?
We can probably avoid that. I agree it doesn't make much sense for the parameters to have a relationship like that. Currently, [`JpegImageParser`](https://github.com/apache/commons-imaging/blob/0ccabc3416cecf25cdec90a522f0912baf19bc58/src/main/java/org/apache/commons/imaging/formats/jpeg/JpegImageParser.java#L751) has some code in common with the `TiffImageParser` for parsing EXIF metadata. I don't recall 100%, but I believe that's why I implemented everything in the `TiffImagingParameters` and, realizing the Jpeg parameters were the same, made the parent-child class relantionship.
>Is the idea that it is confusing on what properties are copied over?
Not at all, that part is clear and looks OK. It's with users being able to use the wrong parameter class by accident, and realizing only in runtime iff a runtime exception is thrown (or in the worst case, no exception is thrown and the program behaves in a wrong way; debugging an issue like that can be annoying I think).
>I attempted to retain the null parameter functionality. In the normalizeParameters method, if a null parameter is passed, a default instance is created and returned, allowing downstream code to bypass null checks. mvn clean install passes with my current setup. Is there a unit test I missed?
Ah, my bad! Your solution works with the method I was looking at. I wrote this one before looking at the method that creates the default parameters :+1:
I need to find some time to “get in the zone” and try a few things. But it would be, mainly, to prevent users being able to use the wrong parameter types by accident, and making it so that we can add/remove parameters (deprecating what was removed) in the 1.x release series without worrying about backward compatibility. If we are able to modify your PR again to accommodate these two requirements, I think we would have the perfect solution :-)
Thanks a lot!
Bruno
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow edited a comment on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow edited a comment on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-961796289
That is a good point @gwlucastrig , and I have no objections to massaging the code of the `Imaging` class. I will spend some time thinking about that, and seeing if/when the parameters would be useful or not. Thanks!!! (and you have a perspective of user + dev, which is really valuable @gwlucastrig !)
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow edited a comment on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow edited a comment on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-963088960
Agreed @darkma773r , +1 on @gwlucastrig suggestion too. Let's review Imaging class. Will start thinking on what/how to change too 👍
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a change in pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on a change in pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#discussion_r785344916
##########
File path: src/main/java/org/apache/commons/imaging/ImageParser.java
##########
@@ -563,27 +561,22 @@ public final BufferedImage getBufferedImage(final File file, final Map<String, O
* <p>The params argument provides a mechanism for individual
* implementations to pass optional information into the parser.
* Not all formats will support this capability. Currently,
- * some of the parsers do not check for null arguments. So in cases
- * where no optional specifications are supported, application
- * code should pass in an empty instance of an implementation of
- * the map interface (i.e. an empty HashMap).
+ * some of the parsers do not check for null arguments.</p>
Review comment:
Good idea! Will create it now.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] gwlucastrig commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
gwlucastrig commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-841644514
AFAIK there are two candidates to do the getter: either the Java Rectangle
class or implement out own version of the Rectangle. At one point you
wanted to avoid java at classes, but in this case, I think Rectangle has
advantages
On Sat, May 15, 2021, 1:44 AM Bruno P. Kinoshita ***@***.***>
wrote:
> @gwlucastrig <https://github.com/gwlucastrig>
>
> great suggestion from @gwlucastrig <https://github.com/gwlucastrig> :
> "use one call for setSubImage, rather than 4 separate calls for
> setSubImageX, Y, Width, Height."
>
> I started making this change in the TiffImagingParameter. First the public
> void setSubImage(Integer width, Integer height, Integer x, Integer y).
> All good so far.
>
> But when I finished and started adding the getter, I was thinking in
> Python or JS, and wanted to return all the 4 parameters at once 😥 Then I
> realized I would have to create another class to return the 4 parameters…
> do you have any suggestion for the getter of the subimage in the tiff
> imaging parameter class?
>
> Thanks!
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/commons-imaging/pull/116#issuecomment-841605341>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AEWJDYOC6UHIHBYT2C46XRDTNYC5FANCNFSM4WPUJSLQ>
> .
>
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] darkma773r commented on a change in pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
darkma773r commented on a change in pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#discussion_r680499709
##########
File path: src/main/java/org/apache/commons/imaging/ImageParser.java
##########
@@ -79,25 +78,18 @@
* that the documentation is perfect, especially in the more obscure
* and specialized areas of implementation.
*
- * <h3>The "Map params" argument</h3>
+ * <h3>The "params" argument</h3>
*
- * Many of the methods specified by this class accept an argument of
- * type Map giving a list of parameters to be used when processing an
- * image. For example, some of the output formats permit the specification
+ * <p>Many of the methods specified by this class accept an argument of
+ * type {@code T} defining the parameters to be used when
+ * processing an image. For example, some of the output formats permit
* of different kinds of image compression or color models. Some of the
* reading methods permit the calling application to require strict
- * format compliance. In many cases, however, an application will not
- * require the use of this argument. While some of the ImageParser
- * implementations check for (and ignore) null arguments for this parameter,
- * not all of them do (at least not at the time these notes were written).
- * Therefore, a prudent programmer will always supply an valid, though
- * empty instance of a Map implementation when calling such methods.
- * Generally, the java HashMap class is useful for this purpose.
+ * format compliance.</p>
*
- * <p>Additionally, developers creating or enhancing classes derived
- * from ImageParser are encouraged to include such checks in their code.
+ * @param <T> type of parameters used by this image parser
*/
-public abstract class ImageParser extends BinaryFileParser {
+public abstract class ImageParser<T extends ImagingParameters> extends BinaryFileParser {
Review comment:
A couple more thoughts here:
- I like the idea of having the TIFF and JPEG parameters extend a common ancestor if possible. That seems like a more maintainable solution. With the current hierarchy, if a property needed to be added to the TIFF parameters, it would also be added to the JPEG one, regardless of whether or not it applied to that format.
- With the removal of the `imageFormat` property, we could completely remove all of the `ImagingParameters` subclasses that do not add any properties, such as `BmpImagingParameters`. They do not seem to add much value anymore and could easily be added later if needed.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a change in pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on a change in pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#discussion_r671819743
##########
File path: src/main/java/org/apache/commons/imaging/ImageFormats.java
##########
@@ -16,40 +16,92 @@
*/
package org.apache.commons.imaging;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+
+import org.apache.commons.imaging.formats.bmp.BmpImagingParameters;
+import org.apache.commons.imaging.formats.gif.GifImagingParameters;
+import org.apache.commons.imaging.formats.icns.IcnsImagingParameters;
+import org.apache.commons.imaging.formats.ico.IcoImagingParameters;
+import org.apache.commons.imaging.formats.jpeg.JpegImagingParameters;
+import org.apache.commons.imaging.formats.pcx.PcxImagingParameters;
+import org.apache.commons.imaging.formats.png.PngImagingParameters;
+import org.apache.commons.imaging.formats.pnm.PnmImagingParameters;
+import org.apache.commons.imaging.formats.psd.PsdImagingParameters;
+import org.apache.commons.imaging.formats.rgbe.RgbeImagingParameters;
+import org.apache.commons.imaging.formats.tiff.TiffImagingParameters;
+import org.apache.commons.imaging.formats.wbmp.WbmpImagingParameters;
+import org.apache.commons.imaging.formats.xbm.XbmImagingParameters;
+import org.apache.commons.imaging.formats.xpm.XpmImagingParameters;
+
/**
* Enum of known image formats.
*/
public enum ImageFormats implements ImageFormat {
- UNKNOWN,
- BMP,
- DCX,
- GIF,
- ICNS,
- ICO,
- JBIG2,
- JPEG,
- PAM,
- PSD,
- PBM,
- PGM,
- PNM,
- PPM,
- PCX,
- PNG,
- RGBE,
- TGA,
- TIFF,
- WBMP,
- XBM,
- XPM;
+ UNKNOWN(ImagingParameters.class),
+ BMP(BmpImagingParameters.class, "bmp", "dib"),
+ DCX(PcxImagingParameters.class, "dcx"),
+ GIF(GifImagingParameters.class, "gif"),
+ ICNS(IcnsImagingParameters.class, "icns"),
+ ICO(IcoImagingParameters.class, "ico"),
+ JBIG2(ImagingParameters.class),
+ JPEG(JpegImagingParameters.class, "jpg", "jpeg"),
+ PAM(PnmImagingParameters.class, "pam"),
+ PSD(PsdImagingParameters.class, "psd"),
+ PBM(PnmImagingParameters.class, "pbm"),
+ PGM(PnmImagingParameters.class, "pgm"),
+ PNM(PnmImagingParameters.class, "pnm"),
+ PPM(PnmImagingParameters.class, "ppm"),
+ PCX(PcxImagingParameters.class, "pcx", "pcc"),
+ PNG(PngImagingParameters.class, "png"),
+ RGBE(RgbeImagingParameters.class, "rgbe"),
+ TGA(ImagingParameters.class),
+ TIFF(TiffImagingParameters.class, "tif", "tiff"),
+ WBMP(WbmpImagingParameters.class, "wbmp"),
+ XBM(XbmImagingParameters.class, "xbm"),
+ XPM(XpmImagingParameters.class, "xpm");
+
+ private static final Logger LOGGER = Logger.getLogger(ImageFormats.class.getName());
+
+ private String[] extensions;
+ private Class<? extends ImagingParameters> parametersClass;
+
+ ImageFormats(Class<? extends ImagingParameters> parametersClass, String ...extensions) {
+ this.extensions = extensions;
+ this.parametersClass = parametersClass;
+ }
@Override
public String getName() {
return name();
}
@Override
- public String getExtension() {
- return name();
+ public String[] getExtensions() {
+ return this.extensions.clone();
+ }
+
+ @Override
+ public String getDefaultExtension() {
+ return this.extensions[0];
+ }
+
+ @Override
+ public ImagingParameters createImagingParameters() {
+ ImagingParameters parameters = null;
+ if (this.parametersClass != null) {
+ Constructor<?> ctor;
+ try {
+ ctor = this.parametersClass.getConstructor();
+ parameters = (ImagingParameters) ctor.newInstance();
+ } catch (NoSuchMethodException | SecurityException | InstantiationException | IllegalAccessException | IllegalArgumentException | InvocationTargetException e) {
+ LOGGER.log(Level.WARNING, "Failed to create imaging parameters: " + e.getMessage(), e);
+ parameters = new ImagingParameters();
+ }
+ parameters.setImageFormat(this);
+ }
+ return parameters;
Review comment:
This was a great advice, and I'm super happy for having asked for feedback in the mailing list @darkma773r. It simplified a lot the code. Got to remember this trick next time I have a similar situation, I feel like my brain is still tuned to use reflections in cases like this...
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] gwlucastrig edited a comment on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
gwlucastrig edited a comment on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-918761710
I saw the note you posted on the Commons mailing list about the design of this. If you're still looking for ideas for alternate approaches, maybe this will help. I'm wondering if just a simple brute-force approach might do the trick. So I will throw out a wild idea (which I'd be the first to admit I have not thought through thoroughly).
We have a number of issues:
1. Handle cases where applications pass in null imaging parameters (just use defaults)
2. Handle cases where applications pass in an instance of the generic ImagingParameters base class.
3. Handle cases where applications pass in an instance of the subject-matter imaging parameters (i.e. TiffImagingParameters, PngImagingParameters, etc.).
4. Handle cases where applications pass in the wrong thing.
```
TestStuff testStuff = new TestStuff(); // works on TIFFs (for example)
testStuff.aTypicalMethod(null);
testStuff.aTypicalMethod(imgParams);
testStuff.aTypicalMethod(tiffParams);
testStuff.aTypicalMethod(pngParams); // throws exception
```
And we'd like to make the contents of "aTypicalMethod" be pretty compact. In this case, using TiffImagingParameters as an example, what if TiffImagingParameters (and all its brothers and sisters) had a static method called resolveParameters(ImagingParameters) that worked like:
```
void aTypicalMethod(ImagingParameters imagingParameters) {
TiffImagingParameters params = TiffImagingParameters.resolveParameters(imagingParameters);
// now do some work using the parameters
}
```
**How we make this work**
The base class would define two constructors. A no-argument constructor and a "copy constructor".
```
public ImagingParameters(){
}
public ImagingParameters(ImagingParameters source) {
this.bufferedImageFactory = source.bufferedImageFactory;
this.fileName = source.fileName;
this.pixelDensity = source.pixelDensity;
this.strict = source.strict;
}
```
Subject-matter specific parameters classes would implement compatible constructors:
```
public TiffImagingParameters(){
}
public TiffImagingParameters(ImagingParameters imagingParameters){
super(imagingParameters);
}
```
Finally, each parameters class would implement a static method:
```
static public TiffImagingParameters resolveParameters(ImagingParameters imagingParameters) {
TiffImagingParameters params;
if (imagingParameters == null) {
return new TiffImagingParameters();
} else if (imagingParameters instanceof TiffImagingParameters) {
return (TiffImagingParameters) imagingParameters;
} else if (ImagingParameters.class.equals(imagingParameters.getClass())) {
// the specified class is the generic, base class.
// so just copy it's content into a new TiffImagingParameters instance
return new TiffImagingParameters(imagingParameters);
} else {
// In this case, the calling application gave us an instance of an
// incompatible parameters class.
throw new IllegalArgumentException(
"Invalid imaging parameters class "
+ imagingParameters.getClass().getSimpleName());
}
}
```
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a change in pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on a change in pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#discussion_r780552113
##########
File path: src/changes/changes.xml
##########
@@ -45,6 +45,21 @@ The <action> type attribute can be add,update,fix,remove.
</properties>
<body>
<release version="1.0-alpha3" date="2020-??-??" description="Third 1.0 alpha release">
+ <action issue="IMAGING-159" dev="kinow" type="fix" due-to="Bruno P. Kinoshita, Gary Lucas, Matt Juntunen">
Review comment:
Thanks for the help!!! :tada:
##########
File path: src/changes/changes.xml
##########
@@ -45,6 +45,21 @@ The <action> type attribute can be add,update,fix,remove.
</properties>
<body>
<release version="1.0-alpha3" date="2020-??-??" description="Third 1.0 alpha release">
+ <action issue="IMAGING-159" dev="kinow" type="fix" due-to="Bruno P. Kinoshita, Gary Lucas, Matt Juntunen">
Review comment:
Thanks for the help!!! :tada: :point_up:
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] coveralls commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-841637226
[](https://coveralls.io/builds/39713497)
Coverage decreased (-0.1%) to 76.521% when pulling **97596136b23973d732c54d235663074d7fef78dd on kinow:IMAGING-159-1** into **535a83e7ca598678114617697aa81cd2d4b558f0 on apache:master**.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] coveralls edited a comment on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-841637226
[](https://coveralls.io/builds/41740806)
Coverage decreased (-0.2%) to 76.543% when pulling **0c2ca31b761d95f86c4a58a76026aea930116f12 on kinow:IMAGING-159-1** into **a87b545e1019b43bfbe31fa5df3a4f3095a8b438 on apache:master**.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] gwlucastrig commented on a change in pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
gwlucastrig commented on a change in pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#discussion_r679432456
##########
File path: src/main/java/org/apache/commons/imaging/ImageParser.java
##########
@@ -79,25 +78,18 @@
* that the documentation is perfect, especially in the more obscure
* and specialized areas of implementation.
*
- * <h3>The "Map params" argument</h3>
+ * <h3>The "params" argument</h3>
*
- * Many of the methods specified by this class accept an argument of
- * type Map giving a list of parameters to be used when processing an
- * image. For example, some of the output formats permit the specification
+ * <p>Many of the methods specified by this class accept an argument of
+ * type {@code T} defining the parameters to be used when
+ * processing an image. For example, some of the output formats permit
* of different kinds of image compression or color models. Some of the
* reading methods permit the calling application to require strict
- * format compliance. In many cases, however, an application will not
- * require the use of this argument. While some of the ImageParser
- * implementations check for (and ignore) null arguments for this parameter,
- * not all of them do (at least not at the time these notes were written).
- * Therefore, a prudent programmer will always supply an valid, though
- * empty instance of a Map implementation when calling such methods.
- * Generally, the java HashMap class is useful for this purpose.
+ * format compliance.</p>
*
- * <p>Additionally, developers creating or enhancing classes derived
- * from ImageParser are encouraged to include such checks in their code.
+ * @param <T> type of parameters used by this image parser
*/
-public abstract class ImageParser extends BinaryFileParser {
+public abstract class ImageParser<T extends ImagingParameters> extends BinaryFileParser {
Review comment:
Well, I am thinking back to my reaction when Java 1.5 first introduced generics. My first thought was "what a good idea". My second thought was "of all the mistakes I have made coding in Java (and there are a lot of them), I don't think I ever tried to store the wrong kind of object in a collection". My point here is that we need to simplify things for the user as much as possible, but if our own code starts to get too complicated we might be going down the wrong road. Also, I wonder why JpegImagingParameters should extend TiffImagingParameters. The TIFF format has MANY Tiff-unique elements (support for old fashioned FAX parameters, Photogrammetric Interpreters, odd-duck compression standards). I would think that if there were any overlap between the two package branches at all, perhaps JpegImagingParameters and TiffImagingParameters might have a common ancestor. Of course, I have been so completely focused on TIFF that I haven't really studied the JPEG package. So I a
m not sure how it is organized.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-918771649
Hi @gwlucastrig
Thanks for the detailed example. The only part of the implementation that bothers me is the static method.
>Finally, each parameters class would implement a static method:
That might become tedious to maintain that code, and also error-prone. Of course generics can also be error-prone, and are not too simple to maintain.
If only we had a way to combine; make so that users can easily write code using auto-complete, IDE, etc, in an intuitive way, and in a way that we don't have any risks in our side (or very low risk of bugs/hard to maintain 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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow edited a comment on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow edited a comment on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-826703877
Note to self:
- [ ] rebase again
- [ ] check Tabs vs. Spaces
- [ ] great suggestion from @gwlucastrig : "use one call for setSubImage, rather than 4 separate calls for setSubImageX, Y, Width, Height."
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-841637202
@gwlucastrig the code is now compiling. I had to make more design decisions than I expected to add the parameters. See `ImagingParameters` and the implementations like `TiffImagingParameters`.
But also take a look at the changes in `ImageFormat` and `ImageFormats` :+1:
Any feedback or suggestions on how to improve it are welcome! Also see comment above about the getters for the subimage in the TIFF parameter class.
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] darkma773r commented on a change in pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
darkma773r commented on a change in pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#discussion_r671862277
##########
File path: src/main/java/org/apache/commons/imaging/ImageFormats.java
##########
@@ -16,40 +16,92 @@
*/
package org.apache.commons.imaging;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+
+import org.apache.commons.imaging.formats.bmp.BmpImagingParameters;
+import org.apache.commons.imaging.formats.gif.GifImagingParameters;
+import org.apache.commons.imaging.formats.icns.IcnsImagingParameters;
+import org.apache.commons.imaging.formats.ico.IcoImagingParameters;
+import org.apache.commons.imaging.formats.jpeg.JpegImagingParameters;
+import org.apache.commons.imaging.formats.pcx.PcxImagingParameters;
+import org.apache.commons.imaging.formats.png.PngImagingParameters;
+import org.apache.commons.imaging.formats.pnm.PnmImagingParameters;
+import org.apache.commons.imaging.formats.psd.PsdImagingParameters;
+import org.apache.commons.imaging.formats.rgbe.RgbeImagingParameters;
+import org.apache.commons.imaging.formats.tiff.TiffImagingParameters;
+import org.apache.commons.imaging.formats.wbmp.WbmpImagingParameters;
+import org.apache.commons.imaging.formats.xbm.XbmImagingParameters;
+import org.apache.commons.imaging.formats.xpm.XpmImagingParameters;
+
/**
* Enum of known image formats.
*/
public enum ImageFormats implements ImageFormat {
- UNKNOWN,
- BMP,
- DCX,
- GIF,
- ICNS,
- ICO,
- JBIG2,
- JPEG,
- PAM,
- PSD,
- PBM,
- PGM,
- PNM,
- PPM,
- PCX,
- PNG,
- RGBE,
- TGA,
- TIFF,
- WBMP,
- XBM,
- XPM;
+ UNKNOWN(ImagingParameters.class),
+ BMP(BmpImagingParameters.class, "bmp", "dib"),
+ DCX(PcxImagingParameters.class, "dcx"),
+ GIF(GifImagingParameters.class, "gif"),
+ ICNS(IcnsImagingParameters.class, "icns"),
+ ICO(IcoImagingParameters.class, "ico"),
+ JBIG2(ImagingParameters.class),
+ JPEG(JpegImagingParameters.class, "jpg", "jpeg"),
+ PAM(PnmImagingParameters.class, "pam"),
+ PSD(PsdImagingParameters.class, "psd"),
+ PBM(PnmImagingParameters.class, "pbm"),
+ PGM(PnmImagingParameters.class, "pgm"),
+ PNM(PnmImagingParameters.class, "pnm"),
+ PPM(PnmImagingParameters.class, "ppm"),
+ PCX(PcxImagingParameters.class, "pcx", "pcc"),
+ PNG(PngImagingParameters.class, "png"),
+ RGBE(RgbeImagingParameters.class, "rgbe"),
+ TGA(ImagingParameters.class),
+ TIFF(TiffImagingParameters.class, "tif", "tiff"),
+ WBMP(WbmpImagingParameters.class, "wbmp"),
+ XBM(XbmImagingParameters.class, "xbm"),
+ XPM(XpmImagingParameters.class, "xpm");
+
+ private static final Logger LOGGER = Logger.getLogger(ImageFormats.class.getName());
+
+ private String[] extensions;
+ private Class<? extends ImagingParameters> parametersClass;
+
+ ImageFormats(Class<? extends ImagingParameters> parametersClass, String ...extensions) {
+ this.extensions = extensions;
+ this.parametersClass = parametersClass;
+ }
@Override
public String getName() {
return name();
}
@Override
- public String getExtension() {
- return name();
+ public String[] getExtensions() {
+ return this.extensions.clone();
+ }
+
+ @Override
+ public String getDefaultExtension() {
+ return this.extensions[0];
+ }
+
+ @Override
+ public ImagingParameters createImagingParameters() {
+ ImagingParameters parameters = null;
+ if (this.parametersClass != null) {
+ Constructor<?> ctor;
+ try {
+ ctor = this.parametersClass.getConstructor();
+ parameters = (ImagingParameters) ctor.newInstance();
+ } catch (NoSuchMethodException | SecurityException | InstantiationException | IllegalAccessException | IllegalArgumentException | InvocationTargetException e) {
+ LOGGER.log(Level.WARNING, "Failed to create imaging parameters: " + e.getMessage(), e);
+ parameters = new ImagingParameters();
+ }
+ parameters.setImageFormat(this);
+ }
+ return parameters;
Review comment:
Woohoo! I learned this trick from Gilles and I pretty much use it all the time now :-)
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-953750338
No worries @darkma773r , thanks for having another look at this one!
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] darkma773r commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
darkma773r commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-1013783823
Woohoo! I vote to merge 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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] darkma773r commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
darkma773r commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-1008425078
Awesome, @kinow! I'll take a look at this as soon as I get some time. Unfortunately, that probably won't be for at least a few more days.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow edited a comment on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow edited a comment on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-961796289
That is a good point @gwlucastrig , and I have no objections to massaging the code of the `Imaging` class. I will spend some time thinking about that, and seeing if/when the parameters would be useful or not. Thanks!!! (and you have a perspective of user + dev, which is really valuable @gwlucastrig !)
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-961461586
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a change in pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on a change in pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#discussion_r780528232
##########
File path: src/main/java/org/apache/commons/imaging/Imaging.java
##########
@@ -857,20 +837,19 @@ public static Dimension getImageSize(final File file) throws ImageReadException,
*
* @param file
* File containing image data.
- * @param params
- * Map of optional parameters, defined in ImagingConstants.
+ * @param params optional parameters.
* @return The width and height of the image.
* @throws ImageReadException if it fails to parse the image
* @throws IOException if it fails to read the image data
*/
- public static Dimension getImageSize(final File file, final Map<String, Object> params)
+ public static Dimension getImageSize(final File file, final ImagingParameters params)
Review comment:
@darkma773r I went with option 2. It reduces the `Imaging.java` size, and prevents the god-class issue in case we added more image formats later. IMHO, it will be simpler to either improve the `ImageParser`'s API, or think in how to better handle parameters in the `Imaging` class (like merging parameters, of providing factories as you suggested).
But for 1.0 this should be better than the old `Hashmap`, and avoid runtime exceptions. Thank you!!!
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] gwlucastrig edited a comment on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
gwlucastrig edited a comment on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-841791184
It looks like providing compatibility with Android OS may not be practical.
Android has its own graphics and media classes. So anything with java.awt
in it will not port. Common Imaging using BufferedImage extensively. Most
other AWT elements are used only a little. Color shows up the most. The
ICC_ColorSpace also shows up. A couple of the test applications use
Graphics to draw some test images as input.
I think I might post a question on Java Coderanch or Stackoverflow to ask
about compatible solutions.
On Sat, May 15, 2021 at 8:18 AM Bruno P. Kinoshita ***@***.***>
wrote:
> If those classes are from awt, the issue is that we have a dependency to
> java.desktop (I think that's the java module name) and also cause an issue
> to users of the API that are using Android (as IIUC, there's no awt in
> Android's Java SDK?).
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/commons-imaging/pull/116#issuecomment-841649340>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AEWJDYNES4M3XQCOJIEZN6TTNZRAXANCNFSM4WPUJSLQ>
> .
>
<details>
<summary>Search "java.awt" (229 hits in 121 files of 557 searched)</summary>
commons-imaging\src\main\java\org\apache\commons\imaging\ColorTools.java (11 hits)
Line 20: import java.awt.RenderingHints;
Line 21: import java.awt.Transparency;
Line 22: import java.awt.color.ColorSpace;
Line 23: import java.awt.color.ICC_ColorSpace;
Line 24: import java.awt.color.ICC_Profile;
Line 25: import java.awt.image.BufferedImage;
Line 26: import java.awt.image.ColorConvertOp;
Line 27: import java.awt.image.ColorModel;
Line 28: import java.awt.image.ComponentColorModel;
Line 29: import java.awt.image.DirectColorModel;
Line 30: import java.awt.image.ImagingOpException;
commons-imaging\src\main\java\org\apache\commons\imaging\common\BufferedImageFactory.java (1 hit)
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\common\ImageBuilder.java (9 hits)
Line 43: import java.awt.color.ColorSpace;
Line 44: import java.awt.image.BufferedImage;
Line 45: import java.awt.image.ColorModel;
Line 46: import java.awt.image.DataBuffer;
Line 47: import java.awt.image.DataBufferInt;
Line 48: import java.awt.image.DirectColorModel;
Line 49: import java.awt.image.Raster;
Line 50: import java.awt.image.RasterFormatException;
Line 51: import java.awt.image.WritableRaster;
commons-imaging\src\main\java\org\apache\commons\imaging\common\RgbBufferedImageFactory.java (1 hit)
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\common\SimpleBufferedImageFactory.java (1 hit)
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\bmp\BmpImageParser.java (2 hits)
Line 27: import java.awt.Dimension;
Line 28: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\bmp\BmpWriter.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\bmp\BmpWriterPalette.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\bmp\BmpWriterRgb.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\dcx\DcxImageParser.java (2 hits)
Line 23: import java.awt.Dimension;
Line 24: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\gif\GifImageParser.java (2 hits)
Line 28: import java.awt.Dimension;
Line 29: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\icns\IcnsDecoder.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\icns\IcnsImageParser.java (2 hits)
Line 23: import java.awt.Dimension;
Line 24: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\ico\IcoImageParser.java (2 hits)
Line 26: import java.awt.Dimension;
Line 27: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\jpeg\decoder\JpegDecoder.java (6 hits)
Line 21: import java.awt.image.BufferedImage;
Line 22: import java.awt.image.ColorModel;
Line 23: import java.awt.image.DataBuffer;
Line 24: import java.awt.image.DirectColorModel;
Line 25: import java.awt.image.Raster;
Line 26: import java.awt.image.WritableRaster;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\jpeg\JpegImageMetadata.java (2 hits)
Line 19: import java.awt.Dimension;
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\jpeg\JpegImageParser.java (2 hits)
Line 23: import java.awt.Dimension;
Line 24: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\pcx\PcxImageParser.java (11 hits)
Line 24: import java.awt.Dimension;
Line 25: import java.awt.Transparency;
Line 26: import java.awt.color.ColorSpace;
Line 27: import java.awt.image.BufferedImage;
Line 28: import java.awt.image.ColorModel;
Line 29: import java.awt.image.ComponentColorModel;
Line 30: import java.awt.image.DataBuffer;
Line 31: import java.awt.image.DataBufferByte;
Line 32: import java.awt.image.IndexColorModel;
Line 33: import java.awt.image.Raster;
Line 34: import java.awt.image.WritableRaster;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\pcx\PcxWriter.java (1 hit)
Line 18: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\png\PngImageParser.java (6 hits)
Line 25: import java.awt.Dimension;
Line 26: import java.awt.color.ColorSpace;
Line 27: import java.awt.color.ICC_ColorSpace;
Line 28: import java.awt.color.ICC_Profile;
Line 29: import java.awt.image.BufferedImage;
Line 30: import java.awt.image.ColorModel;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\png\PngWriter.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\png\ScanExpediter.java (1 hit)
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\png\ScanExpediterInterlaced.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\png\ScanExpediterSimple.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\pnm\PamWriter.java (1 hit)
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\pnm\PbmWriter.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\pnm\PgmWriter.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\pnm\PnmImageParser.java (2 hits)
Line 22: import java.awt.Dimension;
Line 23: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\pnm\PnmWriter.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\pnm\PpmWriter.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\psd\dataparsers\DataParser.java (2 hits)
Line 19: import java.awt.image.BufferedImage;
Line 20: import java.awt.image.DataBuffer;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\psd\datareaders\CompressedDataReader.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\psd\datareaders\DataReader.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\psd\datareaders\UncompressedDataReader.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\psd\PsdImageParser.java (2 hits)
Line 26: import java.awt.Dimension;
Line 27: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\rgbe\RgbeImageParser.java (10 hits)
Line 19: import java.awt.Dimension;
Line 20: import java.awt.Point;
Line 21: import java.awt.Transparency;
Line 22: import java.awt.color.ColorSpace;
Line 23: import java.awt.image.BandedSampleModel;
Line 24: import java.awt.image.BufferedImage;
Line 25: import java.awt.image.ComponentColorModel;
Line 26: import java.awt.image.DataBuffer;
Line 27: import java.awt.image.DataBufferFloat;
Line 28: import java.awt.image.Raster;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\tiff\datareaders\DataReaderStrips.java (1 hit)
Line 20: import java.awt.Rectangle;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\tiff\datareaders\DataReaderTiled.java (1 hit)
Line 26: import java.awt.Rectangle;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\tiff\datareaders\ImageDataReader.java (1 hit)
Line 32: import java.awt.Rectangle;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\tiff\photometricinterpreters\floatingpoint\PaletteEntry.java (1 hit)
Line 19: import java.awt.Color;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\tiff\photometricinterpreters\floatingpoint\PaletteEntryForRange.java (1 hit)
Line 19: import java.awt.Color;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\tiff\photometricinterpreters\floatingpoint\PaletteEntryForValue.java (1 hit)
Line 19: import java.awt.Color;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\tiff\photometricinterpreters\floatingpoint\PhotometricInterpreterFloat.java (1 hit)
Line 19: import java.awt.Color;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\tiff\TiffDirectory.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\tiff\TiffImageMetadata.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\tiff\TiffImageParser.java (3 hits)
Line 28: import java.awt.Dimension;
Line 29: import java.awt.Rectangle;
Line 30: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\tiff\write\TiffImageWriterBase.java (2 hits)
Line 33: import java.awt.image.BufferedImage;
Line 34: import java.awt.image.ColorModel;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\wbmp\WbmpImageParser.java (7 hits)
Line 21: import java.awt.Dimension;
Line 22: import java.awt.image.BufferedImage;
Line 23: import java.awt.image.DataBuffer;
Line 24: import java.awt.image.DataBufferByte;
Line 25: import java.awt.image.IndexColorModel;
Line 26: import java.awt.image.Raster;
Line 27: import java.awt.image.WritableRaster;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\xbm\XbmImageParser.java (8 hits)
Line 19: import java.awt.Dimension;
Line 20: import java.awt.image.BufferedImage;
Line 21: import java.awt.image.ColorModel;
Line 22: import java.awt.image.DataBuffer;
Line 23: import java.awt.image.DataBufferByte;
Line 24: import java.awt.image.IndexColorModel;
Line 25: import java.awt.image.Raster;
Line 26: import java.awt.image.WritableRaster;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\xpm\XpmImageParser.java (8 hits)
Line 18: import java.awt.Dimension;
Line 19: import java.awt.image.BufferedImage;
Line 20: import java.awt.image.ColorModel;
Line 21: import java.awt.image.DataBuffer;
Line 22: import java.awt.image.DirectColorModel;
Line 23: import java.awt.image.IndexColorModel;
Line 24: import java.awt.image.Raster;
Line 25: import java.awt.image.WritableRaster;
commons-imaging\src\main\java\org\apache\commons\imaging\icc\IccProfileParser.java (1 hit)
Line 23: import java.awt.color.ICC_Profile;
commons-imaging\src\main\java\org\apache\commons\imaging\ImageDump.java (4 hits)
Line 19: import java.awt.color.ColorSpace;
Line 20: import java.awt.color.ICC_ColorSpace;
Line 21: import java.awt.color.ICC_Profile;
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\ImageParser.java (2 hits)
Line 19: import java.awt.Dimension;
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\Imaging.java (3 hits)
Line 22: import java.awt.Dimension;
Line 23: import java.awt.color.ICC_Profile;
Line 24: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\internal\Debug.java (1 hit)
Line 19: import java.awt.color.ICC_Profile;
commons-imaging\src\main\java\org\apache\commons\imaging\palette\Dithering.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\palette\MedianCutQuantizer.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\palette\PaletteFactory.java (3 hits)
Line 19: import java.awt.color.ColorSpace;
Line 20: import java.awt.image.BufferedImage;
Line 21: import java.awt.image.ColorModel;
commons-imaging\src\test\java\org\apache\commons\imaging\common\bytesource\ByteSourceImageTest.java (2 hits)
Line 24: import java.awt.Dimension;
Line 25: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\common\bytesource\ByteSourceInputStreamTest.java (1 hit)
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\common\ImageBuilderTest.java (3 hits)
Line 19: import java.awt.image.BufferedImage;
Line 20: import java.awt.image.ColorModel;
Line 21: import java.awt.image.RasterFormatException;
commons-imaging\src\test\java\org\apache\commons\imaging\common\RgbBufferedImageFactoryTest.java (1 hit)
Line 22: import java.awt.image.DirectColorModel;
commons-imaging\src\test\java\org\apache\commons\imaging\examples\ApacheImagingSpeedAndMemoryTest.java (1 hit)
Line 156: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\examples\ImageReadExample.java (5 hits)
Line 19: import java.awt.GraphicsConfiguration;
Line 20: import java.awt.GraphicsDevice;
Line 21: import java.awt.GraphicsEnvironment;
Line 22: import java.awt.Transparency;
Line 23: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\examples\ImageWriteExample.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\examples\SampleUsage.java (3 hits)
Line 19: import java.awt.Dimension;
Line 20: import java.awt.color.ICC_Profile;
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\examples\tiff\ExampleReadFloatingPointData.java (2 hits)
Line 19: import java.awt.Color;
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\examples\tiff\ReadAndRenderFloatingPoint.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\examples\tiff\ReadTagsAndImages.java (1 hit)
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\bmp\BmpReadTest.java (1 hit)
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\bmp\BmpRoundtripTest.java (1 hit)
Line 23: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\bmp\BmpWriterRgbTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\dcx\DcxReadTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\gif\GifReadTest.java (1 hit)
Line 24: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\icns\IcnsReadTest.java (1 hit)
Line 23: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\icns\IcnsRoundTripTest.java (1 hit)
Line 23: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\ico\IcoReadTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\ico\IcoRoundtripTest.java (1 hit)
Line 23: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\jpeg\exif\MicrosoftTagTest.java (1 hit)
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\jpeg\iptc\IptcFullDiscardTest.java (2 hits)
Line 20: import java.awt.Graphics2D;
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\jpeg\JpegReadTest.java (1 hit)
Line 23: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\jpeg\JpegWithJpegThumbnailTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\jpeg\specific\JpegImageParserTest.java (1 hit)
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\pam\PamReadTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\pcx\PcxReadTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\png\ConvertPngToGifTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\png\PngMultipleRoundtripTest.java (1 hit)
Line 23: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\png\PngReadTest.java (1 hit)
Line 23: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\png\PngTextTest.java (2 hits)
Line 24: import java.awt.Color;
Line 25: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\png\PngWriteForceTrueColorText.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\png\PngWriteReadTest.java (1 hit)
Line 25: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\pnm\PnmImageParserTest.java (2 hits)
Line 23: import java.awt.image.BufferedImage;
Line 24: import java.awt.image.DataBufferInt;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\psd\PsdReadTest.java (1 hit)
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\rgbe\RgbeReadTest.java (1 hit)
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\tiff\photometricinterpreters\floatingpoint\PaletteEntryForRangeTest.java (1 hit)
Line 24: import java.awt.Color;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\tiff\photometricinterpreters\floatingpoint\PaletteEntryForValueTest.java (1 hit)
Line 24: import java.awt.Color;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\tiff\photometricinterpreters\floatingpoint\PhotometricInterpreterFloatTest.java (1 hit)
Line 23: import java.awt.Color;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\tiff\TiffAlphaRoundTripTest.java (4 hits)
Line 19: import java.awt.AlphaComposite;
Line 20: import java.awt.Color;
Line 21: import java.awt.Graphics2D;
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\tiff\TiffCcittTest.java (1 hit)
Line 23: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\tiff\TiffFloatingPointReadTest.java (2 hits)
Line 23: import java.awt.Color;
Line 24: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\tiff\TiffFloatingPointRoundTripTest.java (1 hit)
Line 23: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\tiff\TiffReadAlphaTest.java (3 hits)
Line 19: import java.awt.Color;
Line 20: import java.awt.Graphics2D;
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\tiff\TiffReadTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\tiff\TiffRoundtripTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\tiff\TiffSubImageTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\wbmp\WbmpReadTest.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\xbm\XbmReadTest.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\xmp\XmpUpdateTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\xpm\XpmReadTest.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\ImageDumpTest.java (2 hits)
Line 21: import java.awt.color.ColorSpace;
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\palette\DitheringTest.java (1 hit)
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\palette\PaletteQuantizationTest.java (1 hit)
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\roundtrip\BitmapRoundtripTest.java (1 hit)
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\roundtrip\FullColorRoundtrip.java (1 hit)
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\roundtrip\GrayscaleRountripTest.java (1 hit)
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\roundtrip\ImageAsserts.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\roundtrip\LimitedColorRoundtripTest.java (1 hit)
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\roundtrip\NullParametersRoundtripTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\roundtrip\PixelDensityRoundtrip.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\roundtrip\RoundtripBase.java (1 hit)
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\roundtrip\TestImages.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
Search "java.awt" (0 hits in 0 files of 1 searched)
</details>
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a change in pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on a change in pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#discussion_r785353746
##########
File path: src/main/java/org/apache/commons/imaging/ImageFormat.java
##########
@@ -33,6 +34,7 @@
*
* @return String extension
*/
- String getExtension();
+ String[] getExtensions();
Review comment:
Tentative fix version set to alpha 3, or it might be done for 1.0 final release: https://issues.apache.org/jira/browse/IMAGING-322
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a change in pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on a change in pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#discussion_r785353990
##########
File path: src/main/java/org/apache/commons/imaging/ImageParser.java
##########
@@ -563,27 +561,22 @@ public final BufferedImage getBufferedImage(final File file, final Map<String, O
* <p>The params argument provides a mechanism for individual
* implementations to pass optional information into the parser.
* Not all formats will support this capability. Currently,
- * some of the parsers do not check for null arguments. So in cases
- * where no optional specifications are supported, application
- * code should pass in an empty instance of an implementation of
- * the map interface (i.e. an empty HashMap).
+ * some of the parsers do not check for null arguments.</p>
Review comment:
Done! https://issues.apache.org/jira/browse/IMAGING-323
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] darkma773r commented on a change in pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
darkma773r commented on a change in pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#discussion_r785343858
##########
File path: src/main/java/org/apache/commons/imaging/internal/Util.java
##########
@@ -0,0 +1,54 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.imaging.internal;
+
+import java.util.function.Predicate;
+import java.util.function.Supplier;
+
+import org.apache.commons.imaging.ImageFormat;
+import org.apache.commons.imaging.ImageParser;
+import org.apache.commons.imaging.ImagingParameters;
+
+/**
+ * Internal utilities.
+ *
+ * @since 1.0-alpha3
+ */
+public class Util {
+
+ private Util() {}
+
+ public static <T extends ImagingParameters> ImageParser<T> getImageParser(ImageFormat format) {
+ return getImageParser((parser) -> parser.canAcceptType(format), () -> new RuntimeException("Unknown Format: " + format));
+ }
+
+ public static <T extends ImagingParameters> ImageParser<T> getImageParser(String fileExtension) {
+ return getImageParser((parser) -> parser.canAcceptExtension(fileExtension), () -> new RuntimeException("Unknown Extension: " + fileExtension));
+ }
+
+ // This generics suppression is as good as the predicate given. If the predicate violates a generics design,
+ // then there will be an error during runtime.
+ @SuppressWarnings("unchecked")
+ private static <T extends ImagingParameters> ImageParser<T> getImageParser(Predicate<ImageParser<?>> pred, Supplier<? extends RuntimeException> supl) {
+ return (ImageParser<T>) ImageParser
+ .getAllImageParsers()
+ .stream()
+ .filter((parser) -> pred.test(parser))
+ .findFirst()
Review comment:
The generic parameter T is actually not needed with the way this method is being used. Removing the `<T extends ImagingParameters>` part of the signatures here and just returning `ImageParser<?>` for all of them removes the need for the warning suppression.
##########
File path: src/main/java/org/apache/commons/imaging/ImageParser.java
##########
@@ -563,27 +561,22 @@ public final BufferedImage getBufferedImage(final File file, final Map<String, O
* <p>The params argument provides a mechanism for individual
* implementations to pass optional information into the parser.
* Not all formats will support this capability. Currently,
- * some of the parsers do not check for null arguments. So in cases
- * where no optional specifications are supported, application
- * code should pass in an empty instance of an implementation of
- * the map interface (i.e. an empty HashMap).
+ * some of the parsers do not check for null arguments.</p>
Review comment:
Should there be a future issue to make sure that all parsers check for null?
##########
File path: src/main/java/org/apache/commons/imaging/Imaging.java
##########
@@ -50,62 +48,40 @@
*
* <h3>Using this class</h3>
*
- * <p>
- * Almost all of the Apache Commons Imaging library's core functionality can
+ * <p>Almost all of the Apache Commons Imaging library's core functionality can
* be accessed through the methods provided by this class.
* The use of the Imaging class is similar to the Java API's ImageIO class,
- * though Imaging supports formats and options not included in the standard
- * Java API.
- * </p>
+ * though Imaging supports formats not included in the standard Java API.</p>
*
- * <p>
- * All of methods provided by the Imaging class are declared static.
- * </p>
+ * <p>All of methods provided by the Imaging class are declared static.</p>
*
- * <p>
- * The Apache Commons Imaging package is a pure Java implementation.
- * </p>
+ * <p>The Apache Commons Imaging package is a pure Java implementation.</p>
*
* <h3>Format support</h3>
*
- * <p>
- * While the Apache Commons Imaging package handles a number of different
+ * <p>While the Apache Commons Imaging package handles a number of different
* graphics formats, support for some formats is not yet complete.
* For the most recent information on support for specific formats, refer to
* <a href="https://commons.apache.org/imaging/formatsupport.html">Format Support</a>
- * at the main project development web site.
- * </p>
+ * at the main project development web site.</p>
*
* <h3>Optional parameters for image reading and writing</h3>
*
- * <p>
- * Some of the methods provided by this class accept an optional
- * <strong>params</strong> argument that permits the application to specify
- * elements for special handling. If these specifications are not required by
- * the application, the params argument may be omitted (as appropriate) or
- * a null argument may be provided. In image-writing operations, the option
- * parameters may include options such as data-compression type (if any),
- * color model, or other format-specific data representations. The parameters
- * map may also be used to provide EXIF Tags and other metadata to those
- * formats that support them. In image-reading operations,
- * the parameters may include information about special handling in reading
- * the image data.
- * </p>
+ * <p>Many of the operations provided in this class as static calls can be accessed directly
+ * using {@link ImageParser}'s. It is, however, difficult to design an API that is generic
+ * enough so users can safely read or write byte arrays when specifying the type of parameters,
+ * since the only way to confirm the parameters are valid for a specific format is by reading
+ * the byte array (when it is already too late.)</p>
Review comment:
I like this design. I think it will provide a good base for future extensions.
I don't see a need to mention the design difficulties in the docs, though. The paragraph here could just be
```
Many of the operations provided in this class as static calls can be accessed directly
using format-specific {@link ImageParser} instances. These static methods are provided
for convenience in simple use cases.
```
This leads right into the next paragraph, in which case the two could be combined.
##########
File path: src/main/java/org/apache/commons/imaging/ImageParser.java
##########
@@ -955,18 +944,15 @@ protected BufferedImageFactory getBufferedImageFactory(final Map<String, Object>
/**
* A utility method to search a params specification and determine
- * whether it contains the ImagingConstants.PARAM_KEY_STRICT
- * specification. Intended
+ * whether it contains the parameters contain the strict flag. Intended
* for internal use by ImageParser implementations.
*
- * @param params A valid Map object (or a null).
+ * @param params optional parameters.
* @return If the params specify strict format compliance, true;
* otherwise, false.
+ * @param <T> type of parameters used by this image parser
*/
- public static boolean isStrict(final Map<String, Object> params) {
- if (params == null || !params.containsKey(ImagingConstants.PARAM_KEY_STRICT)) {
- return false;
- }
- return ((Boolean) params.get(ImagingConstants.PARAM_KEY_STRICT)).booleanValue();
+ public static <T extends ImagingParameters> boolean isStrict(final T params) {
+ return params.isStrict();
Review comment:
This method doesn't seem very useful anymore, especially since I only see 2 places where it is used. Remove?
##########
File path: src/changes/changes.xml
##########
@@ -45,6 +45,21 @@ The <action> type attribute can be add,update,fix,remove.
</properties>
<body>
<release version="1.0-alpha3" date="2020-??-??" description="Third 1.0 alpha release">
+ <action issue="IMAGING-159" dev="kinow" type="fix" due-to="Bruno P. Kinoshita, Gary Lucas, Matt Juntunen">
Review comment:
No problem!
##########
File path: src/main/java/org/apache/commons/imaging/internal/Util.java
##########
@@ -0,0 +1,73 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.imaging.internal;
+
+import org.apache.commons.imaging.ImageFormat;
+import org.apache.commons.imaging.ImageFormats;
+import org.apache.commons.imaging.ImageParser;
+import org.apache.commons.imaging.Imaging;
+import org.apache.commons.imaging.ImagingParameters;
+import org.apache.commons.imaging.common.bytesource.ByteSource;
+
+import java.io.IOException;
+import java.util.function.Predicate;
+import java.util.function.Supplier;
+
+/**
+ * Internal utilities.
+ *
+ * @since 1.0-alpha3
+ */
+public class Util {
+
+ private Util() {}
+
+ public static <T extends ImagingParameters> ImageParser<T> getImageParser(ImageFormat format) {
+ return getImageParser((parser) -> parser.canAcceptType(format), () -> new RuntimeException("Unknown Format: " + format));
+ }
+
+ public static <T extends ImagingParameters> ImageParser<T> getImageParser(String fileExtension) {
+ return getImageParser((parser) -> parser.canAcceptExtension(fileExtension), () -> new RuntimeException("Unknown Extension: " + fileExtension));
Review comment:
Should probably use `IllegalArgumentException` for consistency with line 71. Also applies to line 40.
##########
File path: src/main/java/org/apache/commons/imaging/ImageFormat.java
##########
@@ -33,6 +34,7 @@
*
* @return String extension
*/
- String getExtension();
+ String[] getExtensions();
Review comment:
Sounds good to me!
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] coveralls edited a comment on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-841637226
[](https://coveralls.io/builds/41446433)
Coverage decreased (-0.2%) to 76.56% when pulling **75831b95c9a25ff0d7b09a412bb323ca6a11c20e on kinow:IMAGING-159-1** into **a87b545e1019b43bfbe31fa5df3a4f3095a8b438 on apache:master**.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] gwlucastrig commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
gwlucastrig commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-841791184
It looks like providing compatibility with Android OS may not be practical.
Android has its own graphics and media classes. So anything with java.awt
in it will not port. Common Imaging using BufferedImage extensively. Most
other AWT elements are used only a little. Color shows up the most. The
ICC_ColorSpace also shows up. A couple of the test applications use
Graphics to draw some test images as input.
I think I might post a question on Java Coderanch or Stackoverflow to ask
about compatible solutions.
On Sat, May 15, 2021 at 8:18 AM Bruno P. Kinoshita ***@***.***>
wrote:
> If those classes are from awt, the issue is that we have a dependency to
> java.desktop (I think that's the java module name) and also cause an issue
> to users of the API that are using Android (as IIUC, there's no awt in
> Android's Java SDK?).
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/commons-imaging/pull/116#issuecomment-841649340>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AEWJDYNES4M3XQCOJIEZN6TTNZRAXANCNFSM4WPUJSLQ>
> .
>
Search "java.awt" (229 hits in 121 files of 557 searched)
commons-imaging\src\main\java\org\apache\commons\imaging\ColorTools.java (11 hits)
Line 20: import java.awt.RenderingHints;
Line 21: import java.awt.Transparency;
Line 22: import java.awt.color.ColorSpace;
Line 23: import java.awt.color.ICC_ColorSpace;
Line 24: import java.awt.color.ICC_Profile;
Line 25: import java.awt.image.BufferedImage;
Line 26: import java.awt.image.ColorConvertOp;
Line 27: import java.awt.image.ColorModel;
Line 28: import java.awt.image.ComponentColorModel;
Line 29: import java.awt.image.DirectColorModel;
Line 30: import java.awt.image.ImagingOpException;
commons-imaging\src\main\java\org\apache\commons\imaging\common\BufferedImageFactory.java (1 hit)
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\common\ImageBuilder.java (9 hits)
Line 43: import java.awt.color.ColorSpace;
Line 44: import java.awt.image.BufferedImage;
Line 45: import java.awt.image.ColorModel;
Line 46: import java.awt.image.DataBuffer;
Line 47: import java.awt.image.DataBufferInt;
Line 48: import java.awt.image.DirectColorModel;
Line 49: import java.awt.image.Raster;
Line 50: import java.awt.image.RasterFormatException;
Line 51: import java.awt.image.WritableRaster;
commons-imaging\src\main\java\org\apache\commons\imaging\common\RgbBufferedImageFactory.java (1 hit)
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\common\SimpleBufferedImageFactory.java (1 hit)
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\bmp\BmpImageParser.java (2 hits)
Line 27: import java.awt.Dimension;
Line 28: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\bmp\BmpWriter.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\bmp\BmpWriterPalette.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\bmp\BmpWriterRgb.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\dcx\DcxImageParser.java (2 hits)
Line 23: import java.awt.Dimension;
Line 24: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\gif\GifImageParser.java (2 hits)
Line 28: import java.awt.Dimension;
Line 29: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\icns\IcnsDecoder.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\icns\IcnsImageParser.java (2 hits)
Line 23: import java.awt.Dimension;
Line 24: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\ico\IcoImageParser.java (2 hits)
Line 26: import java.awt.Dimension;
Line 27: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\jpeg\decoder\JpegDecoder.java (6 hits)
Line 21: import java.awt.image.BufferedImage;
Line 22: import java.awt.image.ColorModel;
Line 23: import java.awt.image.DataBuffer;
Line 24: import java.awt.image.DirectColorModel;
Line 25: import java.awt.image.Raster;
Line 26: import java.awt.image.WritableRaster;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\jpeg\JpegImageMetadata.java (2 hits)
Line 19: import java.awt.Dimension;
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\jpeg\JpegImageParser.java (2 hits)
Line 23: import java.awt.Dimension;
Line 24: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\pcx\PcxImageParser.java (11 hits)
Line 24: import java.awt.Dimension;
Line 25: import java.awt.Transparency;
Line 26: import java.awt.color.ColorSpace;
Line 27: import java.awt.image.BufferedImage;
Line 28: import java.awt.image.ColorModel;
Line 29: import java.awt.image.ComponentColorModel;
Line 30: import java.awt.image.DataBuffer;
Line 31: import java.awt.image.DataBufferByte;
Line 32: import java.awt.image.IndexColorModel;
Line 33: import java.awt.image.Raster;
Line 34: import java.awt.image.WritableRaster;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\pcx\PcxWriter.java (1 hit)
Line 18: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\png\PngImageParser.java (6 hits)
Line 25: import java.awt.Dimension;
Line 26: import java.awt.color.ColorSpace;
Line 27: import java.awt.color.ICC_ColorSpace;
Line 28: import java.awt.color.ICC_Profile;
Line 29: import java.awt.image.BufferedImage;
Line 30: import java.awt.image.ColorModel;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\png\PngWriter.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\png\ScanExpediter.java (1 hit)
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\png\ScanExpediterInterlaced.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\png\ScanExpediterSimple.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\pnm\PamWriter.java (1 hit)
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\pnm\PbmWriter.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\pnm\PgmWriter.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\pnm\PnmImageParser.java (2 hits)
Line 22: import java.awt.Dimension;
Line 23: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\pnm\PnmWriter.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\pnm\PpmWriter.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\psd\dataparsers\DataParser.java (2 hits)
Line 19: import java.awt.image.BufferedImage;
Line 20: import java.awt.image.DataBuffer;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\psd\datareaders\CompressedDataReader.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\psd\datareaders\DataReader.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\psd\datareaders\UncompressedDataReader.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\psd\PsdImageParser.java (2 hits)
Line 26: import java.awt.Dimension;
Line 27: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\rgbe\RgbeImageParser.java (10 hits)
Line 19: import java.awt.Dimension;
Line 20: import java.awt.Point;
Line 21: import java.awt.Transparency;
Line 22: import java.awt.color.ColorSpace;
Line 23: import java.awt.image.BandedSampleModel;
Line 24: import java.awt.image.BufferedImage;
Line 25: import java.awt.image.ComponentColorModel;
Line 26: import java.awt.image.DataBuffer;
Line 27: import java.awt.image.DataBufferFloat;
Line 28: import java.awt.image.Raster;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\tiff\datareaders\DataReaderStrips.java (1 hit)
Line 20: import java.awt.Rectangle;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\tiff\datareaders\DataReaderTiled.java (1 hit)
Line 26: import java.awt.Rectangle;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\tiff\datareaders\ImageDataReader.java (1 hit)
Line 32: import java.awt.Rectangle;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\tiff\photometricinterpreters\floatingpoint\PaletteEntry.java (1 hit)
Line 19: import java.awt.Color;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\tiff\photometricinterpreters\floatingpoint\PaletteEntryForRange.java (1 hit)
Line 19: import java.awt.Color;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\tiff\photometricinterpreters\floatingpoint\PaletteEntryForValue.java (1 hit)
Line 19: import java.awt.Color;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\tiff\photometricinterpreters\floatingpoint\PhotometricInterpreterFloat.java (1 hit)
Line 19: import java.awt.Color;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\tiff\TiffDirectory.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\tiff\TiffImageMetadata.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\tiff\TiffImageParser.java (3 hits)
Line 28: import java.awt.Dimension;
Line 29: import java.awt.Rectangle;
Line 30: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\tiff\write\TiffImageWriterBase.java (2 hits)
Line 33: import java.awt.image.BufferedImage;
Line 34: import java.awt.image.ColorModel;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\wbmp\WbmpImageParser.java (7 hits)
Line 21: import java.awt.Dimension;
Line 22: import java.awt.image.BufferedImage;
Line 23: import java.awt.image.DataBuffer;
Line 24: import java.awt.image.DataBufferByte;
Line 25: import java.awt.image.IndexColorModel;
Line 26: import java.awt.image.Raster;
Line 27: import java.awt.image.WritableRaster;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\xbm\XbmImageParser.java (8 hits)
Line 19: import java.awt.Dimension;
Line 20: import java.awt.image.BufferedImage;
Line 21: import java.awt.image.ColorModel;
Line 22: import java.awt.image.DataBuffer;
Line 23: import java.awt.image.DataBufferByte;
Line 24: import java.awt.image.IndexColorModel;
Line 25: import java.awt.image.Raster;
Line 26: import java.awt.image.WritableRaster;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\xpm\XpmImageParser.java (8 hits)
Line 18: import java.awt.Dimension;
Line 19: import java.awt.image.BufferedImage;
Line 20: import java.awt.image.ColorModel;
Line 21: import java.awt.image.DataBuffer;
Line 22: import java.awt.image.DirectColorModel;
Line 23: import java.awt.image.IndexColorModel;
Line 24: import java.awt.image.Raster;
Line 25: import java.awt.image.WritableRaster;
commons-imaging\src\main\java\org\apache\commons\imaging\icc\IccProfileParser.java (1 hit)
Line 23: import java.awt.color.ICC_Profile;
commons-imaging\src\main\java\org\apache\commons\imaging\ImageDump.java (4 hits)
Line 19: import java.awt.color.ColorSpace;
Line 20: import java.awt.color.ICC_ColorSpace;
Line 21: import java.awt.color.ICC_Profile;
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\ImageParser.java (2 hits)
Line 19: import java.awt.Dimension;
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\Imaging.java (3 hits)
Line 22: import java.awt.Dimension;
Line 23: import java.awt.color.ICC_Profile;
Line 24: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\internal\Debug.java (1 hit)
Line 19: import java.awt.color.ICC_Profile;
commons-imaging\src\main\java\org\apache\commons\imaging\palette\Dithering.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\palette\MedianCutQuantizer.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\palette\PaletteFactory.java (3 hits)
Line 19: import java.awt.color.ColorSpace;
Line 20: import java.awt.image.BufferedImage;
Line 21: import java.awt.image.ColorModel;
commons-imaging\src\test\java\org\apache\commons\imaging\common\bytesource\ByteSourceImageTest.java (2 hits)
Line 24: import java.awt.Dimension;
Line 25: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\common\bytesource\ByteSourceInputStreamTest.java (1 hit)
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\common\ImageBuilderTest.java (3 hits)
Line 19: import java.awt.image.BufferedImage;
Line 20: import java.awt.image.ColorModel;
Line 21: import java.awt.image.RasterFormatException;
commons-imaging\src\test\java\org\apache\commons\imaging\common\RgbBufferedImageFactoryTest.java (1 hit)
Line 22: import java.awt.image.DirectColorModel;
commons-imaging\src\test\java\org\apache\commons\imaging\examples\ApacheImagingSpeedAndMemoryTest.java (1 hit)
Line 156: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\examples\ImageReadExample.java (5 hits)
Line 19: import java.awt.GraphicsConfiguration;
Line 20: import java.awt.GraphicsDevice;
Line 21: import java.awt.GraphicsEnvironment;
Line 22: import java.awt.Transparency;
Line 23: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\examples\ImageWriteExample.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\examples\SampleUsage.java (3 hits)
Line 19: import java.awt.Dimension;
Line 20: import java.awt.color.ICC_Profile;
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\examples\tiff\ExampleReadFloatingPointData.java (2 hits)
Line 19: import java.awt.Color;
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\examples\tiff\ReadAndRenderFloatingPoint.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\examples\tiff\ReadTagsAndImages.java (1 hit)
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\bmp\BmpReadTest.java (1 hit)
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\bmp\BmpRoundtripTest.java (1 hit)
Line 23: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\bmp\BmpWriterRgbTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\dcx\DcxReadTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\gif\GifReadTest.java (1 hit)
Line 24: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\icns\IcnsReadTest.java (1 hit)
Line 23: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\icns\IcnsRoundTripTest.java (1 hit)
Line 23: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\ico\IcoReadTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\ico\IcoRoundtripTest.java (1 hit)
Line 23: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\jpeg\exif\MicrosoftTagTest.java (1 hit)
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\jpeg\iptc\IptcFullDiscardTest.java (2 hits)
Line 20: import java.awt.Graphics2D;
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\jpeg\JpegReadTest.java (1 hit)
Line 23: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\jpeg\JpegWithJpegThumbnailTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\jpeg\specific\JpegImageParserTest.java (1 hit)
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\pam\PamReadTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\pcx\PcxReadTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\png\ConvertPngToGifTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\png\PngMultipleRoundtripTest.java (1 hit)
Line 23: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\png\PngReadTest.java (1 hit)
Line 23: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\png\PngTextTest.java (2 hits)
Line 24: import java.awt.Color;
Line 25: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\png\PngWriteForceTrueColorText.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\png\PngWriteReadTest.java (1 hit)
Line 25: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\pnm\PnmImageParserTest.java (2 hits)
Line 23: import java.awt.image.BufferedImage;
Line 24: import java.awt.image.DataBufferInt;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\psd\PsdReadTest.java (1 hit)
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\rgbe\RgbeReadTest.java (1 hit)
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\tiff\photometricinterpreters\floatingpoint\PaletteEntryForRangeTest.java (1 hit)
Line 24: import java.awt.Color;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\tiff\photometricinterpreters\floatingpoint\PaletteEntryForValueTest.java (1 hit)
Line 24: import java.awt.Color;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\tiff\photometricinterpreters\floatingpoint\PhotometricInterpreterFloatTest.java (1 hit)
Line 23: import java.awt.Color;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\tiff\TiffAlphaRoundTripTest.java (4 hits)
Line 19: import java.awt.AlphaComposite;
Line 20: import java.awt.Color;
Line 21: import java.awt.Graphics2D;
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\tiff\TiffCcittTest.java (1 hit)
Line 23: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\tiff\TiffFloatingPointReadTest.java (2 hits)
Line 23: import java.awt.Color;
Line 24: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\tiff\TiffFloatingPointRoundTripTest.java (1 hit)
Line 23: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\tiff\TiffReadAlphaTest.java (3 hits)
Line 19: import java.awt.Color;
Line 20: import java.awt.Graphics2D;
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\tiff\TiffReadTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\tiff\TiffRoundtripTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\tiff\TiffSubImageTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\wbmp\WbmpReadTest.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\xbm\XbmReadTest.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\xmp\XmpUpdateTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\xpm\XpmReadTest.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\ImageDumpTest.java (2 hits)
Line 21: import java.awt.color.ColorSpace;
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\palette\DitheringTest.java (1 hit)
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\palette\PaletteQuantizationTest.java (1 hit)
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\roundtrip\BitmapRoundtripTest.java (1 hit)
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\roundtrip\FullColorRoundtrip.java (1 hit)
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\roundtrip\GrayscaleRountripTest.java (1 hit)
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\roundtrip\ImageAsserts.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\roundtrip\LimitedColorRoundtripTest.java (1 hit)
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\roundtrip\NullParametersRoundtripTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\roundtrip\PixelDensityRoundtrip.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\roundtrip\RoundtripBase.java (1 hit)
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\roundtrip\TestImages.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
Search "java.awt" (0 hits in 0 files of 1 searched)
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] gwlucastrig commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
gwlucastrig commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-783319821
Hey Bruno,
Anything I can do to help with this?
Gary
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow edited a comment on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow edited a comment on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-841605341
@gwlucastrig
>great suggestion from @gwlucastrig : "use one call for setSubImage, rather than 4 separate calls for setSubImageX, Y, Width, Height."
I started making this change in the `TiffImagingParameter`. First the `public void setSubImage(Integer width, Integer height, Integer x, Integer y)`. All good so far.
But when I finished and started adding the getter, I was thinking in Python or JS, and wanted to return all the 4 parameters at once :disappointed_relieved: Then I realized I would have to create another class to return the 4 parameters… do you have any suggestion for the getter of the subimage in the tiff imaging parameter class?
Or would it be OK to return each parameter separately?
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow edited a comment on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow edited a comment on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-826703877
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-824644871
Now the code in `src/main` is using the `ImagingParameters` and subclasses. 🎉
If I have time tomorrow or over the weekend, will go through the pending items. After that it should be ready for review.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-961796289
That is a good point @gwlucastrig , and I have no objections to massaging the code of the `Imaging` class. I will spend some time thinking about that, and seeing if/when the parameters would be useful or not. 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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-963088960
Agreed @darkma773r , +1 on @gwlucastrig suggestion too. Let's review Imaging class. Will start thinking on what/how to change 👍
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a change in pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on a change in pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#discussion_r673020403
##########
File path: src/main/java/org/apache/commons/imaging/ImageParser.java
##########
@@ -79,25 +78,18 @@
* that the documentation is perfect, especially in the more obscure
* and specialized areas of implementation.
*
- * <h3>The "Map params" argument</h3>
+ * <h3>The "params" argument</h3>
*
- * Many of the methods specified by this class accept an argument of
- * type Map giving a list of parameters to be used when processing an
- * image. For example, some of the output formats permit the specification
+ * <p>Many of the methods specified by this class accept an argument of
+ * type {@code T} defining the parameters to be used when
+ * processing an image. For example, some of the output formats permit
* of different kinds of image compression or color models. Some of the
* reading methods permit the calling application to require strict
- * format compliance. In many cases, however, an application will not
- * require the use of this argument. While some of the ImageParser
- * implementations check for (and ignore) null arguments for this parameter,
- * not all of them do (at least not at the time these notes were written).
- * Therefore, a prudent programmer will always supply an valid, though
- * empty instance of a Map implementation when calling such methods.
- * Generally, the java HashMap class is useful for this purpose.
+ * format compliance.</p>
*
- * <p>Additionally, developers creating or enhancing classes derived
- * from ImageParser are encouraged to include such checks in their code.
+ * @param <T> type of parameters used by this image parser
*/
-public abstract class ImageParser extends BinaryFileParser {
+public abstract class ImageParser<T extends ImagingParameters> extends BinaryFileParser {
Review comment:
I remember trying that.. now I'm in the process of trying that again, and I think I've hit the same wall as in the last time.
The `ImageParser` has some methods like
```java
public abstract BufferedImage getBufferedImage(ByteSource byteSource, ImagingParameters params)
throws ImageReadException, IOException;
```
Then in `BmpImageParser`, for example, when I try
```java
@Override
public BufferedImage getBufferedImage(final ByteSource byteSource, final BmpImagingParameters params)
throws ImageReadException, IOException {
```
It fails due to the `Override` annotation, with `Method does not override method from its superclass`. If I rename the parameter to `ImagingParameter`, that compiles. But then we could have users sending a `JpegImagingParameters` instead, and it would fail in runtime.
I think this is where I stopped last time, and thought I would have to either accept `ImagingParameters` everywhere, or use generics... any advice @darkma773r ?
Last item pending in this review :tada:
Thanks!!!
Bruno
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] coveralls edited a comment on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-841637226
[](https://coveralls.io/builds/41446323)
Coverage decreased (-0.2%) to 76.563% when pulling **622e0b80936e5fa4ae56d426771c30404f578cce on kinow:IMAGING-159-1** into **a87b545e1019b43bfbe31fa5df3a4f3095a8b438 on apache:master**.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a change in pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on a change in pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#discussion_r777275935
##########
File path: src/main/java/org/apache/commons/imaging/ImageFormat.java
##########
@@ -33,6 +34,7 @@
*
* @return String extension
*/
- String getExtension();
+ String[] getExtensions();
Review comment:
I started changing it, but later in the ImageParser implementations they are returning arrays for accepted extensions too. The commit started to grow a little too much, so I thought it'd be best to move it to a follow-up issue/discussion :+1:
##########
File path: src/main/java/org/apache/commons/imaging/ImageFormats.java
##########
@@ -16,40 +16,68 @@
*/
package org.apache.commons.imaging;
+import org.apache.commons.imaging.formats.bmp.BmpImagingParameters;
+import org.apache.commons.imaging.formats.gif.GifImagingParameters;
+import org.apache.commons.imaging.formats.icns.IcnsImagingParameters;
+import org.apache.commons.imaging.formats.ico.IcoImagingParameters;
+import org.apache.commons.imaging.formats.jpeg.JpegImagingParameters;
+import org.apache.commons.imaging.formats.pcx.PcxImagingParameters;
+import org.apache.commons.imaging.formats.png.PngImagingParameters;
+import org.apache.commons.imaging.formats.pnm.PnmImagingParameters;
+import org.apache.commons.imaging.formats.psd.PsdImagingParameters;
+import org.apache.commons.imaging.formats.rgbe.RgbeImagingParameters;
+import org.apache.commons.imaging.formats.tiff.TiffImagingParameters;
+import org.apache.commons.imaging.formats.wbmp.WbmpImagingParameters;
+import org.apache.commons.imaging.formats.xbm.XbmImagingParameters;
+import org.apache.commons.imaging.formats.xpm.XpmImagingParameters;
+
+import java.util.function.Supplier;
+
/**
* Enum of known image formats.
*/
public enum ImageFormats implements ImageFormat {
- UNKNOWN,
- BMP,
- DCX,
- GIF,
- ICNS,
- ICO,
- JBIG2,
- JPEG,
- PAM,
- PSD,
- PBM,
- PGM,
- PNM,
- PPM,
- PCX,
- PNG,
- RGBE,
- TGA,
- TIFF,
- WBMP,
- XBM,
- XPM;
+ UNKNOWN(null),
+ BMP(BmpImagingParameters::new, "bmp", "dib"),
Review comment:
Good spot! Removed!
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-1003833509
Rebased and addressed some of the excellent feedback. Thanks @darkma773r !
I'm looking at the JPEG & TIFF relationship issue as that's easier to fix. Then I think we will have the last issue, of ClassCastException, and the public methods of `Imaging`.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-1007768801
@gwlucastrig , @darkma773r I believe this is ready for review again. And I think it is really close to being ready to be merged! :tada:
@darkma773r I've removed the `Imaging` public methods that used `ImagingParameter`'s (and `Hashmap`'s before). This reduced the amount of public methods in `Imaging`, and later we can decide whether to add back methods with Parameter factories/builders/mergers/etc, or create builders in each parsers, or improve the parsers' API'...
After rebasing, there was a bit of Javadoc errors, probably from recent plug-in versions. Fixed those too, reviewed the code removing unnecessary changes, and finally added the `changes.xml` entry. With this, leaving the 126 modified files for you guys to review :smile: (sorry!)
Thanks a lot for the help,
Bruno
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a change in pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on a change in pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#discussion_r760907698
##########
File path: src/main/java/org/apache/commons/imaging/ImageParser.java
##########
@@ -898,7 +892,7 @@ public boolean canAcceptType(final ImageFormat type) {
* @param file An valid file reference.
* @return If the parser can accept the format, true; otherwise, false.
*/
- protected final boolean canAcceptExtension(final File file) {
+ public boolean canAcceptExtension(final File file) {
Review comment:
Made it public because I created methods in the new internal `Util` class. Similar to the already public `canAcceptType(ImageFormat)` of this class.
##########
File path: src/main/java/org/apache/commons/imaging/ImageParser.java
##########
@@ -123,12 +115,18 @@
new TiffImageParser(),
new WbmpImageParser(),
new XbmImageParser(),
- new XpmImageParser(),
+ new XpmImageParser()
// new JBig2ImageParser(),
// new TgaImageParser(),
- };
+ );
}
+ /**
+ * Get a default parameters instance for this parser.
+ * @return default parameters instance
+ */
+ public abstract T getDefaultParameters();
Review comment:
Copied from @darkma773r 's PR. By moving it here from the `ImageFormat` (i.e. responsible for defining the default parameter is the parser, not the format) several generics warnings don't happen anymore.
##########
File path: src/main/java/org/apache/commons/imaging/internal/Util.java
##########
@@ -0,0 +1,54 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.imaging.internal;
+
+import java.util.function.Predicate;
+import java.util.function.Supplier;
+
+import org.apache.commons.imaging.ImageFormat;
+import org.apache.commons.imaging.ImageParser;
+import org.apache.commons.imaging.ImagingParameters;
+
+/**
+ * Internal utilities.
+ *
+ * @since 1.0-alpha3
+ */
+public class Util {
+
+ private Util() {}
+
+ public static <T extends ImagingParameters> ImageParser<T> getImageParser(ImageFormat format) {
+ return getImageParser((parser) -> parser.canAcceptType(format), () -> new RuntimeException("Unknown Format: " + format));
+ }
+
+ public static <T extends ImagingParameters> ImageParser<T> getImageParser(String fileExtension) {
+ return getImageParser((parser) -> parser.canAcceptExtension(fileExtension), () -> new RuntimeException("Unknown Extension: " + fileExtension));
+ }
+
+ // This generics suppression is as good as the predicate given. If the predicate violates a generics design,
+ // then there will be an error during runtime.
+ @SuppressWarnings("unchecked")
+ private static <T extends ImagingParameters> ImageParser<T> getImageParser(Predicate<ImageParser<?>> pred, Supplier<? extends RuntimeException> supl) {
+ return (ImageParser<T>) ImageParser
+ .getAllImageParsers()
+ .stream()
+ .filter((parser) -> pred.test(parser))
+ .findFirst()
Review comment:
This is the only place where we have an issue with the generics used in this PR now. The `Predicate` given to this function should return `true` only if the parser matches something like a file format, file extension, etc.
Assuming the predicate is correct, there should be no runtime errors for the parameters :+1: that's the best we can achieve with Java generics IMO
##########
File path: src/main/java/org/apache/commons/imaging/Imaging.java
##########
@@ -16,9 +16,6 @@
*/
package org.apache.commons.imaging;
-import static org.apache.commons.imaging.ImagingConstants.PARAM_KEY_FILENAME;
-import static org.apache.commons.imaging.ImagingConstants.PARAM_KEY_FORMAT;
Review comment:
N.B. no generics warnings being suppressed in `Imaging` now :tada:
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-958618064
>> If the JpegImageParser requires a parameter different than the TiffImageParser, I assume we would have to create a new JpegImagingParameter. In that case, wouldn't we have to keep the old constructor for backward compatibility until a new major release?
>
>I'm not totally sure what you mean here. Can you give an example?
Sorry, I wasn't very clear. At the moment we have `JpegImageParser parser = new JpegImageParser();`. Internally, the `JpegImageParser` is using a `TiffImagingParameters`.
But if we need to add a parameter that is used only by the `JpegImageParser`, I think we would want to have a `JpegImagingParameters`. In which case I am not sure if we would be able to maintain backward compatibility by replacing the `TiffImagingParameters` in the `JpegImageParser`.
>since it makes it it kind of hard to tell what parameters to use when creating images of a certain type.
:point_up: this is my main concern. Avoid users having to guess what's the parameter that they want to use when parsing a Jpeg, Tiff, Gif, etc. Preferably something that their IDE's can assist with, using auto-complete for example.
>One idea for making this readily available in the API would be to add convenience factory methods in Imaging for creating instances of each parameters type. Ex:
Possibly another design to consider/experiment :-)
>Side note: Is there a reason the JpegImageParser uses TiffImageParameters? The two formats are not related, correct? Perhaps we could have a common base class for them?
We can probably avoid that. I agree it doesn't make much sense for the parameters to have a relationship like that. Currently, [`JpegImageParser`](https://github.com/apache/commons-imaging/blob/0ccabc3416cecf25cdec90a522f0912baf19bc58/src/main/java/org/apache/commons/imaging/formats/jpeg/JpegImageParser.java#L751) has some code in common with the `TiffImageParser` for parsing EXIF metadata. I don't recall 100%, but I believe that's why I implemented everything in the `TiffImagingParameters` and, realizing the Jpeg parameters were the same, made the parent-child class relantionship.
>Is the idea that it is confusing on what properties are copied over?
Not at all, that part is clear and looks OK. It's with users being able to use the wrong parameter class by accident, and realizing only in runtime iff a runtime exception is thrown (or in the worst case, no exception is thrown and the program behaves in a wrong way; debugging an issue like that can be annoying I think).
>I attempted to retain the null parameter functionality. In the normalizeParameters method, if a null parameter is passed, a default instance is created and returned, allowing downstream code to bypass null checks. mvn clean install passes with my current setup. Is there a unit test I missed?
Ah, my bad! Your solution works with the method I was looking at. I wrote this one before looking at the method that creates the default parameters :+1:
I need to find some time to “get in the zone” and try a few things. But it would be, mainly, to prevent users being able to use the wrong parameter types by accident, and making it so that we can add/remove parameters (deprecating what was removed) in the 1.x release series without worrying about backward compatibility. If we are able to modify your PR again to accommodate these two requirements, I think we would have the perfect solution :-)
Thanks a lot!
Bruno
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a change in pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on a change in pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#discussion_r673020403
##########
File path: src/main/java/org/apache/commons/imaging/ImageParser.java
##########
@@ -79,25 +78,18 @@
* that the documentation is perfect, especially in the more obscure
* and specialized areas of implementation.
*
- * <h3>The "Map params" argument</h3>
+ * <h3>The "params" argument</h3>
*
- * Many of the methods specified by this class accept an argument of
- * type Map giving a list of parameters to be used when processing an
- * image. For example, some of the output formats permit the specification
+ * <p>Many of the methods specified by this class accept an argument of
+ * type {@code T} defining the parameters to be used when
+ * processing an image. For example, some of the output formats permit
* of different kinds of image compression or color models. Some of the
* reading methods permit the calling application to require strict
- * format compliance. In many cases, however, an application will not
- * require the use of this argument. While some of the ImageParser
- * implementations check for (and ignore) null arguments for this parameter,
- * not all of them do (at least not at the time these notes were written).
- * Therefore, a prudent programmer will always supply an valid, though
- * empty instance of a Map implementation when calling such methods.
- * Generally, the java HashMap class is useful for this purpose.
+ * format compliance.</p>
*
- * <p>Additionally, developers creating or enhancing classes derived
- * from ImageParser are encouraged to include such checks in their code.
+ * @param <T> type of parameters used by this image parser
*/
-public abstract class ImageParser extends BinaryFileParser {
+public abstract class ImageParser<T extends ImagingParameters> extends BinaryFileParser {
Review comment:
I remember trying that.. now I'm in the process of trying that again, and I think I've hit the same wall as in the last time.
The `ImageParser` has some methods like
```java
public abstract BufferedImage getBufferedImage(ByteSource byteSource, ImagingParameters params)
throws ImageReadException, IOException;
```
Then in `BmpImageParser`, for example, when I try
```java
@Override
public BufferedImage getBufferedImage(final ByteSource byteSource, final BmpImagingParameters params)
throws ImageReadException, IOException {
```
It fails due to the `Override` annotation, with `Method does not override method from its superclass`. If I rename the parameter to `ImagingParameter`, that compiles. But then we could have users sending a `JpegImagingParameters` instead, and it would fail in runtime.
I think this is where I stopped last time, and thought I would have to either accept `ImagingParameters` everywhere, or use generics... any advice @darkma773r ?
Last item pending in this review :tada:
Thanks!!!
Bruno
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] gwlucastrig commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
gwlucastrig commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-841663106
I'll look into it. I think, BufferedImage is also part of java awt. I'm
traveling right now
On Sat, May 15, 2021, 8:18 AM Bruno P. Kinoshita ***@***.***>
wrote:
> If those classes are from awt, the issue is that we have a dependency to
> java.desktop (I think that's the java module name) and also cause an issue
> to users of the API that are using Android (as IIUC, there's no awt in
> Android's Java SDK?).
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/commons-imaging/pull/116#issuecomment-841649340>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AEWJDYNES4M3XQCOJIEZN6TTNZRAXANCNFSM4WPUJSLQ>
> .
>
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] gwlucastrig commented on a change in pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
gwlucastrig commented on a change in pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#discussion_r673100545
##########
File path: src/main/java/org/apache/commons/imaging/ImageParser.java
##########
@@ -79,25 +78,18 @@
* that the documentation is perfect, especially in the more obscure
* and specialized areas of implementation.
*
- * <h3>The "Map params" argument</h3>
+ * <h3>The "params" argument</h3>
*
- * Many of the methods specified by this class accept an argument of
- * type Map giving a list of parameters to be used when processing an
- * image. For example, some of the output formats permit the specification
+ * <p>Many of the methods specified by this class accept an argument of
+ * type {@code T} defining the parameters to be used when
+ * processing an image. For example, some of the output formats permit
* of different kinds of image compression or color models. Some of the
* reading methods permit the calling application to require strict
- * format compliance. In many cases, however, an application will not
- * require the use of this argument. While some of the ImageParser
- * implementations check for (and ignore) null arguments for this parameter,
- * not all of them do (at least not at the time these notes were written).
- * Therefore, a prudent programmer will always supply an valid, though
- * empty instance of a Map implementation when calling such methods.
- * Generally, the java HashMap class is useful for this purpose.
+ * format compliance.</p>
*
- * <p>Additionally, developers creating or enhancing classes derived
- * from ImageParser are encouraged to include such checks in their code.
+ * @param <T> type of parameters used by this image parser
*/
-public abstract class ImageParser extends BinaryFileParser {
+public abstract class ImageParser<T extends ImagingParameters> extends BinaryFileParser {
Review comment:
Here's an idea for a plan (or at least 12 percent of an plan):
In the package example.common, we have the class ExampleParameters and the interface ExampleParser
```
public class ExampleParameters {
public int test;
}
public interface ExampleParser {
public void parse(ExampleParameters paramaters);
}
```
There are multiple implementing format packages. In package example.format.alpha, we have
```
public class AlphaParameters extends ExampleParameters {
int mess;
/**
* A copy constructor
* @param source
*/
AlphaParameters(ExampleParameters source){
this.mess =source.test;
}
}
public class AlphaParser implements ExampleParser {
@Override
public void parse(ExampleParameters parameters) {
AlphaParameters alphaParameters;
if(parameters instanceof AlphaParameters){
alphaParameters = (AlphaParameters)parameters;
}else{
alphaParameters = new AlphaParameters(parameters);
}
parse(alphaParameters);
}
public void parse(AlphaParameters parameters){
// do the work
}
}
```
My thinking is that most applications that have format-specific parameters, will call their parser class directly and pass in the specific class that goes with it (in this example, AlphaParameter... in real life TiffImagingParameters, etc.). Even those that passed in a generic would experience a small overhead for the downcast or the copy constructor... Those application that are use the high-level ImageParser class (which determines format and branches based on format type) would pass in an instance of the base class ExampleParameters. After all, it's only a small performance hit and if they really have format-specific needs (as I do in my applications), they would instantiate a format-specific parser.
This approach has the advantage of avoiding Generics, which some developers might find challenging or obscure, especially in this particular case.
Also, the copy constructor that I used in this example wouldn't always be necessary. A parser class could also do something like:
```
@Override
public void parse(ExampleParameters parameters) {
if (parameters instanceof AlphaParameters) {
processAlphaParameters((AlphaParameters) parameters);
} else {
processGeneralParameters(parameters);
}
```
Whether we use a copy constructor or a branch as shown above, my overall approach does have one disadvantage: it doesn't detect misuse of the API for all contingencies at compile time. For example, if we had
```
// BetaParameters extends ExampleParameters
BetaParameters betaParameters = new BetaParameters();
AlphaParser alphaParser = new AlphaParser();
alphaParser.parse(betaParameters);
```
This would compile just fine. However, betaParameters would contain information that was irrelevant to the alpha parser. It would probably be a case where the programmer made a mistake. The program would probably run, but it might not do quite what the developer hoped. Some debugging would be necessary. How could we help this case? We could introduce run-time checking into all of our parsers, or we could just let it go.
I'm leaning to the "just let it go option", but here's one way that the run-time checking could be accomplished. In the AlphaParser class, we have
```
if (parameters instanceof AlphaParameters) {
alphaParameters = (AlphaParameters) parameters;
} else {
enforceUseOfBaseClass(parameters);
alphaParameters = new AlphaParameters(parameters);
}
```
And the method:
```
void enforceUseOfBaseClass(ExampleParameters parameters){
if(!ExampleParameters.class.equals(parameters.getClass())){
throw new IllegalArgumentException("parameters is not the base class");
}
}
```
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a change in pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on a change in pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#discussion_r678842742
##########
File path: src/main/java/org/apache/commons/imaging/ImageParser.java
##########
@@ -79,25 +78,18 @@
* that the documentation is perfect, especially in the more obscure
* and specialized areas of implementation.
*
- * <h3>The "Map params" argument</h3>
+ * <h3>The "params" argument</h3>
*
- * Many of the methods specified by this class accept an argument of
- * type Map giving a list of parameters to be used when processing an
- * image. For example, some of the output formats permit the specification
+ * <p>Many of the methods specified by this class accept an argument of
+ * type {@code T} defining the parameters to be used when
+ * processing an image. For example, some of the output formats permit
* of different kinds of image compression or color models. Some of the
* reading methods permit the calling application to require strict
- * format compliance. In many cases, however, an application will not
- * require the use of this argument. While some of the ImageParser
- * implementations check for (and ignore) null arguments for this parameter,
- * not all of them do (at least not at the time these notes were written).
- * Therefore, a prudent programmer will always supply an valid, though
- * empty instance of a Map implementation when calling such methods.
- * Generally, the java HashMap class is useful for this purpose.
+ * format compliance.</p>
*
- * <p>Additionally, developers creating or enhancing classes derived
- * from ImageParser are encouraged to include such checks in their code.
+ * @param <T> type of parameters used by this image parser
*/
-public abstract class ImageParser extends BinaryFileParser {
+public abstract class ImageParser<T extends ImagingParameters> extends BinaryFileParser {
Review comment:
>I'm also leaning toward the "just let it go option" for users that pass format-specific parameters to parsers of the wrong format type. In my mind, any format-specific parameters that do not apply should just be ignored.
Yeah, I realize that this would be simpler for us to maintain, but I still think this could lead to problems to users. There certain formats that people may be confused about which parameter class to use.
For example, someone could try the TIFF for JPEG, or vice-versa. The `TiffImagingParameters` extends `XmpImagingParameters`. And the `JpegImagingParameters` extends the `TiffImagingParameters`. So you could have code using a Tiff parser with the `JpegImagingParameters` that works fine. Until we - for instance - override a property like compression in the JPEG parameter. That could break the code of the user, and would require some debugging to understand why that started happening.
Maybe we should consult with others in the mailing list?
>Side note: @kinow, I'm looking at the ImagingParameters class and I can't find any usages of the getImageFormat() method elsewhere in the code. Is that method used anywhere?
Ah! It's not! Brilliant catch @darkma773r ! The parsers are using the format to write/read. And a parser knows its own format. No need to have it in the parameters used in the parser. Thanks a lot for finding this one @darkma773r , less one new public API that would need updating later on :+1:
Removing 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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a change in pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on a change in pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#discussion_r678842742
##########
File path: src/main/java/org/apache/commons/imaging/ImageParser.java
##########
@@ -79,25 +78,18 @@
* that the documentation is perfect, especially in the more obscure
* and specialized areas of implementation.
*
- * <h3>The "Map params" argument</h3>
+ * <h3>The "params" argument</h3>
*
- * Many of the methods specified by this class accept an argument of
- * type Map giving a list of parameters to be used when processing an
- * image. For example, some of the output formats permit the specification
+ * <p>Many of the methods specified by this class accept an argument of
+ * type {@code T} defining the parameters to be used when
+ * processing an image. For example, some of the output formats permit
* of different kinds of image compression or color models. Some of the
* reading methods permit the calling application to require strict
- * format compliance. In many cases, however, an application will not
- * require the use of this argument. While some of the ImageParser
- * implementations check for (and ignore) null arguments for this parameter,
- * not all of them do (at least not at the time these notes were written).
- * Therefore, a prudent programmer will always supply an valid, though
- * empty instance of a Map implementation when calling such methods.
- * Generally, the java HashMap class is useful for this purpose.
+ * format compliance.</p>
*
- * <p>Additionally, developers creating or enhancing classes derived
- * from ImageParser are encouraged to include such checks in their code.
+ * @param <T> type of parameters used by this image parser
*/
-public abstract class ImageParser extends BinaryFileParser {
+public abstract class ImageParser<T extends ImagingParameters> extends BinaryFileParser {
Review comment:
>I'm also leaning toward the "just let it go option" for users that pass format-specific parameters to parsers of the wrong format type. In my mind, any format-specific parameters that do not apply should just be ignored.
Yeah, I realize that this would be simpler for us to maintain, but I still think this could lead to problems to users. There certain formats that people may be confused about which parameter class to use.
For example, someone could try the TIFF for JPEG, or vice-versa. The `TiffImagingParameters` extends `XmpImagingParameters`. And the `JpegImagingParameters` extends the `TiffImagingParameters`. So you could have code using a Tiff parser with the `JpegImagingParameters` that works fine. Until we - for instance - override a property like compression in the JPEG parameter. That could break the code of the user, and would require some debugging to understand why that started happening.
If it were not for the `Imaging.java` file, I think we wouldn't have any problems with the generics and the current approach in this PR :disappointed_relieved: Maybe we should consult with others in the mailing list?
>Side note: @kinow, I'm looking at the ImagingParameters class and I can't find any usages of the getImageFormat() method elsewhere in the code. Is that method used anywhere?
Ah! It's not! Brilliant catch @darkma773r ! The parsers are using the format to write/read. And a parser knows its own format. No need to have it in the parameters used in the parser. Thanks a lot for finding this one @darkma773r , less one new public API that would need updating later on :+1:
Removing 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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-841797895
Whew, I knew we relied on awt, but looks like we are using it in a lot more places than I imagined. Let me update the PR to use a rectangle as ou suggested @gwlucastrig . 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-826283762
Whew, rebased after a few pull requests were merged :cold_sweat:
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-826283762
Whew, rebased after a few pull requests were merged :cold_sweat:
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-841605341
@gwlucastrig
>great suggestion from @gwlucastrig : "use one call for setSubImage, rather than 4 separate calls for setSubImageX, Y, Width, Height."
I started making this change in the `TiffImagingParameter`. First the `public void setSubImage(Integer width, Integer height, Integer x, Integer y)`. All good so far.
But when I finished and started adding the getter, I was thinking in Python or JS, and wanted to return all the 4 parameters at once :disappointed_relieved: Then I realized I would have to create another class to return the 4 parameters… do you have any suggestion for the getter of the subimage in the tiff imaging parameter class?
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] gwlucastrig commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
gwlucastrig commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-918761710
I saw the note you posted on the Commons mailing list about the design of this. I'm wondering if just a simple brute-force approach might do the trick. So I will throw out a wild idea (which I'd be the first to admit I have not thought through thoroughly).
We have a number of issues:
1. Handle cases where applications pass in null imaging parameters (just use defaults)
2. Handle cases where applications pass in an instance of the generic ImagingParameters base class.
3. Handle cases where applications pass in an instance of the subject-matter imaging parameters (i.e. TiffImagingParameters, PngImagingParameters, etc.).
4. Handle cases where applications pass in the wrong thing.
```
TestStuff testStuff = new TestStuff(); // works on TIFFs (for example)
testStuff.aTypicalMethod(null);
testStuff.aTypicalMethod(imgParams);
testStuff.aTypicalMethod(tiffParams);
testStuff.aTypicalMethod(pngParams); // throws exception
```
And we'd like to make the contents of "aTypicalMethod" be pretty compact. In this case, using TiffImagingParameters as an example, what if TiffImagingParameters (and all its brothers and sisters) had a static method called resolveParameters(ImagingParameters) that worked like:
```
void aTypicalMethod(ImagingParameters imagingParameters) {
TiffImagingParameters params = TiffImagingParameters.resolveParameters(imagingParameters);
// now do some work using the parameters
}
```
**How we make this work**
The base class would define two constructors. A no-argument constructor and a "copy constructor".
```
public ImagingParameters(){
}
public ImagingParameters(ImagingParameters source) {
this.bufferedImageFactory = source.bufferedImageFactory;
this.fileName = source.fileName;
this.pixelDensity = source.pixelDensity;
this.strict = source.strict;
}
```
Subject-matter specific parameters classes would implement compatible constructors:
```
public TiffImagingParameters(){
}
public TiffImagingParameters(ImagingParameters imagingParameters){
super(imagingParameters);
}
```
Finally, each parameters class would implement a static method:
```
static public TiffImagingParameters resolveParameters(ImagingParameters imagingParameters) {
TiffImagingParameters params;
if (imagingParameters == null) {
return new TiffImagingParameters();
} else if (imagingParameters instanceof TiffImagingParameters) {
return (TiffImagingParameters) imagingParameters;
} else if (ImagingParameters.class.equals(imagingParameters.getClass())) {
// the specified class is the generic, base class.
// so just copy it's content into a new TiffImagingParameters instance
return new TiffImagingParameters(imagingParameters);
} else {
// In this case, the calling application gave us an instance of an
// incompatible parameters class.
throw new IllegalArgumentException(
"Invalid imaging parameters class "
+ imagingParameters.getClass().getSimpleName());
}
}
```
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] gwlucastrig commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
gwlucastrig commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-766068067
I need a bit more time to review this, but one thing occurs to me right away. In the TIFF standard, there are a number of special data formats and bit-packing sequences (T4 fax, etc.) that are unique to TIFF and some that are similar to other formats. Also, I am not sure that Commons Imaging supports all the TIFF options. So having an enumerated type specific to the TIFF package makes sense for that purpose because it serves as a kind of documentation of what is done (values included in the enumeration) and what still needs to be done (values not found in the enumeration). I'm not sure about compression type (LZW versus Zip, etc.), but that could be the same kind of thing.
On the other hand, there are other a few options that are common to all or many graphics formats (indexed color model versus RGB versus CMYK, etc.). So I would agree that those go in the common package.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-823975384
Rebased. Re-starting work soon, planning to finish by end of this month or May. Then wrap up other issues, and try a 1.0 final release 🙏 📿
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] coveralls edited a comment on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-841637226
[](https://coveralls.io/builds/41817506)
Coverage decreased (-0.2%) to 76.512% when pulling **f91923a6ebd29679b438bcee9a0ba9e83eb463a9 on kinow:IMAGING-159-1** into **a87b545e1019b43bfbe31fa5df3a4f3095a8b438 on apache:master**.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] gwlucastrig commented on a change in pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
gwlucastrig commented on a change in pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#discussion_r673100545
##########
File path: src/main/java/org/apache/commons/imaging/ImageParser.java
##########
@@ -79,25 +78,18 @@
* that the documentation is perfect, especially in the more obscure
* and specialized areas of implementation.
*
- * <h3>The "Map params" argument</h3>
+ * <h3>The "params" argument</h3>
*
- * Many of the methods specified by this class accept an argument of
- * type Map giving a list of parameters to be used when processing an
- * image. For example, some of the output formats permit the specification
+ * <p>Many of the methods specified by this class accept an argument of
+ * type {@code T} defining the parameters to be used when
+ * processing an image. For example, some of the output formats permit
* of different kinds of image compression or color models. Some of the
* reading methods permit the calling application to require strict
- * format compliance. In many cases, however, an application will not
- * require the use of this argument. While some of the ImageParser
- * implementations check for (and ignore) null arguments for this parameter,
- * not all of them do (at least not at the time these notes were written).
- * Therefore, a prudent programmer will always supply an valid, though
- * empty instance of a Map implementation when calling such methods.
- * Generally, the java HashMap class is useful for this purpose.
+ * format compliance.</p>
*
- * <p>Additionally, developers creating or enhancing classes derived
- * from ImageParser are encouraged to include such checks in their code.
+ * @param <T> type of parameters used by this image parser
*/
-public abstract class ImageParser extends BinaryFileParser {
+public abstract class ImageParser<T extends ImagingParameters> extends BinaryFileParser {
Review comment:
Here's an idea for a plan (or at least 12 percent of an plan):
In the package example.common, we have the class ExampleParameters and the interface ExampleParser
```
public class ExampleParameters {
public int test;
}
public interface ExampleParser {
public void parse(ExampleParameters paramaters);
}
```
There are multiple implementing format packages. In package example.format.alpha, we have
```
public class AlphaParameters extends ExampleParameters {
int mess;
/**
* A copy constructor
* @param source
*/
AlphaParameters(ExampleParameters source){
this.mess =source.test;
}
}
public class AlphaParser implements ExampleParser {
@Override
public void parse(ExampleParameters parameters) {
AlphaParameters alphaParameters;
if(parameters instanceof AlphaParameters){
alphaParameters = (AlphaParameters)parameters;
}else{
alphaParameters = new AlphaParameters(parameters);
}
parse(alphaParameters);
}
public void parse(AlphaParameters parameters){
// do the work
}
}
```
My thinking is that most applications that have format-specific parameters, will call their parser class directly and pass in the specific class that goes with it (in this example, AlphaParameter... in real life TiffImagingParameters, etc.). Even those that passed in a generic would experience a small overhead for the downcast or the copy constructor... Those application that are use the high-level ImageParser class (which determines format and branches based on format type) would pass in an instance of the base class ExampleParameters. After all, it's only a small performance hit and if they really have format-specific needs (as I do in my applications), they would instantiate a format-specific parser.
This approach has the advantage of avoiding Generics, which some developers might find challenging or obscure, especially in this particular case.
Also, the copy constructor that I used in this example wouldn't always be necessary. A parser class could also do something like:
```
@Override
public void parse(ExampleParameters parameters) {
if (parameters instanceof AlphaParameters) {
processAlphaParameters((AlphaParameters) parameters);
} else {
processGeneralParameters(parameters);
}
```
This approach has one disadvantage in that we couldn't test for all contingencies at compile time. For example, if we had
```
// BetaParameters extends ExampleParameters
BetaParameters betaParameters = new BetaParameters();
AlphaParser alphaParser = new AlphaParser();
alphaParser.parse(betaParameters);
```
This would compile just fine. However, betaParameters would contain information that was irrelevant to the alpha parser. It would probably be a case where the programmer made a mistake. We could introduce run-time checking into all of our parsers, or we could just let it go.
I'm leaning to the "just let it go option", but here's one way that the run-time checking could be accomplished. In the AlphaParser class, we have
```
if (parameters instanceof AlphaParameters) {
alphaParameters = (AlphaParameters) parameters;
} else {
enforceUseOfBaseClass(parameters);
alphaParameters = new AlphaParameters(parameters);
}
```
And the method:
```
void enforceUseOfBaseClass(ExampleParameters parameters){
if(!ExampleParameters.class.equals(parameters.getClass())){
throw new IllegalArgumentException("parameters is not the base class");
}
}
```
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] coveralls edited a comment on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-841637226
[](https://coveralls.io/builds/41446310)
Coverage decreased (-0.2%) to 76.563% when pulling **622e0b80936e5fa4ae56d426771c30404f578cce on kinow:IMAGING-159-1** into **a87b545e1019b43bfbe31fa5df3a4f3095a8b438 on apache:master**.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] darkma773r commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
darkma773r commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-959156762
I just had another thought on this: it strikes me that the main issue with the generic `ImagerParser<P extends ImagingParameters>` format is how to deal with format-agnostic code, mainly the utility methods in `Imaging`. In other words, how do users specify parameters when they don't even know what the format is? The current approach is to have users pass in their own parameters objects to these utility methods but this gets into the problem of what happens when they pass the wrong parameters type. What if we flipped this? Instead of having the users create the parameters object, we have the parser create one and then have the user modify it as needed in a callback. So, instead of this
```
public static byte[] getICCProfileBytes(final File file, final ImagingParameters params) {
// hopefully params is of the correct type!
}
```
we do this
```
public static byte[] getICCProfileBytes(final File file, final Consumer<ImagingParameters> configurer) {
ImageParser<?> parser = ...; // get the parser
return getICCProfileBytes(parser, configurer);
}
private static <P extends ImagingParameters> byte[] getICCProfileBytes(final ImageParser<P> parser, final Consumer<ImagingParameters> configurer) {
P params = parser.getDefaultParameters();
if (configurer != null) {
// let the caller configure the parameters
configurer.accept(params);
}
// params is known to be the correct type
return parser.getICCProfileBytes(params);
}
```
Users would then call it like this:
```
Imaging.getICCProfileBytes(file, p -> p.setStrict(true));
```
We could also add methods to `ImageParser` that accept consumers like this for consistency. That would probably be best.
Thoughts?
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] gwlucastrig commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
gwlucastrig commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-961784862
My perspective is limited because I am so focused on TIFFs. And I seldom use the basic Imaging class (preferring the TIFF-specific parser and write classes). So I am sure I am unaware of a number of valid use cases.
That being said, I might think that simpler is better. The Imaging class API could be cut back to just include only limited support for parameters. The read-related operations might not even take parameters arguments. The write-related would only take a very basic parameters. In fact, except for a "dots per inch" or "mm per pixel" setting, I can't think of anything that would be supported across the majority of write operations.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] gwlucastrig commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
gwlucastrig commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-961784862
My perspective is limited because I am so focused on TIFFs. And I seldom use the basic Imaging class (preferring the TIFF-specific parser and write classes). So I am sure I am unaware of a number of valid use cases.
That being said, I might think that simpler is better. The Imaging class API could be cut back to just include only limited support for parameters. The read-related operations might not even take parameters arguments. The write-related would only take a very basic parameters. In fact, except for a "dots per inch" or "mm per pixel" setting, I can't think of anything that would be supported across the majority of write operations.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] darkma773r commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
darkma773r commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-963083902
@kinow
I agree with @gwlucastrig, that we should try to keep the API simple. It might be helpful to take a step back and enumerate all of the use cases the API should cover and then decide how they should be handled. Below is what I see (from my limited perspective) as the main use cases. Please add any I've missed.
1. The user wants to read/write an image and does not know or care about the format.
2. The user wants to read/write an image and does not know the format but wants to apply some format-specific parameters if applicable.
3. The user wants to read/write an image in a specific format.
For the first use case, I think @gwlucastrig's idea of removing format-specific parameters from `Imaging` applies. The caller would use the `Imaging` class and then either pass no parameters or pass a very basic `ImagingParameters` instance.
For the second use case, I think the responsibility for creating the correct parameters object could fall to the user. For example, the user would use `Imaging.guessFormat()` to get the format and then use the format-specific parser directly to access additional features. The `Imaging` class would not play a role after the initial determination of the format.
For the third use case, I think users should use the format-specific parser directly.
So, what I'm picturing basically boils down to this:
- Do you want to read/write images and only care about the most basic of image parameters? Great! Use `Imaging`.
- Do you want to read/write images in a specific format and access low-level format features? Cool. Use the parser for the format.
This approach would remove some of the design headaches we keep running into with `Imaging`, where we're trying to allow access to format-specific features from generic code. If we can agree on how the API should address these use cases then we can move on to deciding how to make the API readable and user-friendly.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] gwlucastrig edited a comment on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
gwlucastrig edited a comment on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-841791184
It looks like providing compatibility with Android OS may not be practical.
Android has its own graphics and media classes. So anything with java.awt
in it will not port. Common Imaging using BufferedImage extensively. Most
other AWT elements are used only a little. Color shows up the most. The
ICC_ColorSpace also shows up. A couple of the test applications use
Graphics to draw some test images as input.
I think I might post a question on Java Coderanch or Stackoverflow to ask
about compatible solutions.
On Sat, May 15, 2021 at 8:18 AM Bruno P. Kinoshita ***@***.***>
wrote:
> If those classes are from awt, the issue is that we have a dependency to
> java.desktop (I think that's the java module name) and also cause an issue
> to users of the API that are using Android (as IIUC, there's no awt in
> Android's Java SDK?).
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/commons-imaging/pull/116#issuecomment-841649340>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AEWJDYNES4M3XQCOJIEZN6TTNZRAXANCNFSM4WPUJSLQ>
> .
>
<details>
<summary>Search "java.awt" (229 hits in 121 files of 557 searched)</summary>
commons-imaging\src\main\java\org\apache\commons\imaging\ColorTools.java (11 hits)
Line 20: import java.awt.RenderingHints;
Line 21: import java.awt.Transparency;
Line 22: import java.awt.color.ColorSpace;
Line 23: import java.awt.color.ICC_ColorSpace;
Line 24: import java.awt.color.ICC_Profile;
Line 25: import java.awt.image.BufferedImage;
Line 26: import java.awt.image.ColorConvertOp;
Line 27: import java.awt.image.ColorModel;
Line 28: import java.awt.image.ComponentColorModel;
Line 29: import java.awt.image.DirectColorModel;
Line 30: import java.awt.image.ImagingOpException;
commons-imaging\src\main\java\org\apache\commons\imaging\common\BufferedImageFactory.java (1 hit)
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\common\ImageBuilder.java (9 hits)
Line 43: import java.awt.color.ColorSpace;
Line 44: import java.awt.image.BufferedImage;
Line 45: import java.awt.image.ColorModel;
Line 46: import java.awt.image.DataBuffer;
Line 47: import java.awt.image.DataBufferInt;
Line 48: import java.awt.image.DirectColorModel;
Line 49: import java.awt.image.Raster;
Line 50: import java.awt.image.RasterFormatException;
Line 51: import java.awt.image.WritableRaster;
commons-imaging\src\main\java\org\apache\commons\imaging\common\RgbBufferedImageFactory.java (1 hit)
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\common\SimpleBufferedImageFactory.java (1 hit)
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\bmp\BmpImageParser.java (2 hits)
Line 27: import java.awt.Dimension;
Line 28: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\bmp\BmpWriter.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\bmp\BmpWriterPalette.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\bmp\BmpWriterRgb.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\dcx\DcxImageParser.java (2 hits)
Line 23: import java.awt.Dimension;
Line 24: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\gif\GifImageParser.java (2 hits)
Line 28: import java.awt.Dimension;
Line 29: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\icns\IcnsDecoder.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\icns\IcnsImageParser.java (2 hits)
Line 23: import java.awt.Dimension;
Line 24: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\ico\IcoImageParser.java (2 hits)
Line 26: import java.awt.Dimension;
Line 27: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\jpeg\decoder\JpegDecoder.java (6 hits)
Line 21: import java.awt.image.BufferedImage;
Line 22: import java.awt.image.ColorModel;
Line 23: import java.awt.image.DataBuffer;
Line 24: import java.awt.image.DirectColorModel;
Line 25: import java.awt.image.Raster;
Line 26: import java.awt.image.WritableRaster;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\jpeg\JpegImageMetadata.java (2 hits)
Line 19: import java.awt.Dimension;
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\jpeg\JpegImageParser.java (2 hits)
Line 23: import java.awt.Dimension;
Line 24: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\pcx\PcxImageParser.java (11 hits)
Line 24: import java.awt.Dimension;
Line 25: import java.awt.Transparency;
Line 26: import java.awt.color.ColorSpace;
Line 27: import java.awt.image.BufferedImage;
Line 28: import java.awt.image.ColorModel;
Line 29: import java.awt.image.ComponentColorModel;
Line 30: import java.awt.image.DataBuffer;
Line 31: import java.awt.image.DataBufferByte;
Line 32: import java.awt.image.IndexColorModel;
Line 33: import java.awt.image.Raster;
Line 34: import java.awt.image.WritableRaster;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\pcx\PcxWriter.java (1 hit)
Line 18: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\png\PngImageParser.java (6 hits)
Line 25: import java.awt.Dimension;
Line 26: import java.awt.color.ColorSpace;
Line 27: import java.awt.color.ICC_ColorSpace;
Line 28: import java.awt.color.ICC_Profile;
Line 29: import java.awt.image.BufferedImage;
Line 30: import java.awt.image.ColorModel;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\png\PngWriter.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\png\ScanExpediter.java (1 hit)
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\png\ScanExpediterInterlaced.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\png\ScanExpediterSimple.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\pnm\PamWriter.java (1 hit)
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\pnm\PbmWriter.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\pnm\PgmWriter.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\pnm\PnmImageParser.java (2 hits)
Line 22: import java.awt.Dimension;
Line 23: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\pnm\PnmWriter.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\pnm\PpmWriter.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\psd\dataparsers\DataParser.java (2 hits)
Line 19: import java.awt.image.BufferedImage;
Line 20: import java.awt.image.DataBuffer;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\psd\datareaders\CompressedDataReader.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\psd\datareaders\DataReader.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\psd\datareaders\UncompressedDataReader.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\psd\PsdImageParser.java (2 hits)
Line 26: import java.awt.Dimension;
Line 27: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\rgbe\RgbeImageParser.java (10 hits)
Line 19: import java.awt.Dimension;
Line 20: import java.awt.Point;
Line 21: import java.awt.Transparency;
Line 22: import java.awt.color.ColorSpace;
Line 23: import java.awt.image.BandedSampleModel;
Line 24: import java.awt.image.BufferedImage;
Line 25: import java.awt.image.ComponentColorModel;
Line 26: import java.awt.image.DataBuffer;
Line 27: import java.awt.image.DataBufferFloat;
Line 28: import java.awt.image.Raster;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\tiff\datareaders\DataReaderStrips.java (1 hit)
Line 20: import java.awt.Rectangle;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\tiff\datareaders\DataReaderTiled.java (1 hit)
Line 26: import java.awt.Rectangle;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\tiff\datareaders\ImageDataReader.java (1 hit)
Line 32: import java.awt.Rectangle;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\tiff\photometricinterpreters\floatingpoint\PaletteEntry.java (1 hit)
Line 19: import java.awt.Color;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\tiff\photometricinterpreters\floatingpoint\PaletteEntryForRange.java (1 hit)
Line 19: import java.awt.Color;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\tiff\photometricinterpreters\floatingpoint\PaletteEntryForValue.java (1 hit)
Line 19: import java.awt.Color;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\tiff\photometricinterpreters\floatingpoint\PhotometricInterpreterFloat.java (1 hit)
Line 19: import java.awt.Color;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\tiff\TiffDirectory.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\tiff\TiffImageMetadata.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\tiff\TiffImageParser.java (3 hits)
Line 28: import java.awt.Dimension;
Line 29: import java.awt.Rectangle;
Line 30: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\tiff\write\TiffImageWriterBase.java (2 hits)
Line 33: import java.awt.image.BufferedImage;
Line 34: import java.awt.image.ColorModel;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\wbmp\WbmpImageParser.java (7 hits)
Line 21: import java.awt.Dimension;
Line 22: import java.awt.image.BufferedImage;
Line 23: import java.awt.image.DataBuffer;
Line 24: import java.awt.image.DataBufferByte;
Line 25: import java.awt.image.IndexColorModel;
Line 26: import java.awt.image.Raster;
Line 27: import java.awt.image.WritableRaster;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\xbm\XbmImageParser.java (8 hits)
Line 19: import java.awt.Dimension;
Line 20: import java.awt.image.BufferedImage;
Line 21: import java.awt.image.ColorModel;
Line 22: import java.awt.image.DataBuffer;
Line 23: import java.awt.image.DataBufferByte;
Line 24: import java.awt.image.IndexColorModel;
Line 25: import java.awt.image.Raster;
Line 26: import java.awt.image.WritableRaster;
commons-imaging\src\main\java\org\apache\commons\imaging\formats\xpm\XpmImageParser.java (8 hits)
Line 18: import java.awt.Dimension;
Line 19: import java.awt.image.BufferedImage;
Line 20: import java.awt.image.ColorModel;
Line 21: import java.awt.image.DataBuffer;
Line 22: import java.awt.image.DirectColorModel;
Line 23: import java.awt.image.IndexColorModel;
Line 24: import java.awt.image.Raster;
Line 25: import java.awt.image.WritableRaster;
commons-imaging\src\main\java\org\apache\commons\imaging\icc\IccProfileParser.java (1 hit)
Line 23: import java.awt.color.ICC_Profile;
commons-imaging\src\main\java\org\apache\commons\imaging\ImageDump.java (4 hits)
Line 19: import java.awt.color.ColorSpace;
Line 20: import java.awt.color.ICC_ColorSpace;
Line 21: import java.awt.color.ICC_Profile;
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\ImageParser.java (2 hits)
Line 19: import java.awt.Dimension;
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\Imaging.java (3 hits)
Line 22: import java.awt.Dimension;
Line 23: import java.awt.color.ICC_Profile;
Line 24: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\internal\Debug.java (1 hit)
Line 19: import java.awt.color.ICC_Profile;
commons-imaging\src\main\java\org\apache\commons\imaging\palette\Dithering.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\palette\MedianCutQuantizer.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\main\java\org\apache\commons\imaging\palette\PaletteFactory.java (3 hits)
Line 19: import java.awt.color.ColorSpace;
Line 20: import java.awt.image.BufferedImage;
Line 21: import java.awt.image.ColorModel;
commons-imaging\src\test\java\org\apache\commons\imaging\common\bytesource\ByteSourceImageTest.java (2 hits)
Line 24: import java.awt.Dimension;
Line 25: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\common\bytesource\ByteSourceInputStreamTest.java (1 hit)
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\common\ImageBuilderTest.java (3 hits)
Line 19: import java.awt.image.BufferedImage;
Line 20: import java.awt.image.ColorModel;
Line 21: import java.awt.image.RasterFormatException;
commons-imaging\src\test\java\org\apache\commons\imaging\common\RgbBufferedImageFactoryTest.java (1 hit)
Line 22: import java.awt.image.DirectColorModel;
commons-imaging\src\test\java\org\apache\commons\imaging\examples\ApacheImagingSpeedAndMemoryTest.java (1 hit)
Line 156: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\examples\ImageReadExample.java (5 hits)
Line 19: import java.awt.GraphicsConfiguration;
Line 20: import java.awt.GraphicsDevice;
Line 21: import java.awt.GraphicsEnvironment;
Line 22: import java.awt.Transparency;
Line 23: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\examples\ImageWriteExample.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\examples\SampleUsage.java (3 hits)
Line 19: import java.awt.Dimension;
Line 20: import java.awt.color.ICC_Profile;
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\examples\tiff\ExampleReadFloatingPointData.java (2 hits)
Line 19: import java.awt.Color;
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\examples\tiff\ReadAndRenderFloatingPoint.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\examples\tiff\ReadTagsAndImages.java (1 hit)
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\bmp\BmpReadTest.java (1 hit)
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\bmp\BmpRoundtripTest.java (1 hit)
Line 23: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\bmp\BmpWriterRgbTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\dcx\DcxReadTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\gif\GifReadTest.java (1 hit)
Line 24: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\icns\IcnsReadTest.java (1 hit)
Line 23: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\icns\IcnsRoundTripTest.java (1 hit)
Line 23: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\ico\IcoReadTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\ico\IcoRoundtripTest.java (1 hit)
Line 23: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\jpeg\exif\MicrosoftTagTest.java (1 hit)
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\jpeg\iptc\IptcFullDiscardTest.java (2 hits)
Line 20: import java.awt.Graphics2D;
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\jpeg\JpegReadTest.java (1 hit)
Line 23: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\jpeg\JpegWithJpegThumbnailTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\jpeg\specific\JpegImageParserTest.java (1 hit)
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\pam\PamReadTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\pcx\PcxReadTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\png\ConvertPngToGifTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\png\PngMultipleRoundtripTest.java (1 hit)
Line 23: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\png\PngReadTest.java (1 hit)
Line 23: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\png\PngTextTest.java (2 hits)
Line 24: import java.awt.Color;
Line 25: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\png\PngWriteForceTrueColorText.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\png\PngWriteReadTest.java (1 hit)
Line 25: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\pnm\PnmImageParserTest.java (2 hits)
Line 23: import java.awt.image.BufferedImage;
Line 24: import java.awt.image.DataBufferInt;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\psd\PsdReadTest.java (1 hit)
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\rgbe\RgbeReadTest.java (1 hit)
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\tiff\photometricinterpreters\floatingpoint\PaletteEntryForRangeTest.java (1 hit)
Line 24: import java.awt.Color;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\tiff\photometricinterpreters\floatingpoint\PaletteEntryForValueTest.java (1 hit)
Line 24: import java.awt.Color;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\tiff\photometricinterpreters\floatingpoint\PhotometricInterpreterFloatTest.java (1 hit)
Line 23: import java.awt.Color;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\tiff\TiffAlphaRoundTripTest.java (4 hits)
Line 19: import java.awt.AlphaComposite;
Line 20: import java.awt.Color;
Line 21: import java.awt.Graphics2D;
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\tiff\TiffCcittTest.java (1 hit)
Line 23: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\tiff\TiffFloatingPointReadTest.java (2 hits)
Line 23: import java.awt.Color;
Line 24: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\tiff\TiffFloatingPointRoundTripTest.java (1 hit)
Line 23: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\tiff\TiffReadAlphaTest.java (3 hits)
Line 19: import java.awt.Color;
Line 20: import java.awt.Graphics2D;
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\tiff\TiffReadTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\tiff\TiffRoundtripTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\tiff\TiffSubImageTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\wbmp\WbmpReadTest.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\xbm\XbmReadTest.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\xmp\XmpUpdateTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\formats\xpm\XpmReadTest.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\ImageDumpTest.java (2 hits)
Line 21: import java.awt.color.ColorSpace;
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\palette\DitheringTest.java (1 hit)
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\palette\PaletteQuantizationTest.java (1 hit)
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\roundtrip\BitmapRoundtripTest.java (1 hit)
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\roundtrip\FullColorRoundtrip.java (1 hit)
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\roundtrip\GrayscaleRountripTest.java (1 hit)
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\roundtrip\ImageAsserts.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\roundtrip\LimitedColorRoundtripTest.java (1 hit)
Line 20: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\roundtrip\NullParametersRoundtripTest.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\roundtrip\PixelDensityRoundtrip.java (1 hit)
Line 22: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\roundtrip\RoundtripBase.java (1 hit)
Line 21: import java.awt.image.BufferedImage;
commons-imaging\src\test\java\org\apache\commons\imaging\roundtrip\TestImages.java (1 hit)
Line 19: import java.awt.image.BufferedImage;
Search "java.awt" (0 hits in 0 files of 1 searched)
</details>
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a change in pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on a change in pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#discussion_r780503881
##########
File path: src/main/java/org/apache/commons/imaging/Imaging.java
##########
@@ -50,62 +48,40 @@
*
* <h3>Using this class</h3>
*
- * <p>
- * Almost all of the Apache Commons Imaging library's core functionality can
+ * <p>Almost all of the Apache Commons Imaging library's core functionality can
* be accessed through the methods provided by this class.
* The use of the Imaging class is similar to the Java API's ImageIO class,
- * though Imaging supports formats and options not included in the standard
- * Java API.
- * </p>
+ * though Imaging supports formats not included in the standard Java API.</p>
*
- * <p>
- * All of methods provided by the Imaging class are declared static.
- * </p>
+ * <p>All of methods provided by the Imaging class are declared static.</p>
*
- * <p>
- * The Apache Commons Imaging package is a pure Java implementation.
- * </p>
+ * <p>The Apache Commons Imaging package is a pure Java implementation.</p>
*
* <h3>Format support</h3>
*
- * <p>
- * While the Apache Commons Imaging package handles a number of different
+ * <p>While the Apache Commons Imaging package handles a number of different
* graphics formats, support for some formats is not yet complete.
* For the most recent information on support for specific formats, refer to
* <a href="https://commons.apache.org/imaging/formatsupport.html">Format Support</a>
- * at the main project development web site.
- * </p>
+ * at the main project development web site.</p>
*
* <h3>Optional parameters for image reading and writing</h3>
*
- * <p>
- * Some of the methods provided by this class accept an optional
- * <strong>params</strong> argument that permits the application to specify
- * elements for special handling. If these specifications are not required by
- * the application, the params argument may be omitted (as appropriate) or
- * a null argument may be provided. In image-writing operations, the option
- * parameters may include options such as data-compression type (if any),
- * color model, or other format-specific data representations. The parameters
- * map may also be used to provide EXIF Tags and other metadata to those
- * formats that support them. In image-reading operations,
- * the parameters may include information about special handling in reading
- * the image data.
- * </p>
+ * <p>Many of the operations provided in this class as static calls can be accessed directly
+ * using {@link ImageParser}'s. It is, however, difficult to design an API that is generic
+ * enough so users can safely read or write byte arrays when specifying the type of parameters,
+ * since the only way to confirm the parameters are valid for a specific format is by reading
+ * the byte array (when it is already too late.)</p>
Review comment:
**N.B.** we are removing the public methods of `Imaging`, and updating the docs here. Now users must use the `ImageParser` classes directly if they want to customize how to read/write images.
This reduced a lot the size of `Imaging`. I think this prevents issues we have in [lang] where some classes grow longer and longer as more features are added - in our case it would grow by 3-4 methods (one for `File`, one for `ByteSource`, one for `bytes`, etc) for each new image file format added to [imaging].
And, of course, that solves the issue of trying to prevent `ClassCastException`'s, generics, and trying to create a good API for users in `Imaging`.
##########
File path: src/main/java/org/apache/commons/imaging/ImageParser.java
##########
@@ -79,25 +78,18 @@
* that the documentation is perfect, especially in the more obscure
* and specialized areas of implementation.
*
- * <h3>The "Map params" argument</h3>
+ * <h3>The "params" argument</h3>
*
- * Many of the methods specified by this class accept an argument of
- * type Map giving a list of parameters to be used when processing an
- * image. For example, some of the output formats permit the specification
+ * <p>Many of the methods specified by this class accept an argument of
+ * type {@code T} defining the parameters to be used when
+ * processing an image. For example, some of the output formats permit
* of different kinds of image compression or color models. Some of the
* reading methods permit the calling application to require strict
- * format compliance. In many cases, however, an application will not
- * require the use of this argument. While some of the ImageParser
- * implementations check for (and ignore) null arguments for this parameter,
- * not all of them do (at least not at the time these notes were written).
- * Therefore, a prudent programmer will always supply an valid, though
- * empty instance of a Map implementation when calling such methods.
- * Generally, the java HashMap class is useful for this purpose.
+ * format compliance.</p>
*
- * <p>Additionally, developers creating or enhancing classes derived
- * from ImageParser are encouraged to include such checks in their code.
+ * @param <T> type of parameters used by this image parser
*/
-public abstract class ImageParser extends BinaryFileParser {
+public abstract class ImageParser<T extends ImagingParameters> extends BinaryFileParser {
Review comment:
Separated TIFF and JPEG parameters, now they share a common ancestor, but JPEG doesn't extend TIFF.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow edited a comment on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow edited a comment on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-826703877
Note to self:
- [x] rebase again
- [x] check Tabs vs. Spaces
- [x] great suggestion from @gwlucastrig : "use one call for setSubImage, rather than 4 separate calls for setSubImageX, Y, Width, Height."
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a change in pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on a change in pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#discussion_r780503881
##########
File path: src/main/java/org/apache/commons/imaging/Imaging.java
##########
@@ -50,62 +48,40 @@
*
* <h3>Using this class</h3>
*
- * <p>
- * Almost all of the Apache Commons Imaging library's core functionality can
+ * <p>Almost all of the Apache Commons Imaging library's core functionality can
* be accessed through the methods provided by this class.
* The use of the Imaging class is similar to the Java API's ImageIO class,
- * though Imaging supports formats and options not included in the standard
- * Java API.
- * </p>
+ * though Imaging supports formats not included in the standard Java API.</p>
*
- * <p>
- * All of methods provided by the Imaging class are declared static.
- * </p>
+ * <p>All of methods provided by the Imaging class are declared static.</p>
*
- * <p>
- * The Apache Commons Imaging package is a pure Java implementation.
- * </p>
+ * <p>The Apache Commons Imaging package is a pure Java implementation.</p>
*
* <h3>Format support</h3>
*
- * <p>
- * While the Apache Commons Imaging package handles a number of different
+ * <p>While the Apache Commons Imaging package handles a number of different
* graphics formats, support for some formats is not yet complete.
* For the most recent information on support for specific formats, refer to
* <a href="https://commons.apache.org/imaging/formatsupport.html">Format Support</a>
- * at the main project development web site.
- * </p>
+ * at the main project development web site.</p>
*
* <h3>Optional parameters for image reading and writing</h3>
*
- * <p>
- * Some of the methods provided by this class accept an optional
- * <strong>params</strong> argument that permits the application to specify
- * elements for special handling. If these specifications are not required by
- * the application, the params argument may be omitted (as appropriate) or
- * a null argument may be provided. In image-writing operations, the option
- * parameters may include options such as data-compression type (if any),
- * color model, or other format-specific data representations. The parameters
- * map may also be used to provide EXIF Tags and other metadata to those
- * formats that support them. In image-reading operations,
- * the parameters may include information about special handling in reading
- * the image data.
- * </p>
+ * <p>Many of the operations provided in this class as static calls can be accessed directly
+ * using {@link ImageParser}'s. It is, however, difficult to design an API that is generic
+ * enough so users can safely read or write byte arrays when specifying the type of parameters,
+ * since the only way to confirm the parameters are valid for a specific format is by reading
+ * the byte array (when it is already too late.)</p>
Review comment:
**N.B.** we are removing the public methods of `Imaging`, and updating the docs here. Now users must use the `ImageParser` classes directly if they want to customize how to read/write images.
This reduced a lot the size of `Imaging`. I think this prevents issues we have in [lang] where some classes grow longer and longer as more features are added - in our case it would grow by 3-4 methods (one for `File`, one for `ByteSource`, one for `bytes`, etc) for each new image file format added to [imaging].
And, of course, that solves the issue of trying to prevent `ClassCastException`'s, generics, and trying to create a good API for users in `Imaging`.
##########
File path: src/main/java/org/apache/commons/imaging/ImageParser.java
##########
@@ -79,25 +78,18 @@
* that the documentation is perfect, especially in the more obscure
* and specialized areas of implementation.
*
- * <h3>The "Map params" argument</h3>
+ * <h3>The "params" argument</h3>
*
- * Many of the methods specified by this class accept an argument of
- * type Map giving a list of parameters to be used when processing an
- * image. For example, some of the output formats permit the specification
+ * <p>Many of the methods specified by this class accept an argument of
+ * type {@code T} defining the parameters to be used when
+ * processing an image. For example, some of the output formats permit
* of different kinds of image compression or color models. Some of the
* reading methods permit the calling application to require strict
- * format compliance. In many cases, however, an application will not
- * require the use of this argument. While some of the ImageParser
- * implementations check for (and ignore) null arguments for this parameter,
- * not all of them do (at least not at the time these notes were written).
- * Therefore, a prudent programmer will always supply an valid, though
- * empty instance of a Map implementation when calling such methods.
- * Generally, the java HashMap class is useful for this purpose.
+ * format compliance.</p>
*
- * <p>Additionally, developers creating or enhancing classes derived
- * from ImageParser are encouraged to include such checks in their code.
+ * @param <T> type of parameters used by this image parser
*/
-public abstract class ImageParser extends BinaryFileParser {
+public abstract class ImageParser<T extends ImagingParameters> extends BinaryFileParser {
Review comment:
Separated TIFF and JPEG parameters, now they share a common ancestor, but JPEG doesn't extend TIFF.
##########
File path: src/main/java/org/apache/commons/imaging/Imaging.java
##########
@@ -857,20 +837,19 @@ public static Dimension getImageSize(final File file) throws ImageReadException,
*
* @param file
* File containing image data.
- * @param params
- * Map of optional parameters, defined in ImagingConstants.
+ * @param params optional parameters.
* @return The width and height of the image.
* @throws ImageReadException if it fails to parse the image
* @throws IOException if it fails to read the image data
*/
- public static Dimension getImageSize(final File file, final Map<String, Object> params)
+ public static Dimension getImageSize(final File file, final ImagingParameters params)
Review comment:
@darkma773r I went with option 2. It reduces the `Imaging.java` size, and prevents the god-class issue in case we added more image formats later. IMHO, it will be simpler to either improve the `ImageParser`'s API, or think in how to better handle parameters in the `Imaging` class (like merging parameters, of providing factories as you suggested).
But for 1.0 this should be better than the old `Hashmap`, and avoid runtime exceptions. Thank you!!!
##########
File path: src/changes/changes.xml
##########
@@ -45,6 +45,21 @@ The <action> type attribute can be add,update,fix,remove.
</properties>
<body>
<release version="1.0-alpha3" date="2020-??-??" description="Third 1.0 alpha release">
+ <action issue="IMAGING-159" dev="kinow" type="fix" due-to="Bruno P. Kinoshita, Gary Lucas, Matt Juntunen">
Review comment:
Thanks for the help!!! :tada:
##########
File path: src/changes/changes.xml
##########
@@ -45,6 +45,21 @@ The <action> type attribute can be add,update,fix,remove.
</properties>
<body>
<release version="1.0-alpha3" date="2020-??-??" description="Third 1.0 alpha release">
+ <action issue="IMAGING-159" dev="kinow" type="fix" due-to="Bruno P. Kinoshita, Gary Lucas, Matt Juntunen">
Review comment:
Thanks for the help!!! :tada: :point_up:
##########
File path: src/changes/changes.xml
##########
@@ -45,6 +45,21 @@ The <action> type attribute can be add,update,fix,remove.
</properties>
<body>
<release version="1.0-alpha3" date="2020-??-??" description="Third 1.0 alpha release">
+ <action issue="IMAGING-159" dev="kinow" type="fix" due-to="Bruno P. Kinoshita, Gary Lucas, Matt Juntunen">
Review comment:
Thanks for the help!!! :point_up: :bowing_man:
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-1007768801
@gwlucastrig , @darkma773r I believe this is ready for review again. And I think it is really close to being ready to be merged! :tada:
@darkma773r I've removed the `Imaging` public methods that used `ImagingParameter`'s (and `Hashmap`'s before). This reduced the amount of public methods in `Imaging`, and later we can decide whether to add back methods with Parameter factories/builders/mergers/etc, or create builders in each parsers, or improve the parsers' API'...
After rebasing, there was a bit of Javadoc errors, probably from recent plug-in versions. Fixed those too, reviewed the code removing unnecessary changes, and finally added the `changes.xml` entry. With this, leaving the 126 modified files for you guys to review :smile: (sorry!)
Thanks a lot for the help,
Bruno
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] darkma773r commented on a change in pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
darkma773r commented on a change in pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#discussion_r673625407
##########
File path: src/main/java/org/apache/commons/imaging/ImageParser.java
##########
@@ -79,25 +78,18 @@
* that the documentation is perfect, especially in the more obscure
* and specialized areas of implementation.
*
- * <h3>The "Map params" argument</h3>
+ * <h3>The "params" argument</h3>
*
- * Many of the methods specified by this class accept an argument of
- * type Map giving a list of parameters to be used when processing an
- * image. For example, some of the output formats permit the specification
+ * <p>Many of the methods specified by this class accept an argument of
+ * type {@code T} defining the parameters to be used when
+ * processing an image. For example, some of the output formats permit
* of different kinds of image compression or color models. Some of the
* reading methods permit the calling application to require strict
- * format compliance. In many cases, however, an application will not
- * require the use of this argument. While some of the ImageParser
- * implementations check for (and ignore) null arguments for this parameter,
- * not all of them do (at least not at the time these notes were written).
- * Therefore, a prudent programmer will always supply an valid, though
- * empty instance of a Map implementation when calling such methods.
- * Generally, the java HashMap class is useful for this purpose.
+ * format compliance.</p>
*
- * <p>Additionally, developers creating or enhancing classes derived
- * from ImageParser are encouraged to include such checks in their code.
+ * @param <T> type of parameters used by this image parser
*/
-public abstract class ImageParser extends BinaryFileParser {
+public abstract class ImageParser<T extends ImagingParameters> extends BinaryFileParser {
Review comment:
@gwlucastrig, your approach is almost exactly like the one I was going to suggest :-)
- Have `ImageParser` and other classes accept `ImagingParameters` directly.
- Add format-specific parameter overloads of these methods as needed,
- Have the general methods either cast the general `ImagingParameters` to the format-specific ones or use a copy constructor to extract relevant fields while leaving others as defaults.
> I'm leaning to the "just let it go option", but here's one way that the run-time checking could be accomplished.
I'm also leaning toward the "just let it go option" for users that pass format-specific parameters to parsers of the wrong format type. In my mind, any format-specific parameters that do not apply should just be ignored. I'm guessing this was the case when the API used a `Map` to pass parameters so we shouldn't be losing anything. I would also not expect a method that accepts an `ImagingParameters` argument to succeed if I pass one parameter subclass but not another one.
Side note: @kinow, I'm looking at the `ImagingParameters` class and I can't find any usages of the `getImageFormat()` method elsewhere in the code. Is that method used anywhere?
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] darkma773r commented on a change in pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
darkma773r commented on a change in pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#discussion_r671476705
##########
File path: src/main/java/org/apache/commons/imaging/ImageParser.java
##########
@@ -106,7 +98,7 @@
*
* @return A valid array of image parsers
*/
- public static ImageParser[] getAllImageParsers() {
+ public static ImageParser<?>[] getAllImageParsers() {
Review comment:
Should this return a list for better compatibility with generics?
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-914929304
Just wrote an e-mail to commons-dev mailing list about the generics issue. Let's wait and see if there's any feedback :+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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-841797895
Whew, I knew we relied on awt, but looks like we are using it in a lot more places than I imagined. Let me update the PR to use a rectangle as ou suggested @gwlucastrig . 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] gwlucastrig commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
gwlucastrig commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-919020286
I wonder whether we might be able to use Reflection and move the static resolveParameters() method into the base class so that it only appears once. resolveParameters would take a class as an additional argument. That approach might let us get rid of the two constructors (copy and no-argument) which I also saw as error-prone.
I'll have to review Java Generics a bit. I almost never use them beyond the implementations supplied by the Java API. Perhaps in this case they can be applied to simplify our problem.
Also, I think my idea sort of falls apart because the TiffImagingParameters does not derive directly from ImagingParameters, but that there is an intermediate class: TiffImagingParameters extends XmpImagingParameters.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a change in pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on a change in pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#discussion_r671615265
##########
File path: src/main/java/org/apache/commons/imaging/ImagingParameters.java
##########
@@ -0,0 +1,108 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.imaging;
+
+import org.apache.commons.imaging.common.BufferedImageFactory;
+
+/**
+ * Imaging parameters.
+ *
+ * <p>Contains parameters that are common to all formats. Implementations must include
+ * the specific parameters for each image format.</p>
+ *
+ * @since 1.0-alpha3
+ */
+public class ImagingParameters {
+
+ /**
+ * Whether to throw an exception when any issue occurs during reading
+ * or writing a file format. Default is {@code false}.
+ */
+ private boolean strict = false;
+
+ /**
+ * An optional file name, used for the description of input streams
+ * where a file name would be hard (or not possible) to be identified.
+ * Default is {@code null}.
+ */
+ private String fileName = null;
+
+ /**
+ * Factory to create {@code BufferedImage}s. Default is {@code null}.
+ */
+ private BufferedImageFactory bufferedImageFactory = null;
+
+ /**
+ * Image format used in write operations to indicate desired image format.
+ * Default is {@code null}.
+ *
+ * <p>Valid values: Any format defined in ImageFormat, such as
+ * ImageFormat.IMAGE_FORMAT_PNG.</p>
+ *
+ * @see org.apache.commons.imaging.ImageFormats
+ */
+ private ImageFormat imageFormat;
Review comment:
Excellent idea! That reduced the code smell, where I had a `setImageFormat` in the constructor of each parameter class and, as you pointed, users could create objects with invalid states. Thanks @darkma773r!
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] coveralls edited a comment on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-841637226
[](https://coveralls.io/builds/41387908)
Coverage decreased (-0.1%) to 76.579% when pulling **aabd298d703efb5b917506d1f6bc6eddfece3c87 on kinow:IMAGING-159-1** into **a87b545e1019b43bfbe31fa5df3a4f3095a8b438 on apache:master**.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] darkma773r commented on a change in pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
darkma773r commented on a change in pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#discussion_r671481293
##########
File path: src/main/java/org/apache/commons/imaging/ImageParser.java
##########
@@ -79,25 +78,18 @@
* that the documentation is perfect, especially in the more obscure
* and specialized areas of implementation.
*
- * <h3>The "Map params" argument</h3>
+ * <h3>The "params" argument</h3>
*
- * Many of the methods specified by this class accept an argument of
- * type Map giving a list of parameters to be used when processing an
- * image. For example, some of the output formats permit the specification
+ * <p>Many of the methods specified by this class accept an argument of
+ * type {@code T} defining the parameters to be used when
+ * processing an image. For example, some of the output formats permit
* of different kinds of image compression or color models. Some of the
* reading methods permit the calling application to require strict
- * format compliance. In many cases, however, an application will not
- * require the use of this argument. While some of the ImageParser
- * implementations check for (and ignore) null arguments for this parameter,
- * not all of them do (at least not at the time these notes were written).
- * Therefore, a prudent programmer will always supply an valid, though
- * empty instance of a Map implementation when calling such methods.
- * Generally, the java HashMap class is useful for this purpose.
+ * format compliance.</p>
*
- * <p>Additionally, developers creating or enhancing classes derived
- * from ImageParser are encouraged to include such checks in their code.
+ * @param <T> type of parameters used by this image parser
*/
-public abstract class ImageParser extends BinaryFileParser {
+public abstract class ImageParser<T extends ImagingParameters> extends BinaryFileParser {
Review comment:
Would it make sense to remove this generic parameter and only accept `ImagingParameters` in methods? Implementations could downcast as needed to access type-specific parameters and this would simplify usage where the image format is not known (as in `Imaging`).
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-958618064
>> If the JpegImageParser requires a parameter different than the TiffImageParser, I assume we would have to create a new JpegImagingParameter. In that case, wouldn't we have to keep the old constructor for backward compatibility until a new major release?
>
>I'm not totally sure what you mean here. Can you give an example?
Sorry, I wasn't very clear. At the moment we have `JpegImageParser parser = new JpegImageParser();`. Internally, the `JpegImageParser` is using a `TiffImagingParameters`.
But if we need to add a parameter that is used only by the `JpegImageParser`, I think we would want to have a `JpegImagingParameters`. In which case I am not sure if we would be able to maintain backward compatibility by replacing the `TiffImagingParameters` in the `JpegImageParser`.
>since it makes it it kind of hard to tell what parameters to use when creating images of a certain type.
:point_up: this is my main concern. Avoid users having to guess what's the parameter that they want to use when parsing a Jpeg, Tiff, Gif, etc. Preferably something that their IDE's can assist with, using auto-complete for example.
>One idea for making this readily available in the API would be to add convenience factory methods in Imaging for creating instances of each parameters type. Ex:
Possibly another design to consider/experiment :-)
>Side note: Is there a reason the JpegImageParser uses TiffImageParameters? The two formats are not related, correct? Perhaps we could have a common base class for them?
We can probably avoid that. I agree it doesn't make much sense for the parameters to have a relationship like that. Currently, [`JpegImageParser`](https://github.com/apache/commons-imaging/blob/0ccabc3416cecf25cdec90a522f0912baf19bc58/src/main/java/org/apache/commons/imaging/formats/jpeg/JpegImageParser.java#L751) has some code in common with the `TiffImageParser` for parsing EXIF metadata. I don't recall 100%, but I believe that's why I implemented everything in the `TiffImagingParameters` and, realizing the Jpeg parameters were the same, made the parent-child class relantionship.
>Is the idea that it is confusing on what properties are copied over?
Not at all, that part is clear and looks OK. It's with users being able to use the wrong parameter class by accident, and realizing only in runtime iff a runtime exception is thrown (or in the worst case, no exception is thrown and the program behaves in a wrong way; debugging an issue like that can be annoying I think).
>I attempted to retain the null parameter functionality. In the normalizeParameters method, if a null parameter is passed, a default instance is created and returned, allowing downstream code to bypass null checks. mvn clean install passes with my current setup. Is there a unit test I missed?
Ah, my bad! Your solution works with the method I was looking at. I wrote this one before looking at the method that creates the default parameters :+1:
I need to find some time to “get in the zone” and try a few things. But it would be, mainly, to prevent users being able to use the wrong parameter types by accident, and making it so that we can add/remove parameters (deprecating what was removed) in the 1.x release series without worrying about backward compatibility. If we are able to modify your PR again to accommodate these two requirements, I think we would have the perfect solution :-)
Thanks a lot!
Bruno
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #116: [IMAGING-159] Add ImagingParameters interface and BaseParameters (POJO)
Posted by GitBox <gi...@apache.org>.
kinow commented on pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-1013758565
@darkma773r applied all changes exactly as suggested, and created two follow-up JIRA issues. Unless there are any last minute review requests, I think we are ready to merge it! :tada:
Thanks a lot @darkma773r , @gwlucastrig , and also @britter who managed the first release vote and created the issue for parameters. I remember all effort put into moving from sanselan to imaging and then the 1.0 work, which we should be ready to prepare a final release soon. Really appreciate it :bow:
-Bruno
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org