On Fri, 27 Dec 2024 11:05:30 GMT, GennadiyKrivoshein <d...@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
