On Mon, 12 Feb 2024 12:33:30 GMT, vtstydev <d...@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:
>
> Done requested fixes 4
src/java.desktop/share/classes/sun/print/RasterPrinterJob.java line 1:
> 1: /*
The copyright year should be updated to 2024?
test/jdk/java/awt/print/PrinterJob/ImagePrinting/AlphaPrintingOffsets.java line
34:
> 32: import static java.awt.print.PageFormat.LANDSCAPE;
> 33: import static java.awt.print.PageFormat.PORTRAIT;
> 34: import static java.awt.print.PageFormat.REVERSE_LANDSCAPE;
Static imports always go after regular ones.
// other imports
import javax.swing.JFrame;
import static java.awt.print.PageFormat.LANDSCAPE;
import static java.awt.print.PageFormat.PORTRAIT;
import static java.awt.print.PageFormat.REVERSE_LANDSCAPE;
test/jdk/java/awt/print/PrinterJob/ImagePrinting/AlphaPrintingOffsets.java line
57:
> 55: private static final String INSTRUCTIONS =
> 56: "Tested bug occurs only on-paper printing so you
> mustn't use PDF printer\n\n" +
> 57: "1.Java print dialog should appear.\n" +
Suggestion:
"1. Java print dialog should appear.\n" +
test/jdk/java/awt/print/PrinterJob/ImagePrinting/AlphaPrintingOffsets.java line
62:
> 60: "If so, press PASS, else press FAIL.\n\n" +
> 61: "Also you may run this test in paper-saving mode. Due
> to tested bug affects pages printed with transparency in LANDSCAPE and
> REVERSE_LANDSCAPE orientations " +
> 62: "there is an option to print only 2 pages affected.
> To do it pass PaperSavingMode parameter.";
These lines are longer than 80-column limit, they're longer than 120-column.
You can also reduce the indentation of the string, usually such lines are
indented by 8 spaces, so that the opening quote would align with ‘s’ in
`static` keyword.
test/jdk/java/awt/print/PrinterJob/ImagePrinting/AlphaPrintingOffsets.java line
155:
> 153: }
> 154:
> 155: class CustomPrintable implements Printable {
I suggest making `CustomPrintable` a static nested class in
`AlphaPrintingOffsets`.
It avoids conflicts when you mark a folder as test sources in an IDE; if
another test has a top-level class with the same name, `CustomPrintable`,
compilation of test sources fails because of duplicate class.
It does not affect jtreg though, so it's just a suggestion.
test/jdk/java/awt/print/PrinterJob/ImagePrinting/AlphaPrintingOffsets.java line
159:
> 157: private static final int MARGIN = 15;
> 158: private static final int SMALL_RECTANGLE_SIZE = 5;
> 159: private int alphaValue;
Suggestion:
private final int alphaValue;
The value of `alphaValue` doesn't change after it's set in the constructor.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17030#discussion_r1486711367
PR Review Comment: https://git.openjdk.org/jdk/pull/17030#discussion_r1486680812
PR Review Comment: https://git.openjdk.org/jdk/pull/17030#discussion_r1486690846
PR Review Comment: https://git.openjdk.org/jdk/pull/17030#discussion_r1486694161
PR Review Comment: https://git.openjdk.org/jdk/pull/17030#discussion_r1486708175
PR Review Comment: https://git.openjdk.org/jdk/pull/17030#discussion_r1486709040
