Re: [OpenJDK 2D-Dev] RFR: 8247370: Clean up unused printing code in awt_PrintJob.cpp
On Thu, 18 Mar 2021 22:59:57 GMT, Phil Race wrote: > This removes a long time un-used code path. Maybe we should put noreg-cleanup in JBS. - Marked as reviewed by psadhukhan (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3083
Re: [OpenJDK 2D-Dev] RFR: 8247370: Clean up unused printing code in awt_PrintJob.cpp
On Thu, 18 Mar 2021 22:59:57 GMT, Phil Race wrote: > This removes a long time un-used code path. Marked as reviewed by serb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3083
[OpenJDK 2D-Dev] Integrated: 8263482: Make access to the ICC color profiles data multithread-friendly
On Fri, 12 Mar 2021 05:10:25 GMT, Sergey Bylokhov wrote: > FYI: probably is better/simpler to review it via webrev. > > After migration to the lcms from the kcms the performance of some operations > was regressed. One possible workaround was to split the operation into > multiple threads. But unfortunately, we have too many bottlenecks which > prevent using multithreading. This is the request to remove/minimize such > bottlenecks(at least some of them), but it does not affect the > single-threaded performance it should be handled separately. > > The main code pattern optimized here is this: > activate(); > byte[] theHeader = getData(cmmProfile, icSigHead); > > CMSManager.getModule().getTagData(p, tagSignature); > Notes about the code above: > > 1. Before the change "activate()" method checked that the "cmmProfile" field > was not null. After that we usually used the "cmmProfile" as the parameter to > some other method. This included two volatile reads, and also required to > check when we need to call the "activate()" method before usage of the > "cmmProfile" field. > Solution: "activate()" renamed to the "cmmProfile()" which became an accessor > for the field, so we will get one volatile read and can easily monitor the > usage of the field itself(it is used directly only in this method). > > 2. The synchronized static method "CMSManager.getModule()" reimplemented to > have only one volatile read. > > 3. The usage of locking in the "getTagData()" is changed. Instead of the > synchronized instance methods, we now use the mix of "ConcurrentHashMap" and > StampedLock. > > See some comments inline. > > Some numbers(small numbers are better): > > 1. Performance of ((ICC_ProfileRGB) > ICC_Profile.getInstance(ColorSpace.CS_sRGB)).getMatrix(); > > jdk 15.0.2 > Benchmark Mode CntScore Error Units > CMMPerf.ThreadsMAX.testGetMatrix avgt5 19,624 ±0,059 us/op > CMMPerf.testGetMatrix avgt50,154 ±0,001 us/op > > jdk - before the fix > Benchmark Mode CntScore Error Units > CMMPerf.ThreadsMAX.testGetMatrix avgt5 12,935 ±0,042 us/op > CMMPerf.testGetMatrix avgt50,127 ±0,007 us/op > > jdk - after the fix > Benchmark Mode CntScore Error Units > CMMPerf.ThreadsMAX.testGetMatrix avgt50,561 ±0,005 us/op > CMMPerf.testGetMatrix avgt50,092 ±0,001 us/op > > 2. Part of performance gain in jdk17 is from some other fixes, for example > Performance of ICC_Profile.getInstance(ColorSpace.CS_sRGB); and > ColorSpace.getInstance(ColorSpace.CS_sRGB); > > jdk 15.0.2 > Benchmark Mode CntScore Error Units > CMMPerf.ThreadsMAX.testGetSRGBProfile avgt52,299 ±0,032 us/op > CMMPerf.ThreadsMAX.testGetSRGBSpaceavgt52,210 ±0,051 us/op > CMMPerf.testGetSRGBProfile avgt50,019 ±0,001 us/op > CMMPerf.testGetSRGBSpace avgt50,018 ±0,001 us/op > > jdk - same before/ after the fix > Benchmark Mode CntScore Error Units > CMMPerf.ThreadsMAX.testGetSRGBProfile avgt50,005 ±0,001 us/op > CMMPerf.ThreadsMAX.testGetSRGBSpaceavgt50,005 ±0,001 us/op > CMMPerf.testGetSRGBProfile avgt50,005 ±0,001 us/op > CMMPerf.testGetSRGBSpace avgt50,005 ±0,001 us/op > > note "ThreadsMAX" is 32 threads. This pull request has now been integrated. Changeset: 1a21f779 Author:Sergey Bylokhov URL: https://git.openjdk.java.net/jdk/commit/1a21f779 Stats: 212 lines in 5 files changed: 52 ins; 83 del; 77 mod 8263482: Make access to the ICC color profiles data multithread-friendly Reviewed-by: azvegint - PR: https://git.openjdk.java.net/jdk/pull/2957
[OpenJDK 2D-Dev] RFR: 8247370: Clean up unused printing code in awt_PrintJob.cpp
This removes a long time un-used code path. - Commit messages: - 8247370: Clean up unused printing code in awt_PrintJob.cpp Changes: https://git.openjdk.java.net/jdk/pull/3083/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3083=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8247370 Stats: 29 lines in 1 file changed: 0 ins; 19 del; 10 mod Patch: https://git.openjdk.java.net/jdk/pull/3083.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3083/head:pull/3083 PR: https://git.openjdk.java.net/jdk/pull/3083
[OpenJDK 2D-Dev] Integrated: 8263833: Stop disabling warnings for sunFont.c with gcc
On Thu, 18 Mar 2021 20:54:27 GMT, Phil Race wrote: > As per the bug report the code that was the reason for disabling warnings on > this file was removed in November 2019 > as part of https://bugs.openjdk.java.net/browse/JDK-8220231 so we can get rid > of the need to disable warnings. > > Longer history in the bug report. This pull request has now been integrated. Changeset: ed1e25d5 Author:Phil Race URL: https://git.openjdk.java.net/jdk/commit/ed1e25d5 Stats: 6 lines in 1 file changed: 0 ins; 6 del; 0 mod 8263833: Stop disabling warnings for sunFont.c with gcc Reviewed-by: erikj - PR: https://git.openjdk.java.net/jdk/pull/3081
Re: [OpenJDK 2D-Dev] RFR: 8263833: Stop disabling warnings for sunFont.c with gcc
On Thu, 18 Mar 2021 20:54:27 GMT, Phil Race wrote: > As per the bug report the code that was the reason for disabling warnings on > this file was removed in November 2019 > as part of https://bugs.openjdk.java.net/browse/JDK-8220231 so we can get rid > of the need to disable warnings. > > Longer history in the bug report. Marked as reviewed by erikj (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3081
[OpenJDK 2D-Dev] RFR: 8263833: Stop disabling warnings for sunFont.c with gcc
As per the bug report the code that was the reason for disabling warnings on this file was removed in November 2019 as part of https://bugs.openjdk.java.net/browse/JDK-8220231 so we can get rid of the need to disable warnings. Longer history in the bug report. - Commit messages: - 8263833: Stop disabling warnings for sunFont.c with gcc Changes: https://git.openjdk.java.net/jdk/pull/3081/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3081=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8263833 Stats: 6 lines in 1 file changed: 0 ins; 6 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/3081.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3081/head:pull/3081 PR: https://git.openjdk.java.net/jdk/pull/3081
[OpenJDK 2D-Dev] Integrated: 8263622: The java.awt.color.ICC_Profile#setData invert the order of bytes for the "head" tag
On Tue, 16 Mar 2021 20:08:59 GMT, Sergey Bylokhov wrote: > The root cause is using the wrong endian, the ICC profile uses big-endian > notation. We even have special methods to convert the data, but for some > reason, their usage was dropped in the JDK-6523398. This pull request has now been integrated. Changeset: 01ddf3d2 Author:Sergey Bylokhov URL: https://git.openjdk.java.net/jdk/commit/01ddf3d2 Stats: 132 lines in 2 files changed: 88 ins; 35 del; 9 mod 8263622: The java.awt.color.ICC_Profile#setData invert the order of bytes for the "head" tag Reviewed-by: azvegint - PR: https://git.openjdk.java.net/jdk/pull/3037
[OpenJDK 2D-Dev] Integrated: 8263439: getSupportedAttributeValues() throws NPE for Finishings attribute
On Wed, 17 Mar 2021 19:25:50 GMT, Phil Race wrote: > This seems to be a code path that has has not been exercised. > We need to check for null values in the array. This pull request has now been integrated. Changeset: 2173fedd Author:Phil Race URL: https://git.openjdk.java.net/jdk/commit/2173fedd Stats: 67 lines in 2 files changed: 67 ins; 0 del; 0 mod 8263439: getSupportedAttributeValues() throws NPE for Finishings attribute Reviewed-by: psadhukhan, azvegint - PR: https://git.openjdk.java.net/jdk/pull/3055
Re: [OpenJDK 2D-Dev] RFR: 8263439: getSupportedAttributeValues() throws NPE for Finishings attribute [v2]
On Thu, 18 Mar 2021 17:30:06 GMT, Phil Race wrote:
>> This seems to be a code path that has has not been exercised.
>> We need to check for null values in the array.
>
> Phil Race has updated the pull request incrementally with one additional
> commit since the last revision:
>
> 8263439: getSupportedAttributeValues() throws NPE for Finishings attribute
Marked as reviewed by azvegint (Reviewer).
src/java.desktop/unix/classes/sun/print/IPPPrintService.java line 572:
> 570: (new ExtFinishing(100)).getAll();
> 571: for (int j=0; j 572: if (fAll[j] == null) {
Are we still insisting on copyright's year update?
-
PR: https://git.openjdk.java.net/jdk/pull/3055
Re: [OpenJDK 2D-Dev] RFR: 8263439: getSupportedAttributeValues() throws NPE for Finishings attribute [v2]
> This seems to be a code path that has has not been exercised. > We need to check for null values in the array. Phil Race has updated the pull request incrementally with one additional commit since the last revision: 8263439: getSupportedAttributeValues() throws NPE for Finishings attribute - Changes: - all: https://git.openjdk.java.net/jdk/pull/3055/files - new: https://git.openjdk.java.net/jdk/pull/3055/files/c7b30e53..f333515f Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3055=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3055=00-01 Stats: 64 lines in 1 file changed: 64 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/3055.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3055/head:pull/3055 PR: https://git.openjdk.java.net/jdk/pull/3055
[OpenJDK 2D-Dev] Integrated: 8263311: Watch registry changes for remote printers update instead of polling
On Wed, 10 Mar 2021 15:38:27 GMT, Alexey Ivanov wrote: > [JDK-8153732](https://bugs.openjdk.java.net/browse/JDK-8153732) implemented > polling for remote printers. > That bug description also mentions watching the registry for changes and > links to the article which describes the method yet it does so in terms of > WMI. Using WMI is not necessary to watch for the registry updates. > > It is possible to replace polling mechanism with registry change > notifications. If the registry at `HKCU\Printers\Connections` is updated, > refresh the list of print services. > > It works perfectly well in my own testing with sharing a Generic / Text Only > printer from another laptop. The notification comes as soon as the printer is > installed, it results in a new key created under `Connections`. If a remote > printer is removed, the notification is also triggered as the key > corresponding to that printer is removed from the registry. > > I updated the steps in the manual test: `RemotePrinterStatusRefresh.java`. This pull request has now been integrated. Changeset: a85dc557 Author:Alexey Ivanov URL: https://git.openjdk.java.net/jdk/commit/a85dc557 Stats: 207 lines in 3 files changed: 31 ins; 158 del; 18 mod 8263311: Watch registry changes for remote printers update instead of polling Reviewed-by: psadhukhan, serb - PR: https://git.openjdk.java.net/jdk/pull/2915
Re: [OpenJDK 2D-Dev] RFR: 8262470: Printed GlyphVector outline with low DPI has bad quality on Windows [v2]
> Printing text using GlyphVector outline has bad quality on printers with low > DPI on Windows. > The GDI system used for text printing on Windows accepts only integer path > coordinates. > Rounding GlyphVector outline coordinates leads to distorted printed text. > > The issue had been reported as JDK-8256264 but was reverted because of the > regression JDK-8259007 "This test printed a blank page". > > The fix JDK-8256264 scaled coordinates in wPrinterJob.moveTo()/lineTo() > methods up and scaled transforms in wPrinterJob.beginPath()/endPath() down. > > The regression was in the WPathGraphics.deviceDrawLine() method which uses > wPrinterJob.moveTo()/lineTo() methods without surrounding them with > wPrinterJob.beginPath()/endPath() so the line coordinates were only scaled up. > > I tried to put wPrinterJob.beginPath()/endPath() methods around > wPrinterJob.moveTo()/lineTo() in the method WPathGraphics.deviceDrawLine() > but the line was not drawn at all even without scaling coordinates up and > transform down (without JDK-8256264 fix). It looks like GDI treats this case > as an empty shape. > > The proposed fix applies path coordinates and transform scaling only in > WPathGraphics.convertToWPath() method. > The one more PathPrecisionScaleFactorShapeTest.java manual test is added > which checks that all methods that draw paths in WPathGraphics are used: line > in WPathGraphics.deviceDrawLine() and SEG_LINETO/SEG_QUADTO/SEG_CUBICTO in > WPathGraphics.convertToWPath() . > > The `java/awt/print` and `java/awt/PrintJob` automatic and manual tests were > run on Windows 10 Pro with the fix. > > There are two failed automated tests which fail without the fix as well: > java/awt/print/PrinterJob/GlyphPositions.java > java/awt/print/PrinterJob/PSQuestionMark.java > > The following manual tests have issues on my system: > - `java/awt/print/Dialog/PrintDlgPageable.java` > java.lang.IllegalAccessException: class > com.sun.javatest.regtest.agent.MainWrapper$MainThread cannot access a member > of class PrintDlgPageable with modifiers "public static" > > - `java/awt/print/PrinterJob/PrintAttributeUpdateTest.java` I select pages > radio button, press the print button but the test does not finish and I do > not see any other dialogs with pass/fail buttons. > > - `java/awt/PrintJob/PrintCheckboxTest/PrintCheckboxManualTest.java` Tests > that there is no ClassCastException thrown in printing checkbox and scrollbar > with XAWT. Error. Can't find HTML file: > test\jdk\java\awt\PrintJob\PrintCheckboxTest\PrintCheckboxManualTest.html > > > - `java/awt/print/PrinterJob/SecurityDialogTest.java` A windows with > instructions is shown but it does not contain print/pass/fail buttons and it > is not possible to close the window. > > - The tests below fail with "Error. Parse Exception: Arguments to `manual' > option not supported: yesno" message: > java/awt/print/Dialog/DialogOrient.java > java/awt/print/Dialog/DialogType.java > java/awt/print/PrinterJob/ImagePrinting/ClippedImages.java > java/awt/print/PrinterJob/ImagePrinting/ImageTypes.java > java/awt/print/PrinterJob/ImagePrinting/PrintARGBImage.java > java/awt/print/PrinterJob/PageDialogTest.java > java/awt/print/PrinterJob/PageRanges.java > java/awt/print/PrinterJob/PageRangesDlgTest.java > java/awt/print/PrinterJob/PrintGlyphVectorTest.java > java/awt/print/PrinterJob/PrintLatinCJKTest.java > java/awt/print/PrinterJob/PrintTextTest.java > java/awt/print/PrinterJob/SwingUIText.java > java/awt/PrintJob/ConstrainedPrintingTest/ConstrainedPrintingTest.java > java/awt/PrintJob/PageSetupDlgBlockingTest/PageSetupDlgBlockingTest.java > java/awt/PrintJob/SaveDialogTitleTest.java Alexander Scherbatiy has updated the pull request incrementally with two additional commits since the last revision: - Use DASSERT to check SetGraphicsMode and WorldTransform results - Change setGraphicsMode() type to void - Changes: - all: https://git.openjdk.java.net/jdk/pull/2756/files - new: https://git.openjdk.java.net/jdk/pull/2756/files/e0278acd..8105885f Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2756=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2756=00-01 Stats: 21 lines in 2 files changed: 8 ins; 2 del; 11 mod Patch: https://git.openjdk.java.net/jdk/pull/2756.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2756/head:pull/2756 PR: https://git.openjdk.java.net/jdk/pull/2756
Re: [OpenJDK 2D-Dev] RFR: 8262470: Printed GlyphVector outline with low DPI has bad quality on Windows [v2]
On Wed, 10 Mar 2021 09:31:32 GMT, Prasanta Sadhukhan
wrote:
>> Alexander Scherbatiy has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - Use DASSERT to check SetGraphicsMode and WorldTransform results
>> - Change setGraphicsMode() type to void
>
> src/java.desktop/windows/classes/sun/awt/windows/WPrinterJob.java line 1025:
>
>> 1023: * {@code GM_COMPATIBLE} or {@code GM_ADVANCED}.
>> 1024: */
>> 1025: private int setGraphicsMode(int mode) {
>
> Is there any need of "int" return value? I dont see it is used in
> restoreTransform()
I updated the code to return void from setGraphicsMode() method.
> src/java.desktop/windows/native/libawt/windows/awt_PrintJob.cpp line 2033:
>
>> 2031: xForm.eDy = (FLOAT) elems[5];
>> 2032:
>> 2033: ::SetWorldTransform((HDC)printDC, );
>
> Probably we should check for the return value of all this system APIs
> SetGraphicsMode, GetWorldTransform, SetWorldTransform, ModifyWorldTransform
> to see if it succeeded?
I added DASSERT to check SetGraphicsMode and Get/Set/ModifyWorldTransform
results.
-
PR: https://git.openjdk.java.net/jdk/pull/2756
Re: [OpenJDK 2D-Dev] RFR: 8263583: Emoji rendering on macOS
On Wed, 17 Mar 2021 10:45:00 GMT, Dmitry Batrak wrote: >>> What kind of test are you having in mind? I can extend the added >>> MacEmoji.java test, that will check that 'something' is painted to >>> different types of BufferedImage-s and/or with transforms applied. Or you >>> want me to add a manual test case? >> >> The automated tests will be enough, not sure about 'something is painted' >> however, it is better to check that it works. > > @mrserb I don't know how to check automatically that glyph painting works > correctly. Could you please suggest a way to do it? In JetBrains Runtime we > have a test that checks that rendered emoji glyph matches one of stored > 'golden images', but I don't think that's suitable for OpenJDK, unless > someone will volunteer to update that list when the need arises (e.g. when > newer version of macOS changes the font, or rendering details). At the > moment, we have already 7 golden images for a single glyph in our repository. You the current image from the MacEmoji test as a golden image for other formats/transforms/extraalpha/etc. For example, if DST type is changed then it is unlikely the shape of the emoji will be changed as well, or if it is too big/ too small. BTW It will be good if the test will fail on the current metal implementation. - PR: https://git.openjdk.java.net/jdk/pull/3007
