RFR: 8307246 : Printing: banded raster path doesn't account for device offset values [v9]

Prasanta Sadhukhan psadhukhan at openjdk.org
Thu Feb 1 07:49:06 UTC 2024


On Thu, 1 Feb 2024 06:48:15 GMT, vtstydev <duke at openjdk.org> wrote:

>> More correct way to take in consideration nonzero PHYSICALOFFSETX, PHYSICALOFFSETY of device for banded-raster printing loop. Only on Windows platform under certain conditions real device prints shifted image on paper.
>
> vtstydev has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Remove trailing workspaces

src/java.desktop/share/classes/sun/print/RasterPrinterJob.java line 2156:

> 2154:          */
> 2155:         Paper paper = page.getPaper();
> 2156:         // if non-portrait status and 270 degree landscape rotation

guess this change is not needed..doesn't add anything new to present comment..

src/java.desktop/share/classes/sun/print/RasterPrinterJob.java line 2438:

> 2436:                         painter.print(painterGraphics, origPage, pageIndex);
> 2437:                         painterGraphics.dispose();
> 2438:                         printBand(data, bandX, bandTop+bandY, bandWidth, bandHeight);

similar spacing needed here too between operators

test/jdk/java/awt/print/PrinterJob/ImagePrinting/AlphaPrintingOffsets.java line 74:

> 72:         if (PrinterJob.lookupPrintServices().length > 0) {
> 73: 
> 74:             String instructionsHeader = "This test prints 6 pages with same image except text messages. \n";

I am not sure what "except text messages" conveys in the instructions? 
I guess this should be "This test prints 6 pages with page margin rectangle and a text message"
We told the checking criteria later "Please check the page margin rectangle are properly drawn and visible on all sides on all pages"

test/jdk/java/awt/print/PrinterJob/ImagePrinting/AlphaPrintingOffsets.java line 75:

> 73: 
> 74:             String instructionsHeader = "This test prints 6 pages with same image except text messages. \n";
> 75:             if (args.length > 0)

{} is needed even for single line as per coding guidelines

test/jdk/java/awt/print/PrinterJob/ImagePrinting/AlphaPrintingOffsets.java line 77:

> 75:             if (args.length > 0)
> 76:                 isAlphaTestModeSet = args[0].equals("testAlpha");
> 77:             if(isAlphaTestModeSet)

space between if and ( and {} is needed

test/jdk/java/awt/print/PrinterJob/ImagePrinting/AlphaPrintingOffsets.java line 78:

> 76:                 isAlphaTestModeSet = args[0].equals("testAlpha");
> 77:             if(isAlphaTestModeSet)
> 78:                 instructionsHeader = "This test prints 2 pages with same image except text messages. \n";

same here for "except text messages"

test/jdk/java/awt/print/PrinterJob/ImagePrinting/AlphaPrintingOffsets.java line 138:

> 136:         Book bookAlphaTest = new Book();
> 137:         bookAlphaTest.append(printableTransparent, pageFormatL);
> 138:         bookAlphaTest.append(printableTransparent, pageFormatRL);

I guess it can be moved inside if (isAlphaModeSet) and bookNoAlphaTest in else part....
Why to instantiate when we are not going to test bookNoAlphaTest bydefault?

test/jdk/java/awt/print/PrinterJob/ImagePrinting/AlphaPrintingOffsets.java line 140:

> 138:         bookAlphaTest.append(printableTransparent, pageFormatRL);
> 139: 
> 140:         if(isAlphaTestModeSet)

{} is needed for if and else condition

test/jdk/java/awt/print/PrinterJob/ImagePrinting/AlphaPrintingOffsets.java line 194:

> 192:                 pageFormat.getImageableWidth(), pageFormat.getImageableHeight());
> 193: 
> 194:         if(AlphaPrintingOffsets.getAlphaTestModeSet())

Please add {} and space after "if"

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/17030#discussion_r1473931951
PR Review Comment: https://git.openjdk.org/jdk/pull/17030#discussion_r1473901302
PR Review Comment: https://git.openjdk.org/jdk/pull/17030#discussion_r1473921000
PR Review Comment: https://git.openjdk.org/jdk/pull/17030#discussion_r1473905552
PR Review Comment: https://git.openjdk.org/jdk/pull/17030#discussion_r1473904958
PR Review Comment: https://git.openjdk.org/jdk/pull/17030#discussion_r1473921855
PR Review Comment: https://git.openjdk.org/jdk/pull/17030#discussion_r1473926391
PR Review Comment: https://git.openjdk.org/jdk/pull/17030#discussion_r1473913023
PR Review Comment: https://git.openjdk.org/jdk/pull/17030#discussion_r1473906470


More information about the client-libs-dev mailing list