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