Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v13]
On Fri, 12 Mar 2021 01:01:09 GMT, Sergey Bylokhov wrote:
>> Ajit Ghaisas has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains 47 additional
>> commits since the last revision:
>>
>> - Lanai PR#214 - 8263324 - avu
>> - Merge branch 'master' into 8260931_lanai_JEP_branch
>> - Lanai PR#213 - 8263325 - avu
>> - Lanai PR#212 - 8259825 - aghaisas
>> - Lanai PR#211 - 8262882 - aghaisas
>> - Merge branch 'master' into 8260931_lanai_JEP_branch
>> - Lanai PR#210 - 8263159 - jdv
>> - Lanai PR#209 - 8262936 - jdv
>> - Lanai PR#208 - 8262928 - jdv
>> - Lanai PR#207 - 8262750 - jdv
>> - ... and 37 more:
>> https://git.openjdk.java.net/jdk/compare/774f3477...369c3d21
>
> src/java.desktop/macosx/classes/sun/java2d/metal/MTLGraphicsConfig.java line
> 148:
>
>> 146: rq.lock();
>> 147: try {
>> 148: // getMTLConfigInfo() creates and destroys temporary
>
> Does it really create and destroy temporary surfaces/contexts?
No, it doesn't. I have updated JDK-8263363 to include this correction.
-
PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v13]
> On 12-Mar-2021, at 10:30 AM, Sergey Bylokhov wrote: > > On Fri, 12 Mar 2021 00:09:54 GMT, Kevin Rushforth wrote: > >>> Ajit Ghaisas has updated the pull request with a new target base due to a >>> merge or a rebase. The incremental webrev excludes the unrelated changes >>> brought in by the merge/rebase. The pull request contains 47 additional >>> commits since the last revision: >>> >>> - Lanai PR#214 - 8263324 - avu >>> - Merge branch 'master' into 8260931_lanai_JEP_branch >>> - Lanai PR#213 - 8263325 - avu >>> - Lanai PR#212 - 8259825 - aghaisas >>> - Lanai PR#211 - 8262882 - aghaisas >>> - Merge branch 'master' into 8260931_lanai_JEP_branch >>> - Lanai PR#210 - 8263159 - jdv >>> - Lanai PR#209 - 8262936 - jdv >>> - Lanai PR#208 - 8262928 - jdv >>> - Lanai PR#207 - 8262750 - jdv >>> - ... and 37 more: >>> https://git.openjdk.java.net/jdk/compare/9d82be31...369c3d21 >> >> Latest changes look good. I confirmed that this PR has all of the content >> from the lanai repo. > >> By default we don?t do LCD anti-aliasing for text, only when we set Text >> Rendering Hint to use LCD antialiasing we take sub-pixel rendering path. > > And it actually produces LCD glyphs? not the grayscale? We get glyphs which take more than a byte to represent each pixel. Based on which we take LCD sub-pixel rendering path. > > - > > PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v13]
On Fri, 12 Mar 2021 00:09:54 GMT, Kevin Rushforth wrote: >> Ajit Ghaisas has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains 47 additional >> commits since the last revision: >> >> - Lanai PR#214 - 8263324 - avu >> - Merge branch 'master' into 8260931_lanai_JEP_branch >> - Lanai PR#213 - 8263325 - avu >> - Lanai PR#212 - 8259825 - aghaisas >> - Lanai PR#211 - 8262882 - aghaisas >> - Merge branch 'master' into 8260931_lanai_JEP_branch >> - Lanai PR#210 - 8263159 - jdv >> - Lanai PR#209 - 8262936 - jdv >> - Lanai PR#208 - 8262928 - jdv >> - Lanai PR#207 - 8262750 - jdv >> - ... and 37 more: >> https://git.openjdk.java.net/jdk/compare/9d82be31...369c3d21 > > Latest changes look good. I confirmed that this PR has all of the content > from the lanai repo. > By default we don?t do LCD anti-aliasing for text, only when we set Text > Rendering Hint to use LCD antialiasing we take sub-pixel rendering path. And it actually produces LCD glyphs? not the grayscale? - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8263311: Watch registry changes for remote printers update instead of polling
On Thu, 11 Mar 2021 14:49:59 GMT, Alexey Ivanov wrote: >> I also am not sure on this. But I think since this is for remote printer, >> sometimes network availability issue might be there so it may fail more >> compared to local printer so I guess we should give this method a fair >> chance, more than that of local printer change, and not bail out on one >> failure.maybe try out after some duration...or 5 times spaced out...as >> you did for the other EnumPrinter fix.. > > No, network connectivity cannot affect this. The function watches for > registry changes, specifically keys created or removed under > `HKCU\Printers\Connections`. If network is down, you won't be able to add a > new printer. Yet you can still remove an existing printer. > > The case with `EnumPrinters` is very different. A printer may be renamed or a > new printer may be added therefore the allocated buffer becomes to small to > fit the updated data. Thus retrying with larger buffer makes perfect sense. > > In this case, `RegNotifyChangeKeyValue` could fail only because of invalid > parameters. If it does, it will fail on the retry because the parameters > haven't changed. > > In that sense, it's the same as with local printers. If the wait/notification > function fails the first time, it will likely fail the second time and so on… Ok - PR: https://git.openjdk.java.net/jdk/pull/2915
Re: RFR: 8263311: Watch registry changes for remote printers update instead of polling
On Thu, 11 Mar 2021 14:59:55 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`. > > src/java.desktop/windows/native/libawt/windows/WPrinterJob.cpp line 258: > >> 256: >> REG_NOTIFY_CHANGE_NAME, >> 257: NULL, >> 258: FALSE); > > [`RegNotifyChangeKeyValue`](https://docs.microsoft.com/en-us/windows/win32/api/winreg/nf-winreg-regnotifychangekeyvalue) > notifies the caller about changes to the attributes or contents of a > specified registry key. > > • `hKey`: A handle to `HKEY_CURRENT_USER\Printers\Connections` key which is > opened above. > • `bWatchSubtree = TRUE`: The function reports changes in the specified key > and its subkeys. > • `dwNotifyFilter = REG_NOTIFY_CHANGE_NAME`: Notify the caller if a subkey is > added or deleted. > • `hEvent = NULL`: If `fAsynchronous` is FALSE, `hEvent` is ignored. > • `fAsynchronous = FALSE`: The function does not return until a change has > occurred. > > When a new remote printer is added, a new key is created under > `HKCU\Printers\Connections`; when an existing remote printer is removed, the > key below `Connections` is removed; no values are added or removed in > `Connections` key, thus `REG_NOTIFY_CHANGE_LAST_SET` filter is not needed. Is this only about addition/removal? What about printer name change? Shouldn't we get notified in that case as trying to print on printer with old name will not find the printer!! If yes, in that regard I guess REG_NOTIFY_CHANGE_LAST_SET is the one to go for as it states `"Notify the caller of changes to a value of the key. This can include adding or deleting a value, or changing an existing value. "` - PR: https://git.openjdk.java.net/jdk/pull/2915
Re: RFR: 8194129: Regression automated Test '/open/test/jdk/java/awt/Window/ShapedAndTranslucentWindows/TranslucentChoice.java' fails
On Fri, 12 Mar 2021 00:25:39 GMT, Alexander Zvegintsev wrote: > First steps of the test are: > > - display undecorated background window > - click on it > - check if the window has received that click > - ... continue with test ... > > Unfortunately on Linux the test makes click before the window is shown. > > Testing is green in over 100 runs. Please close JDK-8221901 as dup of this. - Marked as reviewed by psadhukhan (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2953
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v13]
> On 12-Mar-2021, at 9:29 AM, Scott Palmer wrote:
>
>
>> On Mar 11, 2021, at 9:53 PM, Sergey Bylokhov wrote:
>>
>> On Fri, 12 Mar 2021 02:29:04 GMT, Jayathirth D V wrote:
>>
src/java.desktop/macosx/classes/sun/java2d/metal/MTLSurfaceData.java line
323:
> 321: * more code just to support a few uncommon cases.
> 322: */
> 323: public boolean canRenderLCDText(SunGraphics2D sg2d) {
Just curious, can we render LCD on 10.14+ via metal? Does it work fine?
>>>
>>> Yes Sergey it works fine in 10.14+ systems via metal. Most of the JCK
>>> manual tests use LCD text on UI Components and it is recently verified in
>>> 10.14+ systems for EA10.
>>
>> Ok, for some reason I thought that the new macOS stopped providing LCD
>> glyphs.
>>
>
> Are the glyphs different or just the way they are rasterized?
>
> Newer versions of macOS don’t do LCD text and have gone back to plain
> grey-scale anti-aliasing.
> Java 2D should default to NOT doing LCD anti-aliasing for text on macOS if it
> wants to fit in with the look of native applications.
> (I’m not sure if that applies to non-retina displays.)
>
By default we don’t do LCD anti-aliasing for text, only when we set Text
Rendering Hint to use LCD antialiasing we take sub-pixel rendering path.
> Scott
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v13]
On Fri, 12 Mar 2021 01:04:22 GMT, Sergey Bylokhov wrote: >> Ajit Ghaisas has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains 47 additional >> commits since the last revision: >> >> - Lanai PR#214 - 8263324 - avu >> - Merge branch 'master' into 8260931_lanai_JEP_branch >> - Lanai PR#213 - 8263325 - avu >> - Lanai PR#212 - 8259825 - aghaisas >> - Lanai PR#211 - 8262882 - aghaisas >> - Merge branch 'master' into 8260931_lanai_JEP_branch >> - Lanai PR#210 - 8263159 - jdv >> - Lanai PR#209 - 8262936 - jdv >> - Lanai PR#208 - 8262928 - jdv >> - Lanai PR#207 - 8262750 - jdv >> - ... and 37 more: >> https://git.openjdk.java.net/jdk/compare/23f9642d...369c3d21 > > src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLPaints.m line 813: > >> 811: initTemplatePipelineDescriptors(); >> 812: // This block is not reached in current implementation. >> 813: // Texture paint XOR mode rendering uses a tile based rendering >> using a SW pipe (similar to OGL) > > Do we have a bugid to implement this later? This is not exactly a TODO which we intend to implement later. Hence, there is no bugid. Metal does not support XOR mode rendering. I have kept this code as to know the code path to take in case we need to implement it in future. > src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLSurfaceDataBase.h > line 59: > >> 57: * The offset in pixels of the Metal viewport origin from the lower-left >> 58: * corner of the heavyweight drawable. For example, a top-level frame on >> 59: * Windows XP has lower-left insets of (4,4). The Metal viewport origin > > Do we really use these fields? "Windows XP"?? This will be fixed as a follow-on issue - JDK-8263486. - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v13]
On Fri, 12 Mar 2021 00:48:58 GMT, Sergey Bylokhov wrote:
>> Ajit Ghaisas has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains 47 additional
>> commits since the last revision:
>>
>> - Lanai PR#214 - 8263324 - avu
>> - Merge branch 'master' into 8260931_lanai_JEP_branch
>> - Lanai PR#213 - 8263325 - avu
>> - Lanai PR#212 - 8259825 - aghaisas
>> - Lanai PR#211 - 8262882 - aghaisas
>> - Merge branch 'master' into 8260931_lanai_JEP_branch
>> - Lanai PR#210 - 8263159 - jdv
>> - Lanai PR#209 - 8262936 - jdv
>> - Lanai PR#208 - 8262928 - jdv
>> - Lanai PR#207 - 8262750 - jdv
>> - ... and 37 more:
>> https://git.openjdk.java.net/jdk/compare/aca5b193...369c3d21
>
> src/java.desktop/macosx/native/libawt_lwawt/awt/AWTSurfaceLayers.m line 73:
>
>> 71: // Updates back buffer size of the layer if it's an OpenGL/Metal layer
>> 72: // including all OpenGL/Metal sublayers
>> 73: + (void) repaintLayersRecursively:(CALayer*)aLayer {
>
> The operation of this code is still being investigated. @prsadhuk please add
> a bugid here.
JDK-8263485
-
PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v13]
> On Mar 11, 2021, at 9:53 PM, Sergey Bylokhov wrote:
>
> On Fri, 12 Mar 2021 02:29:04 GMT, Jayathirth D V wrote:
>
>>> src/java.desktop/macosx/classes/sun/java2d/metal/MTLSurfaceData.java line
>>> 323:
>>>
321: * more code just to support a few uncommon cases.
322: */
323: public boolean canRenderLCDText(SunGraphics2D sg2d) {
>>>
>>> Just curious, can we render LCD on 10.14+ via metal? Does it work fine?
>>
>> Yes Sergey it works fine in 10.14+ systems via metal. Most of the JCK manual
>> tests use LCD text on UI Components and it is recently verified in 10.14+
>> systems for EA10.
>
> Ok, for some reason I thought that the new macOS stopped providing LCD glyphs.
>
Are the glyphs different or just the way they are rasterized?
Newer versions of macOS don’t do LCD text and have gone back to plain
grey-scale anti-aliasing.
Java 2D should default to NOT doing LCD anti-aliasing for text on macOS if it
wants to fit in with the look of native applications.
(I’m not sure if that applies to non-retina displays.)
Scott
Re: RFR: 8262981: Create implementation for NSAccessibilitySlider protocol
On Thu, 11 Mar 2021 17:10:01 GMT, Pankaj Bansal wrote: > If you happen to have some idea about how can a user interact with Sliders or > Spinners using JAWS, please let me know. I will definitely verify if the > changes would be helpful for Windows. How the native applications works under JAWS? - PR: https://git.openjdk.java.net/jdk/pull/2874
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v13]
On Fri, 12 Mar 2021 00:42:35 GMT, Sergey Bylokhov wrote:
>> Ajit Ghaisas has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains 47 additional
>> commits since the last revision:
>>
>> - Lanai PR#214 - 8263324 - avu
>> - Merge branch 'master' into 8260931_lanai_JEP_branch
>> - Lanai PR#213 - 8263325 - avu
>> - Lanai PR#212 - 8259825 - aghaisas
>> - Lanai PR#211 - 8262882 - aghaisas
>> - Merge branch 'master' into 8260931_lanai_JEP_branch
>> - Lanai PR#210 - 8263159 - jdv
>> - Lanai PR#209 - 8262936 - jdv
>> - Lanai PR#208 - 8262928 - jdv
>> - Lanai PR#207 - 8262750 - jdv
>> - ... and 37 more:
>> https://git.openjdk.java.net/jdk/compare/53b0bea8...369c3d21
>
> src/java.desktop/macosx/classes/sun/java2d/metal/MTLSurfaceData.java line 323:
>
>> 321: * more code just to support a few uncommon cases.
>> 322: */
>> 323: public boolean canRenderLCDText(SunGraphics2D sg2d) {
>
> Just curious, can we render LCD on 10.14+ via metal? Does it work fine?
Yes Sergey it works fine in 10.14+ systems via metal. Most of the JCK manual
tests use LCD text on UI Components and it is recently verified in 10.14+
systems for EA10.
-
PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v13]
On Thu, 11 Mar 2021 18:00:15 GMT, Ajit Ghaisas wrote:
>> **Description :**
>> This is the implementation of [JEP 382 : New macOS Rendering
>> Pipeline](https://bugs.openjdk.java.net/browse/JDK-8238361)
>> It implements a Java 2D internal rendering pipeline for macOS using the
>> Apple Metal API.
>> The entire work on this was done under [OpenJDK Project -
>> Lanai](http://openjdk.java.net/projects/lanai/)
>>
>> We iterated through several Early Access (EA) builds and have reached a
>> stage where it is ready to be integrated to openjdk/jdk. The latest EA build
>> is available at - https://jdk.java.net/lanai/
>>
>> A new option -Dsun.java2d.metal=true | True needs to be used to use this
>> pipeline.
>>
>> **Testing :**
>> This implementation has been tested with the tests present at - [Test Plan
>> for JEP 382: New macOS Rendering
>> Pipeline](https://bugs.openjdk.java.net/browse/JDK-8251396)
>>
>> **Note to reviewers :**
>> 1) Default rendering pipeline on macOS has not been changed by this PR.
>> OpenGL still stays as the default rendering pipeline and Metal rendering
>> pipeline is optional to choose.
>>
>> 2) To apply and test this PR -
>> To enable the metal pipeline you must specify on command line
>> -Dsun.java2d.metal=true (No message will be printed in this case) or
>> -Dsun.java2d.metal=True (A message indicating Metal rendering pipeline is
>> enabled gets printed)
>>
>> 3) Review comments (including some preliminary informal review comments) are
>> tracked with JBS issues - https://bugs.openjdk.java.net/issues/?filter=40598
>
> Ajit Ghaisas has updated the pull request with a new target base due to a
> merge or a rebase. The incremental webrev excludes the unrelated changes
> brought in by the merge/rebase. The pull request contains 47 additional
> commits since the last revision:
>
> - Lanai PR#214 - 8263324 - avu
> - Merge branch 'master' into 8260931_lanai_JEP_branch
> - Lanai PR#213 - 8263325 - avu
> - Lanai PR#212 - 8259825 - aghaisas
> - Lanai PR#211 - 8262882 - aghaisas
> - Merge branch 'master' into 8260931_lanai_JEP_branch
> - Lanai PR#210 - 8263159 - jdv
> - Lanai PR#209 - 8262936 - jdv
> - Lanai PR#208 - 8262928 - jdv
> - Lanai PR#207 - 8262750 - jdv
> - ... and 37 more:
> https://git.openjdk.java.net/jdk/compare/5e61bf57...369c3d21
src/java.desktop/macosx/classes/sun/java2d/metal/MTLSurfaceData.java line 323:
> 321: * more code just to support a few uncommon cases.
> 322: */
> 323: public boolean canRenderLCDText(SunGraphics2D sg2d) {
Just curious, can we render LCD on 10.14+ via metal? Does it work fine?
src/java.desktop/macosx/native/libawt_lwawt/awt/AWTSurfaceLayers.m line 73:
> 71: // Updates back buffer size of the layer if it's an OpenGL/Metal layer
> 72: // including all OpenGL/Metal sublayers
> 73: + (void) repaintLayersRecursively:(CALayer*)aLayer {
The operation of this code is still being investigated. @prsadhuk please add a
bugid here.
src/java.desktop/macosx/classes/sun/java2d/metal/MTLGraphicsConfig.java line
148:
> 146: rq.lock();
> 147: try {
> 148: // getMTLConfigInfo() creates and destroys temporary
Does it really create and destroy temporary surfaces/contexts?
src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLPaints.m line 813:
> 811: initTemplatePipelineDescriptors();
> 812: // This block is not reached in current implementation.
> 813: // Texture paint XOR mode rendering uses a tile based rendering
> using a SW pipe (similar to OGL)
Do we have a bugid to implement this later?
src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLSurfaceDataBase.h
line 59:
> 57: * The offset in pixels of the Metal viewport origin from the lower-left
> 58: * corner of the heavyweight drawable. For example, a top-level frame on
> 59: * Windows XP has lower-left insets of (4,4). The Metal viewport origin
Do we really use these fields? "Windows XP"??
-
PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v10]
On Tue, 9 Mar 2021 20:16:25 GMT, Alexey Ushakov wrote: >> src/java.desktop/macosx/classes/sun/java2d/metal/MTLBlitLoops.java line 499: >> >>> 497: } >>> 498: >>> 499: // We can convert argb_pre data from MTL surface in two places: >> >> Does anybody check that this is true for the metal pipeline? or It is just >> copied from the OGL? > > I fixed some conversion logic within JDK-8256331. OK, just to confirm. I wrote that text for OGL because it was the fastest way to transfer+convert the data from the video card to the memory. And as far as I understand the metal has the same limitation? It is not possible to convert it to some non-ARGB/Pre format on the fly while transferring the pixles? >> src/java.desktop/macosx/classes/sun/java2d/metal/MTLSurfaceData.java line >> 146: >> >>> 144: return MTLSurfaceRTT; >>> 145: default: >>> 146: return MTLSurface; >> >> Do we really support 3 different types of surface? I guess only two of them >> are different: textures used for manageable images, and surfaces used by the >> volatile image. > > It's true but I don't see any problem returning a more generic type as a > default here tbd - PR: https://git.openjdk.java.net/jdk/pull/2403
RFR: 8194129: Regression automated Test '/open/test/jdk/java/awt/Window/ShapedAndTranslucentWindows/TranslucentChoice.java' fails
First steps of the test are: - display undecorated background window - click on it - check if the window has received that click - ... continue with test ... Unfortunately on Linux the test makes click before the window is shown. - Commit messages: - initial Changes: https://git.openjdk.java.net/jdk/pull/2953/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2953=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8194129 Stats: 5 lines in 2 files changed: 3 ins; 1 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/2953.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2953/head:pull/2953 PR: https://git.openjdk.java.net/jdk/pull/2953
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v13]
On Thu, 11 Mar 2021 18:00:15 GMT, Ajit Ghaisas wrote: >> **Description :** >> This is the implementation of [JEP 382 : New macOS Rendering >> Pipeline](https://bugs.openjdk.java.net/browse/JDK-8238361) >> It implements a Java 2D internal rendering pipeline for macOS using the >> Apple Metal API. >> The entire work on this was done under [OpenJDK Project - >> Lanai](http://openjdk.java.net/projects/lanai/) >> >> We iterated through several Early Access (EA) builds and have reached a >> stage where it is ready to be integrated to openjdk/jdk. The latest EA build >> is available at - https://jdk.java.net/lanai/ >> >> A new option -Dsun.java2d.metal=true | True needs to be used to use this >> pipeline. >> >> **Testing :** >> This implementation has been tested with the tests present at - [Test Plan >> for JEP 382: New macOS Rendering >> Pipeline](https://bugs.openjdk.java.net/browse/JDK-8251396) >> >> **Note to reviewers :** >> 1) Default rendering pipeline on macOS has not been changed by this PR. >> OpenGL still stays as the default rendering pipeline and Metal rendering >> pipeline is optional to choose. >> >> 2) To apply and test this PR - >> To enable the metal pipeline you must specify on command line >> -Dsun.java2d.metal=true (No message will be printed in this case) or >> -Dsun.java2d.metal=True (A message indicating Metal rendering pipeline is >> enabled gets printed) >> >> 3) Review comments (including some preliminary informal review comments) are >> tracked with JBS issues - https://bugs.openjdk.java.net/issues/?filter=40598 > > Ajit Ghaisas has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains 47 additional > commits since the last revision: > > - Lanai PR#214 - 8263324 - avu > - Merge branch 'master' into 8260931_lanai_JEP_branch > - Lanai PR#213 - 8263325 - avu > - Lanai PR#212 - 8259825 - aghaisas > - Lanai PR#211 - 8262882 - aghaisas > - Merge branch 'master' into 8260931_lanai_JEP_branch > - Lanai PR#210 - 8263159 - jdv > - Lanai PR#209 - 8262936 - jdv > - Lanai PR#208 - 8262928 - jdv > - Lanai PR#207 - 8262750 - jdv > - ... and 37 more: > https://git.openjdk.java.net/jdk/compare/204b2bcc...369c3d21 Latest changes look good. I confirmed that this PR has all of the content from the lanai repo. - Marked as reviewed by kcr (Author). PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v13]
On Thu, 11 Mar 2021 18:00:15 GMT, Ajit Ghaisas wrote: >> **Description :** >> This is the implementation of [JEP 382 : New macOS Rendering >> Pipeline](https://bugs.openjdk.java.net/browse/JDK-8238361) >> It implements a Java 2D internal rendering pipeline for macOS using the >> Apple Metal API. >> The entire work on this was done under [OpenJDK Project - >> Lanai](http://openjdk.java.net/projects/lanai/) >> >> We iterated through several Early Access (EA) builds and have reached a >> stage where it is ready to be integrated to openjdk/jdk. The latest EA build >> is available at - https://jdk.java.net/lanai/ >> >> A new option -Dsun.java2d.metal=true | True needs to be used to use this >> pipeline. >> >> **Testing :** >> This implementation has been tested with the tests present at - [Test Plan >> for JEP 382: New macOS Rendering >> Pipeline](https://bugs.openjdk.java.net/browse/JDK-8251396) >> >> **Note to reviewers :** >> 1) Default rendering pipeline on macOS has not been changed by this PR. >> OpenGL still stays as the default rendering pipeline and Metal rendering >> pipeline is optional to choose. >> >> 2) To apply and test this PR - >> To enable the metal pipeline you must specify on command line >> -Dsun.java2d.metal=true (No message will be printed in this case) or >> -Dsun.java2d.metal=True (A message indicating Metal rendering pipeline is >> enabled gets printed) >> >> 3) Review comments (including some preliminary informal review comments) are >> tracked with JBS issues - https://bugs.openjdk.java.net/issues/?filter=40598 > > Ajit Ghaisas has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains 47 additional > commits since the last revision: > > - Lanai PR#214 - 8263324 - avu > - Merge branch 'master' into 8260931_lanai_JEP_branch > - Lanai PR#213 - 8263325 - avu > - Lanai PR#212 - 8259825 - aghaisas > - Lanai PR#211 - 8262882 - aghaisas > - Merge branch 'master' into 8260931_lanai_JEP_branch > - Lanai PR#210 - 8263159 - jdv > - Lanai PR#209 - 8262936 - jdv > - Lanai PR#208 - 8262928 - jdv > - Lanai PR#207 - 8262750 - jdv > - ... and 37 more: > https://git.openjdk.java.net/jdk/compare/ada1a022...369c3d21 Marked as reviewed by prr (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v12]
On Thu, 11 Mar 2021 07:40:47 GMT, Alexey Ushakov wrote: >> Ajit Ghaisas has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains 45 additional >> commits since the last revision: >> >> - Lanai PR#213 - 8263325 - avu >> - Lanai PR#212 - 8259825 - aghaisas >> - Lanai PR#211 - 8262882 - aghaisas >> - Merge branch 'master' into 8260931_lanai_JEP_branch >> - Lanai PR#210 - 8263159 - jdv >> - Lanai PR#209 - 8262936 - jdv >> - Lanai PR#208 - 8262928 - jdv >> - Lanai PR#207 - 8262750 - jdv >> - Merge branch 'master' into 8260931_lanai_JEP_branch >> - Lanai PR#206 - 8262729 - aghaisas >> - ... and 35 more: >> https://git.openjdk.java.net/jdk/compare/d1e5ff8f...3980ecbd > > Marked as reviewed by avu (no project role). We have addressed the review comments (except for a few minor follow-on issues) This version of the PR is the one which we are planning to integrate into the JDK. Please review and approve. - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v13]
> **Description :** > This is the implementation of [JEP 382 : New macOS Rendering > Pipeline](https://bugs.openjdk.java.net/browse/JDK-8238361) > It implements a Java 2D internal rendering pipeline for macOS using the Apple > Metal API. > The entire work on this was done under [OpenJDK Project - > Lanai](http://openjdk.java.net/projects/lanai/) > > We iterated through several Early Access (EA) builds and have reached a stage > where it is ready to be integrated to openjdk/jdk. The latest EA build is > available at - https://jdk.java.net/lanai/ > > A new option -Dsun.java2d.metal=true | True needs to be used to use this > pipeline. > > **Testing :** > This implementation has been tested with the tests present at - [Test Plan > for JEP 382: New macOS Rendering > Pipeline](https://bugs.openjdk.java.net/browse/JDK-8251396) > > **Note to reviewers :** > 1) Default rendering pipeline on macOS has not been changed by this PR. > OpenGL still stays as the default rendering pipeline and Metal rendering > pipeline is optional to choose. > > 2) To apply and test this PR - > To enable the metal pipeline you must specify on command line > -Dsun.java2d.metal=true (No message will be printed in this case) or > -Dsun.java2d.metal=True (A message indicating Metal rendering pipeline is > enabled gets printed) > > 3) Review comments (including some preliminary informal review comments) are > tracked with JBS issues - https://bugs.openjdk.java.net/issues/?filter=40598 Ajit Ghaisas has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 47 additional commits since the last revision: - Lanai PR#214 - 8263324 - avu - Merge branch 'master' into 8260931_lanai_JEP_branch - Lanai PR#213 - 8263325 - avu - Lanai PR#212 - 8259825 - aghaisas - Lanai PR#211 - 8262882 - aghaisas - Merge branch 'master' into 8260931_lanai_JEP_branch - Lanai PR#210 - 8263159 - jdv - Lanai PR#209 - 8262936 - jdv - Lanai PR#208 - 8262928 - jdv - Lanai PR#207 - 8262750 - jdv - ... and 37 more: https://git.openjdk.java.net/jdk/compare/1c8fee73...369c3d21 - Changes: - all: https://git.openjdk.java.net/jdk/pull/2403/files - new: https://git.openjdk.java.net/jdk/pull/2403/files/3980ecbd..369c3d21 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2403=12 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2403=11-12 Stats: 2420 lines in 128 files changed: 1246 ins; 368 del; 806 mod Patch: https://git.openjdk.java.net/jdk/pull/2403.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2403/head:pull/2403 PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v12]
On Mon, 8 Feb 2021 18:47:09 GMT, Sergey Bylokhov wrote:
>> src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLGraphicsConfig.m
>> line 82:
>>
>>> 80: (JNIEnv *env, jclass mtlgc)
>>> 81: {
>>> 82: FILE *f = popen("/usr/sbin/system_profiler SPDisplaysDataType",
>>> "r");
>>
>> How robust is this? It seems like the contents of this could be an
>> implementation detail and subject to change. Is it documented by Apple?
>
> I suggest fixing this before the integration.
Fixed with JDK-8259825.
-
PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8262981: Create implementation for NSAccessibilitySlider protocol
On Wed, 10 Mar 2021 23:09:31 GMT, Sergey Bylokhov wrote: > If it does not work on Windows then I suggest checking that the new > functionality requested via CSR will help it as well. I could not find any key shortcuts where JAWS will consume the key pressed and will be used to interact with the component like the VO is doing here. This similar functionality is present in JSpinner already and JAWS says similar thing when a Spinner is in focus "To set the value, press the arrow keys or type the value". When user presses the keys, it is consumed by Java and value is incremented/decremented as expected. But the value is not being set by JAWS like VO if used with VO keys. So I am not sure how to verify if the changes will be useful for windows as well. I think if there is a way user can interact with Spinner or Slider, then these changes should be useful and if it is not possible, it will not be useful in both. These changes does make the Slider more consistent with Spinner and easy to maintain and cleaner. If you happen to have some idea about how can a user interact with Sliders or Spinners using JAWS, please let me know. I will definitely verify if the changes would be helpful for Windows. - PR: https://git.openjdk.java.net/jdk/pull/2874
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v10]
On Tue, 9 Mar 2021 23:15:57 GMT, Sergey Bylokhov wrote: >> Also, MTLSD_WINDOW is supported > > How it is used? Can we really draw to the window directly? It looks like it > copied from the OGL where it is unused since CLayer integration. Something is > inconsistent here. Here is a follow-up issue JDK-8263463 - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 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`. src/java.desktop/windows/native/libawt/windows/WPrinterJob.cpp line 258: > 256: > REG_NOTIFY_CHANGE_NAME, > 257: NULL, > 258: FALSE); [`RegNotifyChangeKeyValue`](https://docs.microsoft.com/en-us/windows/win32/api/winreg/nf-winreg-regnotifychangekeyvalue) notifies the caller about changes to the attributes or contents of a specified registry key. • `hKey`: A handle to `HKEY_CURRENT_USER\Printers\Connections` key which is opened above. • `bWatchSubtree = TRUE`: The function reports changes in the specified key and its subkeys. • `dwNotifyFilter = REG_NOTIFY_CHANGE_NAME`: Notify the caller if a subkey is added or deleted. • `hEvent = NULL`: If `fAsynchronous` is FALSE, `hEvent` is ignored. • `fAsynchronous = FALSE`: The function does not return until a change has occurred. When a new remote printer is added, a new key is created under `HKCU\Printers\Connections`; when an existing remote printer is removed, the key below `Connections` is removed; no values are added or removed in `Connections` key, thus `REG_NOTIFY_CHANGE_LAST_SET` filter is not needed. - PR: https://git.openjdk.java.net/jdk/pull/2915
Re: RFR: 8263311: Watch registry changes for remote printers update instead of polling
On Thu, 11 Mar 2021 11:30:52 GMT, Prasanta Sadhukhan wrote: >>> I guess having "FALSE" as fAsynchronous value mean the function does not >>> return until a change has occurred so do we still need this do-while >>> monitoring loop? >> >> You're right, `FALSE` for `fAsynchronous` means the function doesn't return >> until a change occurred. >> >> If a change occurs, we refresh the list of print services and then start to >> wait again. If we exit the loop, we'll not catch other changes that may >> occur. >> >>> And if the function fails once, should we stop monitoring? >> I followed Sergey's approach in `notifyLocalPrinterChange`, namely if >> `FindNextPrinterChangeNotification` returns an error, we quit the loop. >> >> I can't see how we can fix the error if it occurs. Will it succeed the next >> time? Probably not. Thus I decided to quit the loop in case of an error. > > I also am not sure on this. But I think since this is for remote printer, > sometimes network availability issue might be there so it may fail more > compared to local printer so I guess we should give this method a fair > chance, more than that of local printer change, and not bail out on one > failure.maybe try out after some duration...or 5 times spaced out...as > you did for the other EnumPrinter fix.. No, network connectivity cannot affect this. The function watches for registry changes, specifically keys created or removed under `HKCU\Printers\Connections`. If network is down, you won't be able to add a new printer. Yet you can still remove an existing printer. The case with `EnumPrinters` is very different. A printer may be renamed or a new printer may be added therefore the allocated buffer becomes to small to fit the updated data. Thus retrying with larger buffer makes perfect sense. In this case, `RegNotifyChangeKeyValue` could fail only because of invalid parameters. If it does, it will fail on the retry because the parameters haven't changed. In that sense, it's the same as with local printers. If the wait/notification function fails the first time, it will likely fail the second time and so on… - PR: https://git.openjdk.java.net/jdk/pull/2915
Re: RFR: 8263311: Watch registry changes for remote printers update instead of polling
On Thu, 11 Mar 2021 10:54:12 GMT, Alexey Ivanov wrote:
>> src/java.desktop/windows/native/libawt/windows/WPrinterJob.cpp line 259:
>>
>>> 257: NULL,
>>> 258: FALSE);
>>> 259: if (keepMonitoring) {
>>
>> I guess having "FALSE" as fAsynchronous value mean the function does not
>> return until a change has occurred so do we still need this do-while
>> monitoring loop? And if the function fails once, should we stop monitoring?
>
>> I guess having "FALSE" as fAsynchronous value mean the function does not
>> return until a change has occurred so do we still need this do-while
>> monitoring loop?
>
> You're right, `FALSE` for `fAsynchronous` means the function doesn't return
> until a change occurred.
>
> If a change occurs, we refresh the list of print services and then start to
> wait again. If we exit the loop, we'll not catch other changes that may occur.
>
>> And if the function fails once, should we stop monitoring?
> I followed Sergey's approach in `notifyLocalPrinterChange`, namely if
> `FindNextPrinterChangeNotification` returns an error, we quit the loop.
>
> I can't see how we can fix the error if it occurs. Will it succeed the next
> time? Probably not. Thus I decided to quit the loop in case of an error.
I also am not sure on this. But I think since this is for remote printer,
sometimes network availability issue might be there so it may fail more
compared to local printer so I guess we should give this method a fair chance,
more than that of local printer change, and not bail out on one
failure.maybe try out after some duration...or 5 times spaced out...as you
did for the other EnumPrinter fix..
-
PR: https://git.openjdk.java.net/jdk/pull/2915
Re: RFR: 8263311: Watch registry changes for remote printers update instead of polling
On Thu, 11 Mar 2021 10:39:56 GMT, Prasanta Sadhukhan wrote: > I guess having "FALSE" as fAsynchronous value mean the function does not > return until a change has occurred so do we still need this do-while > monitoring loop? You're right, `FALSE` for `fAsynchronous` means the function doesn't return until a change occurred. If a change occurs, we refresh the list of print services and then start to wait again. If we exit the loop, we'll not catch other changes that may occur. > And if the function fails once, should we stop monitoring? I followed Sergey's approach in `notifyLocalPrinterChange`, namely if `FindNextPrinterChangeNotification` returns an error, we quit the loop. I can't see how we can fix the error if it occurs. Will it succeed the next time? Probably not. Thus I decided to quit the loop in case of an error. - PR: https://git.openjdk.java.net/jdk/pull/2915
