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

[Alexey Ivanov]

On Mon, 12 Feb 2024 12:33:30 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:
> 
>   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