RFR: 8315113: Print request Chromaticity.MONOCHROME attribute does not work on macOS [v5]
Phil Race
prr at openjdk.org
Fri Mar 7 22:07:06 UTC 2025
On Fri, 27 Dec 2024 11:05:30 GMT, GennadiyKrivoshein <duke at openjdk.org> wrote:
>> This update allows users to print with grayscale using color printers.
>> Actually, it is not possible to use the "Monochrome" option from the "Color Appearance" panel. Also Chromaticity.MONOCHROME can't be used to print grayscale on color printers ([JDK-8315113](https://bugs.openjdk.org/browse/JDK-8315113)).
>>
>> **Fix description**
>> When a printer supports color printing and a user adds Chromaticity.MONOCHROME attribute to a PrintRequestAttributeSet, then the final printing raster is transformed to the grayscale color using java.awt.image.ColorConvertOp. When the job is a PostScript job, then the "setColor" and "setPaint" methods of the Graphics are overridden, and user colors (paints) are transformed to the grayscale form using the new proxy class GrayscaleProxyGraphics2D.
>>
>> This approach is assumed to be platform, CUPS, and IPP protocol independent.
>>
>> **Tests**
>> The fix was tested on Ubuntu 22.04 and MacOS 12.6.1.
>
> GennadiyKrivoshein has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 15 commits:
>
> - Update copyright, fix typos, move the proxy to the macos
> - Merge branch 'master' into monochrome-printing
> - proxy mac only
> - Revert "grammar fixes"
>
> This reverts commit 355b2b8f1dbc71cef433e9a925dfb8a7fff56f99.
> - Revert "formatting"
>
> This reverts commit fde514baeadc2775fa502a2a2d312c6038880e7a.
> - Revert "update copyright"
>
> This reverts commit 60e9b4f024544cfac4ddaddc1ea3653bd4a2fe4c.
> - Revert "move grayscale methods to PSPathGraphics"
>
> This reverts commit 1ef135680645ad2647c4430e852095dda8aa7e0c.
> - Merge remote-tracking branch 'openjdk/master' into monochrome-printing
>
> # Conflicts:
> # src/java.desktop/share/classes/sun/print/RasterPrinterJob.java
> - Merge remote-tracking branch 'openjdk/master' into monochrome-printing
> - move grayscale methods to PSPathGraphics
> - ... and 5 more: https://git.openjdk.org/jdk/compare/6c591854...5486473b
So this approach looks much better. It works as I'd expect.
My quibbles are all with the test.
src/java.desktop/macosx/classes/sun/print/GrayscaleProxyGraphics2D.java line 3:
> 1: /*
> 2: * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
> 3: * Copyright (c) 2024, BELLSOFT. All rights reserved.
I suggest updating everything to be 2025
test/jdk/javax/print/attribute/MonochromePrintTest.java line 63:
> 61: * @key printer
> 62: * @requires (os.family == "mac")
> 63: * @summary javax.print: Support monochrome printing
This needs to be marked as main/manual
In addition you should use the PassFailJFrame framework you will see widely used in other manual tests that have been added or updated in the last year.
test/jdk/javax/print/attribute/MonochromePrintTest.java line 94:
> 92: });
> 93:
> 94: long time = System.currentTimeMillis() + TIMEOUT;
There's an easy way to do this with PassFailJFrame
test/jdk/javax/print/attribute/MonochromePrintTest.java line 116:
> 114: if (testCount != testTotalCount) {
> 115: throw new Exception(
> 116: "Timeout: " + testCount + " tests passed out from " + testTotalCount);
I'm not sure where to put this comment, but the PASS and FAIL buttons are generally expected to mean "the whole test", so the flow wasn't what I expected. I think when you change this to use PassFailJFrame you'll find you need to change this to be conformant with how it works.
test/jdk/javax/print/attribute/MonochromePrintTest.java line 123:
> 121:
> 122: String[] instructions = {
> 123: "Two tests will run and it will test all available color apearances:",
spelling : should be "appearances"
test/jdk/javax/print/attribute/MonochromePrintTest.java line 124:
> 122: String[] instructions = {
> 123: "Two tests will run and it will test all available color apearances:",
> 124: Arrays.toString(supportedChromaticity) + "supported by the printer.",
you need to add spaces around the toString(..). Currently this is displayed as
"Two tests will run and it will test all available color apearances:[color, monochrome]supported by the printer"
test/jdk/javax/print/attribute/MonochromePrintTest.java line 137:
> 135: " The print dialog should appear.",
> 136: " - Select 'Appearance' tab.",
> 137: " - Select '" + chromaticity + "' on the 'Color apearance' panel.",
spelling again
test/jdk/javax/print/attribute/MonochromePrintTest.java line 144:
> 142: "A passing test will print the page with color appearance '" + chromaticity + "'.",
> 143: "The text, shapes and image should be " + chromaticity + ".",
> 144: "The test fails if the page is not printed with required color apearance.",
spelling again
test/jdk/javax/print/attribute/MonochromePrintTest.java line 199:
> 197: private static void print(Chromaticity chromaticity) throws PrinterException {
> 198: PrintRequestAttributeSet attr = new HashPrintRequestAttributeSet();
> 199: attr.add(MediaSizeName.ISO_A4);
You should remove this line .. A4 is not necessarily available.
test/jdk/javax/print/attribute/MonochromePrintTest.java line 213:
> 211: job.print();
> 212: } else {
> 213: throw new RuntimeException("Test for " + chromaticity + " is canceled!");
The test is very badly behaved if I press cancel. The exception is thrown on the Event Thread. Nothing catches it.
The user won't even see the message printed to stdout, and the test hangs, with all options greyed out.
test/jdk/javax/print/attribute/MonochromePrintTest.java line 291:
> 289:
> 290: BufferedImage bufferdImage = getBufferedImage((int) Math.ceil(pageFormat.getImageableWidth() / 3), (int) Math.ceil(pageFormat.getImageableHeight() / 7));
> 291: g.drawImage(bufferdImage, null, sx, sy);
Break up this long line
test/jdk/javax/print/attribute/MonochromePrintTest.java line 304:
> 302:
> 303: Paint paint = new LinearGradientPaint(0, 0, 20, 5, new float[]{0.0f, 0.2f, 1.0f},
> 304: new Color[]{Color.RED, Color.GREEN, Color.CYAN}, MultipleGradientPaint.CycleMethod.REPEAT);
Break up this long line
test/jdk/javax/print/attribute/MonochromePrintTest.java line 313:
> 311: new Color[]{Color.RED, Color.GREEN, Color.CYAN}, MultipleGradientPaint.CycleMethod.REPEAT);
> 312: g.setPaint(paint);
> 313: g.fillRect(sx, imh + 10, 50, 50);
Break up this long line
-------------
PR Review: https://git.openjdk.org/jdk/pull/21930#pullrequestreview-2668401193
PR Review Comment: https://git.openjdk.org/jdk/pull/21930#discussion_r1985742882
PR Review Comment: https://git.openjdk.org/jdk/pull/21930#discussion_r1985721419
PR Review Comment: https://git.openjdk.org/jdk/pull/21930#discussion_r1985724097
PR Review Comment: https://git.openjdk.org/jdk/pull/21930#discussion_r1985742170
PR Review Comment: https://git.openjdk.org/jdk/pull/21930#discussion_r1985725618
PR Review Comment: https://git.openjdk.org/jdk/pull/21930#discussion_r1985732809
PR Review Comment: https://git.openjdk.org/jdk/pull/21930#discussion_r1985733893
PR Review Comment: https://git.openjdk.org/jdk/pull/21930#discussion_r1985728533
PR Review Comment: https://git.openjdk.org/jdk/pull/21930#discussion_r1985726653
PR Review Comment: https://git.openjdk.org/jdk/pull/21930#discussion_r1985737096
PR Review Comment: https://git.openjdk.org/jdk/pull/21930#discussion_r1985727620
PR Review Comment: https://git.openjdk.org/jdk/pull/21930#discussion_r1985728016
PR Review Comment: https://git.openjdk.org/jdk/pull/21930#discussion_r1985728198
More information about the client-libs-dev
mailing list