Improve debug output #1115

rolfhowarth · 2025-03-24T16:42:35Z

Include hex opcode and file offset in debug output. Condense scan line messages in readOpDirectBits().

haraldk

First of all, thanks for taking the time to prepare all these PRs! Much appreciated!

I agree with the main intent of this PR, to clean up the debug output.

However, I need you to fix a few things:

Set your code style correctly, using 4 spaces as tab/indent, no tabs, 8 spaces continuation indent (but please only reformat the lines you have changed 😀 )
Remove the outcommented code, we don't need it (I know I have sinned against this myself, but let's make it better)

haraldk · 2025-03-25T14:13:09Z

imageio/imageio-pict/src/main/java/com/twelvemonkeys/imageio/plugins/pict/PICTImageReader.java

+                if (DEBUG) {
+                	System.out.println("--- opCode 0x"+Integer.toHexString(opCode)+" at pos 0x"+Long.toHexString(pStream.getStreamPosition()-2)+" ---");
+                }


I'm not so fond of the "---" pre-/postfix, keep it clean. 😀
Also, please use System.out.printf or String.format instead of the string concatenation. Use %04x or %08x as format strings, and skip the toHexString methods.

haraldk · 2025-03-25T14:13:39Z

imageio/imageio-pict/src/main/java/com/twelvemonkeys/imageio/plugins/pict/PICTImageReader.java

+//      // Line break at the end
+//      if (DEBUG) {
+//          System.out.println();
+//      }


I think we can just remove these line, no need to comment out. We have version control. 😀

haraldk · 2025-03-25T14:15:53Z

imageio/imageio-pict/src/main/java/com/twelvemonkeys/imageio/plugins/pict/PICTImageReader.java

+                if (DEBUG && (scanline<6 || scanline>=srcRect.height-5)) {
+                	if (scanline==5 && srcRect.height>10) {
+                		System.out.println("...");
+                	} else {
+	                    System.out.print("Line " + scanline + ", byteCount: " + packedBytesCount);
+	                    System.out.print(" dstBytes: " + dstBytes.length);
+	                    System.out.println();
+                	}


I'm not sure if I follow the logic here...

What is the intended output and why?

The point of this was just to reduce "noise" from the debug output. And the point of hyphens was so you could see each opcode, see for example following:

Done reading PICT header! --- opCode 0x1e at pos 0x228 --- opDefHilite --- opCode 0x1 at pos 0x22a --- clipRgn: java.awt.Rectangle[x=0,y=0,width=64,height=64] --- opCode 0x9a at pos 0x236 --- directBitsRect, rowBytes: 256, it is a PixMap, bounds: java.awt.Rectangle[x=0,y=0,width=64,height=64], pmVersion: 0, packType: 4, packSize: 0 hRes: 72.0, vRes: 72.0, RGBDirect, pixelSize:32, cmpCount:3, cmpSize:8, srcRect:java.awt.Rectangle[x=0,y=0,width=64,height=64], dstRect:java.awt.Rectangle[x=0,y=0,width=64,height=64], mode: 0 Line 0, byteCount: 4 dstBytes: 192 Line 1, byteCount: 4 dstBytes: 192 Line 2, byteCount: 4 dstBytes: 192 Line 3, byteCount: 4 dstBytes: 192 Line 4, byteCount: 4 dstBytes: 192 ... Line 59, byteCount: 4 dstBytes: 192 Line 60, byteCount: 4 dstBytes: 192 Line 61, byteCount: 4 dstBytes: 192 Line 62, byteCount: 4 dstBytes: 192 Line 63, byteCount: 4 dstBytes: 192 --- opCode 0xff at pos 0x550 --- Done reading PICT body!

Happy to leave both of these out and follow your preferred style though.

Ah, now I see! :-)

Makes sense. I think however it would be easier to read with something like:

Suggested change

