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 2020/10/29 00:14:26 UTC
[GitHub] [commons-imaging] kinow commented on a change in pull request #105: [Imaging-216] Add support for Alpha channel in RGB images
kinow commented on a change in pull request #105:
URL: https://github.com/apache/commons-imaging/pull/105#discussion_r513832426
##########
File path: src/test/java/org/apache/commons/imaging/formats/tiff/TiffReadAlphaTest.java
##########
@@ -0,0 +1,91 @@
+/*
+ * 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.formats.tiff;
+
+import java.awt.Color;
+import java.awt.Graphics2D;
+import java.awt.image.BufferedImage;
+import java.io.File;
+import java.io.IOException;
+
+import org.apache.commons.imaging.ImageReadException;
+import org.apache.commons.imaging.Imaging;
+import org.apache.commons.imaging.ImagingTestConstants;
+
+import org.junit.jupiter.api.Test;
+import static org.junit.jupiter.api.Assertions.*;
+
+
+
+/**
+ * Performs tests that access the content of TIFF files
+ * containing non-opaque alpha-channel pixels
+ */
+public class TiffReadAlphaTest {
+ private final static String []names = {
+ "TransparencyTestStripAssociated.tif",
+ "TransparencyTestStripUnassociated.tif",
+ "TransparencyTestTileAssociated.tif",
+ "TransparencyTestTileUnassociated.tif"
+ };
+
+ private final static int [][] testSite = {
+ {40, 40, 0xffff0000},
+ {60, 40, 0xff77ff77},
+ {40, 60, 0xffff0000},
+ {60, 60, 0xff008800}
+ };
+
+ /**
+ * Gets a file from the TIFF test directory that contains floating-point
+ * data.
+ *
+ * @param name a valid file name
+ * @return a valid file reference.
+ */
+ private File getTiffFile(String name) {
+ File tiffFolder = new File(ImagingTestConstants.TEST_IMAGE_FOLDER, "tiff");
+ File alphaFolder = new File(tiffFolder, "12");
+ return new File(alphaFolder, name);
+ }
Review comment:
I think this is a windows CRLF?
##########
File path: src/test/java/org/apache/commons/imaging/formats/tiff/TiffReadAlphaTest.java
##########
@@ -0,0 +1,91 @@
+/*
+ * 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.formats.tiff;
+
+import java.awt.Color;
+import java.awt.Graphics2D;
+import java.awt.image.BufferedImage;
+import java.io.File;
+import java.io.IOException;
+
+import org.apache.commons.imaging.ImageReadException;
+import org.apache.commons.imaging.Imaging;
+import org.apache.commons.imaging.ImagingTestConstants;
+
+import org.junit.jupiter.api.Test;
+import static org.junit.jupiter.api.Assertions.*;
+
+
+
+/**
+ * Performs tests that access the content of TIFF files
+ * containing non-opaque alpha-channel pixels
+ */
+public class TiffReadAlphaTest {
+ private final static String []names = {
+ "TransparencyTestStripAssociated.tif",
+ "TransparencyTestStripUnassociated.tif",
+ "TransparencyTestTileAssociated.tif",
+ "TransparencyTestTileUnassociated.tif"
+ };
+
+ private final static int [][] testSite = {
+ {40, 40, 0xffff0000},
+ {60, 40, 0xff77ff77},
+ {40, 60, 0xffff0000},
+ {60, 60, 0xff008800}
+ };
+
+ /**
+ * Gets a file from the TIFF test directory that contains floating-point
+ * data.
+ *
+ * @param name a valid file name
+ * @return a valid file reference.
+ */
+ private File getTiffFile(String name) {
+ File tiffFolder = new File(ImagingTestConstants.TEST_IMAGE_FOLDER, "tiff");
+ File alphaFolder = new File(tiffFolder, "12");
+ return new File(alphaFolder, name);
+ }
+ @Test
+ public void test() {
+ for(String name:names){
+ try{
+ File subject = getTiffFile(name);
+ BufferedImage overlay = Imaging.getBufferedImage(subject);
+ BufferedImage composite = new BufferedImage(100, 100, BufferedImage.TYPE_INT_ARGB);
+ Graphics2D g2d = composite.createGraphics();
+ g2d.setColor(Color.white);
+ g2d.fillRect(0, 0, 101, 101);
+ g2d.setColor(Color.black);
+ g2d.fillRect(0, 50, 101, 51);
+ g2d.drawImage(overlay, 0, 0, null);
+
+ for(int i=0; i<testSite.length; i++){
+ int x = testSite[i][0];
+ int y = testSite[i][1];
+ int p = testSite[i][2];
+ int t = composite.getRGB(x, y);
+ assertEquals(t, p, "Error for "+name+" at position "+x+", "+y);
+ }
+ }catch(ImageReadException | IOException ex){
+ fail("Exception reading "+name+", "+ex.getMessage());
+ }
+ }
+ }
+}
Review comment:
Missing newline
##########
File path: src/test/java/org/apache/commons/imaging/formats/tiff/TiffAlphaRoundTripTest.java
##########
@@ -0,0 +1,137 @@
+/*
+ * 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.formats.tiff;
+
+import java.awt.AlphaComposite;
+import java.awt.Color;
+import java.awt.Graphics2D;
+import java.awt.image.BufferedImage;
+import java.io.File;
+import java.nio.file.Path;
+import java.util.HashMap;
+
+import org.apache.commons.imaging.ImageFormats;
+import org.apache.commons.imaging.Imaging;
+
+import org.junit.jupiter.api.Test;
+import static org.junit.jupiter.api.Assertions.*;
+import org.junit.jupiter.api.io.TempDir;
+
+
+
+/**
+ * Performs a round-trip that writes an image containing Alpha and then reads it back.
+ * Selected non-opaque pixels are tested for correctness,
+ */
+public class TiffAlphaRoundTripTest {
+
+
+ @TempDir
+ Path tempDir;
+
+ @Test
+ public void test() throws Exception {
+
+ // This test will exercise two passes to test the implementation
+ // of the TIFF support for writing and reading images containing
+ // an alpha channel. In the first pass, the alpha writing is enabled
+ // in the second pass it is suppressed.
+ for (int i = 0; i < 2; i++) {
+ // Step 0, create a buffered image that includes transparency
+ // in the form of two rectangles, one completely opaque,
+ // and one giving 50 percent opaque red.
+ int width = 400;
+ int height = 400;
+ BufferedImage image0;
+ if (i == 0) {
+ image0 = new BufferedImage(width, height, BufferedImage.TYPE_INT_ARGB);
+ } else {
+ image0 = new BufferedImage(width, height, BufferedImage.TYPE_INT_RGB);
+ }
+ Graphics2D g2d = image0.createGraphics();
+ g2d.setColor(Color.red);
+ g2d.fillRect(0, 0, width, height);
+ g2d.setComposite(AlphaComposite.getInstance(AlphaComposite.SRC));
+ g2d.setColor(new Color(0, 0, 0, 0));
+ g2d.fillRect(100, 100, 100, 100);
+ g2d.setColor(new Color(0xff, 0, 0, 0x80));
+ g2d.fillRect(200, 200, 100, 100);
+
+ // Step 1: write the Buffered Image to an output file and
+ // then read it back in. This action will test the
+ // correctness of a round-trip test.
+ File file = new File(tempDir.toFile(), "TiffAlphaRoundTripTest.tif");
+ file.delete();
+ HashMap<String, Object> params = new HashMap<>();
+
+ Imaging.writeImage(image0, file, ImageFormats.TIFF, params);
+ BufferedImage image1 = Imaging.getBufferedImage(file);
+
+ // Step 2: create a composite image overlaying a white background
+ // with the results from the TIFF file.
+ BufferedImage compImage
+ = new BufferedImage(width, height, BufferedImage.TYPE_INT_ARGB);
+ g2d = compImage.createGraphics();
+ g2d.setColor(Color.white);
+ g2d.fillRect(0, 0, width, height);
+ g2d.drawImage(image1, 0, 0, null);
+
+ // Step 3, verify that the correct values are in the image.
+ int test1 = compImage.getRGB(150, 150); // in the transparent rectangle
+ int test2 = compImage.getRGB(250, 250);
+ if (i == 0) {
+ doPixelsMatch(150, 150, 0xffffffff, test1);
+ doPixelsMatch(250, 250, 0xffff7f7f, test2);
+ } else {
+ doPixelsMatch(151, 151, 0xff000000, test1);
+ doPixelsMatch(251, 251, 0xffff0000, test2);
+ }
+ }
+ }
+
+
+
+ void doPixelsMatch(int x, int y, int a, int b) {
+ if (!componentMatch(a, b, 0, 2)
+ || !componentMatch(a, b, 8, 2)
+ || !componentMatch(a, b, 16, 2)
+ || !componentMatch(a, b, 24, 2)) {
+
+ String complaint = String.format("Pixel mismatch at (%d,%d): 0x%08x 0x%08x",
+ x, y, a, b);
+ fail(complaint);
+ }
+ }
+
+ /**
+ * Checks to see if a pixel component (A, R, G, or B) for two specified
+ * values are within a specified tolerance.
+ * @param a the first value
+ * @param b the second value
+ * @param iShift a multiple of 8 telling how far to shift values
+ * to extract components (24, 16, 8, or zero for ARGB)
+ * @param iTolerance a small positive integer
+ * @return true if the components of the values match
+ */
+ boolean componentMatch(int a, int b, int iShift, int iTolerance){
+ int delta = ((a >> iShift) & 0xff) - ((b >> iShift) & 0xff);
+ if(delta<0){
+ delta = -delta;
+ }
+ return delta<iTolerance;
+ }
+}
Review comment:
Missing newline
##########
File path: src/test/java/org/apache/commons/imaging/common/ImageBuilderTest.java
##########
@@ -0,0 +1,146 @@
+/*
+ * 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.common;
+
+import java.awt.image.BufferedImage;
+import java.awt.image.ColorModel;
+import java.awt.image.RasterFormatException;
+
+import org.junit.jupiter.api.Test;
+import static org.junit.jupiter.api.Assertions.*;
+
+/**
+ * Provides unit tests for the ImageBuilder class.
+ */
+public class ImageBuilderTest {
+
+
+ /**
+ * Test of bad dimensions in constructor
+ */
+ @Test
+ public void testConstructorBounds() {
+ executeBadConstructor(0, 10);
+ executeBadConstructor(10, 0);
+ }
+
+
+ /**
+ * Test of bad bounds in sub-image
+ */
+ @Test
+ public void testBoundsCheck() {
+
+ ImageBuilder imageBuilder = new ImageBuilder(100, 100, false );
+
+ executeBadBounds(imageBuilder, -1, 0, 50, 50);
+ executeBadBounds(imageBuilder, 0, -1, 50, 50);
+ executeBadBounds(imageBuilder, 0, 0, 0, 50);
+ executeBadBounds(imageBuilder, 0, 0, 50, 0);
+ executeBadBounds(imageBuilder, 90, 0, 50, 50);
+ executeBadBounds(imageBuilder, 0, 90, 50, 50);
+ }
+
+ /**
+ * Test whether sub-image is consistent with source
+ */
+ @Test
+ public void testSubimageAccess() {
+ ImageBuilder imageBuilder = new ImageBuilder(100, 100, false );
+ populate(imageBuilder);
+ BufferedImage bImage = imageBuilder.getSubimage(25, 25, 25, 25);
+ int w = bImage.getWidth();
+ int h = bImage.getHeight();
+ assertEquals(w, 25, "Width of subimage does not match");
+ assertEquals(h, 25, "Height of subimage does not match");
+
+ for(int x=25; x<50; x++){
+ for(int y=25; y<50; y++){
+ int k = bImage.getRGB(x-25, y-25);
+ int rgb = imageBuilder.getRGB(x, y);
+ assertEquals(k, rgb, "Invalid buffered image subpixel at "+x+", "+y);
+ }
+ }
+
+ ImageBuilder testBuilder = imageBuilder.getSubset(25, 25, 25, 25);
+ for(int x=25; x<50; x++){
+ for(int y=25; y<50; y++){
+ int k = testBuilder.getRGB(x-25, y-25);
+ int rgb = imageBuilder.getRGB(x, y);
+ assertEquals(k, rgb, "Invalid image builder subpixel at "+x+", "+y);
+ }
+ }
+ }
Review comment:
These tests :point_up: are for the other change with subset right? Just to confirm :+1:
##########
File path: src/main/java/org/apache/commons/imaging/common/ImageBuilder.java
##########
@@ -200,27 +244,7 @@ public ImageBuilder getSubset(final int x, final int y, final int w, final int h
* within this ImageBuilder
*/
public BufferedImage getSubimage(final int x, final int y, final int w, final int h) {
- if (w <= 0) {
- throw new RasterFormatException("negative or zero subimage width");
- }
- if (h <= 0) {
- throw new RasterFormatException("negative or zero subimage height");
- }
- if (x < 0 || x >= width) {
- throw new RasterFormatException("subimage x is outside raster");
- }
- if (x + w > width) {
- throw new RasterFormatException(
- "subimage (x+width) is outside raster");
- }
- if (y < 0 || y >= height) {
- throw new RasterFormatException("subimage y is outside raster");
- }
- if (y + h > height) {
- throw new RasterFormatException(
- "subimage (y+height) is outside raster");
- }
-
+ checkBounds(x, y, w, h);
Review comment:
:+1: Nice, avoiding code duplication!
##########
File path: src/test/java/org/apache/commons/imaging/formats/tiff/TiffAlphaRoundTripTest.java
##########
@@ -0,0 +1,137 @@
+/*
+ * 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.formats.tiff;
+
+import java.awt.AlphaComposite;
+import java.awt.Color;
+import java.awt.Graphics2D;
+import java.awt.image.BufferedImage;
+import java.io.File;
+import java.nio.file.Path;
+import java.util.HashMap;
+
+import org.apache.commons.imaging.ImageFormats;
+import org.apache.commons.imaging.Imaging;
+
+import org.junit.jupiter.api.Test;
+import static org.junit.jupiter.api.Assertions.*;
+import org.junit.jupiter.api.io.TempDir;
+
+
+
+/**
+ * Performs a round-trip that writes an image containing Alpha and then reads it back.
+ * Selected non-opaque pixels are tested for correctness,
+ */
+public class TiffAlphaRoundTripTest {
+
+
+ @TempDir
+ Path tempDir;
+
Review comment:
Strange character, probably Windows CRLF?
##########
File path: src/main/java/org/apache/commons/imaging/formats/tiff/constants/TiffConstants.java
##########
@@ -70,6 +70,15 @@
public static final String PARAM_KEY_SUBIMAGE_WIDTH = "SUBIMAGE_WIDTH";
public static final String PARAM_KEY_SUBIMAGE_HEIGHT = "SUBIMAGE_HEIGHT";
+
+ /**
+ * Specifies that an application-specified photometric interpreter
+ * is to be used when reading TIFF files to convert raster data samples
+ * to RGB values for the output image.
+ * <p>
+ * The value supplied with this key should be a valid instance of
+ * a class that implements PhotometricInterpreter.
+ */
Review comment:
:+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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org