gwlucastrig commented on a change in pull request #106:
URL: https://github.com/apache/commons-imaging/pull/106#discussion_r522422434



##########
File path: 
src/main/java/org/apache/commons/imaging/formats/tiff/datareaders/DataReaderTiled.java
##########
@@ -141,32 +141,37 @@ private void interpretTile(final ImageBuilder 
imageBuilder, final byte[] bytes,
                 // the tile is padded to beyond the tile width
                 j1 = xLimit;
             }
-            if (predictor == 2) {
-                // pre-apply the predictor logic before feeding
-                // the bytes to the photometric interpretor.
+
+            if (predictor == 
TiffTagConstants.PREDICTOR_VALUE_HORIZONTAL_DIFFERENCING) {
+                applyPredictorToBlock(tileWidth, i1 - i0, samplesPerPixel, 
bytes);
+            }
+
+            if (bitsPerPixel == 24) {
+                // 24 bit case, we don't mask the red byte because any
+                // sign-extended bits get covered by opacity mask
                 for (int i = i0; i < i1; i++) {
                     int k = (i - i0) * tileWidth * 3;
-                    int p0 = bytes[k++] & 0xff;
-                    int p1 = bytes[k++] & 0xff;
-                    int p2 = bytes[k++] & 0xff;
-                    for (int j = 1; j < tileWidth; j++) {
-                        p0 = (bytes[k] + p0) & 0xff;
-                        bytes[k++] = (byte) p0;
-                        p1 = (bytes[k] + p1) & 0xff;
-                        bytes[k++] = (byte) p1;
-                        p2 = (bytes[k] + p2) & 0xff;
-                        bytes[k++] = (byte) p2;
+                    for (int j = j0; j < j1; j++, k += 3) {
+                        final int rgb = 0xff000000
+                            | (bytes[k] << 16)
+                            | ((bytes[k + 1] & 0xff) << 8)
+                            | (bytes[k + 2] & 0xff);
+                        imageBuilder.setRGB(j, i, rgb);
                     }
                 }
-            }
-
-            for (int i = i0; i < i1; i++) {
-                int k = (i - i0) * tileWidth * 3;
-                for (int j = j0; j < j1; j++, k += 3) {
-                    final int rgb = 0xff000000
-                        | (((bytes[k] << 8) | (bytes[k + 1] & 0xff)) << 8)
-                        | (bytes[k + 2] & 0xff);
-                    imageBuilder.setRGB(j, i, rgb);
+            } else if (bitsPerPixel == 32) {
+                // 32 bit case, we don't mask the high byte because any
+                // sign-extended bits get shifted up and out of result.
+                for (int i = i0; i < i1; i++) {
+                    int k = (i - i0) * tileWidth * 4;
+                    for (int j = j0; j < j1; j++, k += 4) {
+                        final int rgb
+                            = ((bytes[k] & 0xff) << 16)
+                            | ((bytes[k + 1] & 0xff) << 8)
+                            | (bytes[k + 2] & 0xff)
+                            | (bytes[k + 3] << 24);
+                        imageBuilder.setRGB(j, i, rgb);
+                    }

Review comment:
       Data reader strips and data reader tiles have always had a lot of 
overlap.  Really, a strip and a tile are both rectangles filled with pixels... 
Though TIFF has slightly different rules for cases that run off the edges of 
images...  Someday, I'd like to take a look at just consolidating the two 
classes. But it's going to be a pretty big change to the existing reader 
classes.




----------------------------------------------------------------
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:
[email protected]


Reply via email to