if (DEBUG && (scanline<6 || scanline>=srcRect.height-5)) {

if (scanline==5 && srcRect.height>10) {

System.out.println("...");

} else {

System.out.print("Line " + scanline + ", byteCount: " + packedBytesCount);

System.out.print(" dstBytes: " + dstBytes.length);

System.out.println();

}

if (DEBUG) {

if (scanline < 5 || scanline >= srcRect.height - 5) {

System.out.print("Line " + scanline + ", byteCount: " + packedBytesCount);

System.out.print(" dstBytes: " + dstBytes.length);

System.out.println();

}

else if (scanline == 5) {

// Truncate any remaining scanlines...

System.out.println("...");

}

}

Or, with my preferred style these days (this code is old):

Suggested change

if (DEBUG && (scanline<6 || scanline>=srcRect.height-5)) {

if (scanline==5 && srcRect.height>10) {

System.out.println("...");

} else {

System.out.print("Line " + scanline + ", byteCount: " + packedBytesCount);

System.out.print(" dstBytes: " + dstBytes.length);

System.out.println();

}

if (DEBUG) {

if (scanline < 5 || scanline >= srcRect.height - 5) {

System.out.printf("Line %d, byteCount: %d dstBytes: %d%n", scanline, packedBytesCount, dstBytes.length);

}

else if (scanline == 5) {

// Truncate any remaining scanlines...

System.out.println("...");

}

}

haraldk · 2025-03-25T14:16:08Z

imageio/imageio-pict/src/main/java/com/twelvemonkeys/imageio/plugins/pict/PICTImageReader.java

+//        // Line break at the end
+//        if (DEBUG) {
+//            System.out.println();
+//        }


Just remove it.

haraldk · 2025-03-25T14:16:21Z

imageio/imageio-pict/src/main/java/com/twelvemonkeys/imageio/plugins/pict/PICTImageReader.java

@@ -2558,7 +2565,7 @@ private void verboseRegionCmd(String pCmd, Rectangle pBounds, Area pRegion) {
 //        for (int i = 1; pPolygon != null && i < pPolygon.npoints; i++) {
 //            System.out.print(", (" + pPolygon.xpoints[i] + "," + pPolygon.ypoints[i] + ")");
 //        }
-        System.out.println();
+//        System.out.println();


haraldk · 2025-03-25T14:18:17Z

imageio/imageio-pict/src/main/java/com/twelvemonkeys/imageio/plugins/pict/PICTImageReader.java

@@ -2653,6 +2660,7 @@ public static void main(final String[] pArgs) {
            try {
                ImageInputStream input = ImageIO.createImageInputStream(file);
                String title = file.getName();
+                System.out.println("======= "+title+" =======");


Don't think we need this.

rolfhowarth · 2025-03-28T18:23:13Z

Ok, I think I've made all these changes but I'm not 100% sure as I'm still not very familiar with GitHub!

haraldk

Just reformat the changes according to the code style, and this PR is good! 👍🏻

imageio/imageio-pict/src/main/java/com/twelvemonkeys/imageio/plugins/pict/PICTImageReader.java

…lugins/pict/PICTImageReader.java Co-authored-by: Harald Kuhr <harald.kuhr@gmail.com>

haraldk

As said, the code changes in this PR are all good, but the formatting has to match the code style.

It's easier if you just set it up correctly from the start. 😀
But I made it very easy for you now, just accept the suggested changes here from the Github UI (web}.

Update: I figured I could just commit the changes directly, so I did. 😀

imageio/imageio-pict/src/main/java/com/twelvemonkeys/imageio/plugins/pict/PICTImageReader.java

…lugins/pict/PICTImageReader.java

rolfhowarth · 2025-03-31T20:22:18Z

Great thank you! Like you, I take code formatting very seriously, but I have my own personal style I adhere to. But when checking in changes to existing code the rule is always: follow the existing style :-)

Improve debug output

1ae8d97

Include hex opcode and file offset in debug output. Condense scan line messages in readOpDirectBits().

haraldk reviewed Mar 25, 2025

View reviewed changes

Apply haraldk review comments

3352652

rolfhowarth requested a review from haraldk March 28, 2025 18:22

haraldk requested changes Mar 28, 2025

View reviewed changes

imageio/imageio-pict/src/main/java/com/twelvemonkeys/imageio/plugins/pict/PICTImageReader.java Outdated Show resolved Hide resolved

Update imageio/imageio-pict/src/main/java/com/twelvemonkeys/imageio/p…

0efd01d

…lugins/pict/PICTImageReader.java Co-authored-by: Harald Kuhr <harald.kuhr@gmail.com>

This was referenced Mar 31, 2025

Fix rendering of images with JPEG slices #1124

Open

Fix rendering of images with JPEG slices #1116

Closed

haraldk requested changes Mar 31, 2025

View reviewed changes

imageio/imageio-pict/src/main/java/com/twelvemonkeys/imageio/plugins/pict/PICTImageReader.java Outdated Show resolved Hide resolved

imageio/imageio-pict/src/main/java/com/twelvemonkeys/imageio/plugins/pict/PICTImageReader.java Outdated Show resolved Hide resolved

haraldk added 2 commits March 31, 2025 21:31

Update imageio/imageio-pict/src/main/java/com/twelvemonkeys/imageio/p…

d769cb7

…lugins/pict/PICTImageReader.java

Update imageio/imageio-pict/src/main/java/com/twelvemonkeys/imageio/p…

29f65bd

…lugins/pict/PICTImageReader.java

haraldk approved these changes Mar 31, 2025

View reviewed changes

haraldk merged commit f8b919e into haraldk:master Mar 31, 2025
21 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve debug output #1115

Improve debug output #1115

rolfhowarth commented Mar 24, 2025

haraldk left a comment

haraldk Mar 25, 2025

haraldk Mar 25, 2025

haraldk Mar 25, 2025

rolfhowarth Mar 25, 2025

haraldk Mar 25, 2025

haraldk Mar 25, 2025

haraldk Mar 25, 2025

haraldk Mar 25, 2025

rolfhowarth commented Mar 28, 2025

haraldk left a comment

haraldk left a comment •

edited

Loading

rolfhowarth commented Mar 31, 2025

Improve debug output #1115

Improve debug output #1115

Conversation

rolfhowarth commented Mar 24, 2025

haraldk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rolfhowarth commented Mar 28, 2025

haraldk left a comment

Choose a reason for hiding this comment

haraldk left a comment • edited Loading

Choose a reason for hiding this comment

rolfhowarth commented Mar 31, 2025

haraldk left a comment •

edited

Loading