Re: [OpenJDK 2D-Dev] RFR: 8273102: Delete deprecated for removal the empty finalize() in java.desktop module
On Sun, 29 Aug 2021 01:09:36 GMT, Sergey Bylokhov wrote: > The "java.desktop" module has a few implementations of the finalize() which > do nothing, deprecated since jdk9, and are marked "forRemoval = true" since > jdk16. > > This is a request to delete such empty methods. > > CSR: https://bugs.openjdk.java.net/browse/JDK-8273103 Looks good to me - Marked as reviewed by pbansal (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5292
Re: [OpenJDK 2D-Dev] RFR: 8272491: Problem list javax/swing/JFrame/NSTexturedJFrame/NSTexturedJFrame.java on macos
On Sun, 15 Aug 2021 16:23:52 GMT, Phil Race wrote: > This will fail regularly so problem list it. Marked as reviewed by pbansal (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5119
Re: [OpenJDK 2D-Dev] RFR: 8252758: Lanai: Optimize index calculation while copying glyphs
On Wed, 5 May 2021 13:50:47 GMT, Jayathirth D V wrote: > Loop optimization while copying glyph content. > J2DDemo, SwingSet2 and Font2DTest are green. Marked as reviewed by pbansal (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3883
Re: [OpenJDK 2D-Dev] RFR: 8263124: Missed initialization of baselineY in sun.font.StrikeMetrics
On Mon, 3 May 2021 23:10:13 GMT, Phil Race wrote: > This is just clean up. The value will be zero by default and explicitly (if > you follow along far enough) zero. > The no-args constructor just merges physical fonts and those are set up with > zero anyway Marked as reviewed by pbansal (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3846
Re: [OpenJDK 2D-Dev] RFR: 8263488: Verify CWarningWindow works with metal rendering pipeline
On Fri, 19 Mar 2021 10:17:57 GMT, Ajit Ghaisas wrote: > Root cause : > CWarningWindow creates a MTLLayer with null peer. > In MTLLayer.replaceSurfaceData() method, insets should be set only if peer is > not null. > > Fix : > Added the required null check. Marked as reviewed by pbansal (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3086
Re: [OpenJDK 2D-Dev] RFR: 8262497: Delete unused utility methods in ICC_Profile class
On Sun, 28 Feb 2021 04:59:48 GMT, Sergey Bylokhov wrote: > A few utility methods in the ICC_Profile class are unused since 1997, I think > we can delete them. Marked as reviewed by pbansal (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2764
Re: [OpenJDK 2D-Dev] RFR: 6211198: ICC_Profile.getInstance(byte[]): IAE is not specified [v2]
On Sat, 6 Feb 2021 07:05:05 GMT, Sergey Bylokhov wrote: >> The specification of the java.awt.color.ICC_Profile.getInstance(byte[]) is >> updated. >> Its implementation changed over time, and different exceptions were thrown, >> but since JDK-8013430 always throws an IllegalArgumentException on null and >> invalid data. > > Sergey Bylokhov 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 three additional > commits since the last revision: > > - Merge branch 'master' into JDK-6211198 > - Merge branch 'master' into JDK-6211198 > - Initial fix Marked as reviewed by pbansal (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2328
Re: [OpenJDK 2D-Dev] RFR: 8216358: [accessibility] [macos] The focus is invisible when tab to "Image Radio Buttons" and "Image CheckBoxes" [v4]
On Fri, 5 Feb 2021 05:12:00 GMT, Alexander Zuev wrote: >> 8216358: [accessibility] [macos] The focus is invisible when tab to "Image >> Radio Buttons" and "Image CheckBoxes" > > Alexander Zuev has updated the pull request incrementally with one additional > commit since the last revision: > > Remove leftovers from last experiments. Marked as reviewed by pbansal (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2384
Re: [OpenJDK 2D-Dev] RFR: 6436374: Graphics.setColor(null) is not documented
On Wed, 3 Feb 2021 07:04:22 GMT, Prasanta Sadhukhan wrote: > Document setting null setColor similar to way setFont(null) is documented. Marked as reviewed by pbansal (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2371
Re: [OpenJDK 2D-Dev] [jdk16] RFR: 8259237: Demo selection changes with left/right arrow key. No need to press space for selection. [v2]
On Mon, 11 Jan 2021 11:16:02 GMT, Alexander Zuev wrote: > Quick question: why we special casing JRadioButton case? Is there any reason > we should call the action on them? This was the behaviour which was there before JDK-8249548 for JRadioButton and I am just reverting to it. As explained in the PR description, the behaviour before JDK-8249548 was as below Before fix for JDK-8249548 JToggleButton: For all L, user can not navigate/select the buttons added to ButtonGroup properly as mentioned in the JDK-8249548. JRadioButton: For Synth L (Nimbus L), user can not navigate/select the buttons added to ButtonGroup. For AquaL, user can select the buttons added to ButtonGroup by pressing left/right key and needs to press the "space" to perform the set Action. For Other L, user can select/navigate the buttons and the set Action is also performed without pressing "space" - PR: https://git.openjdk.java.net/jdk16/pull/99
Re: [OpenJDK 2D-Dev] [jdk16] RFR: 8259237: Demo selection changes with left/right arrow key. No need to press space for selection. [v2]
> Please review a fix for jdk16 > > Issue: In SwingSet2, if the user navigates through the demos, the demo gets > selected/starts on pressing left/right key only. There is no need to press > the "space" key. Earlier, on pressing the left/right key, only demo icon used > to get focused and user needed to press the "space" to actually select a demo. > > Cause: The SwingSet2 has JToggleButtons added to a ButtonGroup. Each > JToggleButton has an AbstractAction set on it, which loads the demo. Earlier, > when the user pressed Left/Right button, only the selected button used to > change. The Action set on JToggleButton used to perform only on pressing the > "Space" button. Now, the Action is performed on navigating the JToggleButtons > using Left/Right keys without the need to press the "space" key. > > This issue is a side effect of fix done for > https://bugs.openjdk.java.net/browse/JDK-8249548. The issues fixed in > JDK-8249548 were not present in JRadioButton as there was code available to > handle it in AquaButtonRadioUI and BasicRadioButtonUI. The fix done for > JDK-8249548 moved duplicate code from AquaButtonRadioUI and > BasicRadioButtonUI and moved it to BasicButtonUI, so this code is available > for JToggleButton and JRadioButton for all L This was wrong as there is a > difference in behaviour for JRadioButtons added to ButtonGroup for AquaL > and other L In AquaL, the AbstractAction set on JRadioButton is not > performed on button selection and user has to press "space". In other L, > the AbstractAction is performed on selection of Button itself without the > need to press "space". > > Fix: The current change fixes the issue in present bug and keeps the fixes > done in JDK-8249548. Following is the behaviour of JToggleButton and > JRadioButton for different L before JDK-8249548 fix, after JDK-8249548 fix > and after current fix. > > Before fix for JDK-8249548 > JToggleButton: > For all L, user can not navigate/select the buttons added to ButtonGroup > properly as mentioned in the JDK-8249548. > JRadioButton: > For Synth L (Nimbus L), user can not navigate/select the buttons added to > ButtonGroup. > For AquaL, user can select the buttons added to ButtonGroup by pressing > left/right key and needs to press the "space" to perform the set Action. > For Other L, user can select/navigate the buttons and the set Action is > also performed without pressing "space" > > After fix for JDK-8249548 > JToggleButton: > For all L, user can navigate/select the buttons added to ButtonGroups and > the AbstractAction set is performed without the need to press "space". > JRadioButton: > For all L, user can navigate/select the buttons added to ButtonGroups and > the AbstractAction set is performed without the need to press "space". > > After current fix: > JToggleButton: > For all L, user can navigate/select the buttons added to ButtonGroups. > User needs to press "space" to perform the set AbstractAction. > JRadioButton: > For AquaL, the behaviour before JDK-8249548 is restored, so user needs to > press the "space" to perform the set Action. > For all other L (including Synth L), the behaviour is same. User can > navigate/select the buttons added to ButtonGroups and set Action is performed > without pressing "space" > > I have run mach5 jobs with full jtreg/jck and all looks good. Links in JBS. > The test TestButtonGroupFocusTraversal.java is modified such that it fails > without current fix and passes after the fix. The fix also should be verified > by running SwingSet2. Pankaj Bansal has updated the pull request incrementally with one additional commit since the last revision: Remove duplicate call to remove listener - Changes: - all: https://git.openjdk.java.net/jdk16/pull/99/files - new: https://git.openjdk.java.net/jdk16/pull/99/files/903ffcbd..724f5999 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk16=99=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk16=99=00-01 Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod Patch: https://git.openjdk.java.net/jdk16/pull/99.diff Fetch: git fetch https://git.openjdk.java.net/jdk16 pull/99/head:pull/99 PR: https://git.openjdk.java.net/jdk16/pull/99
Re: [OpenJDK 2D-Dev] [jdk16] RFR: 8259237: Demo selection changes with left/right arrow key. No need to press space for selection.
On Mon, 11 Jan 2021 08:05:21 GMT, Prasanta Sadhukhan
wrote:
>> Please review a fix for jdk16
>>
>> Issue: In SwingSet2, if the user navigates through the demos, the demo gets
>> selected/starts on pressing left/right key only. There is no need to press
>> the "space" key. Earlier, on pressing the left/right key, only demo icon
>> used to get focused and user needed to press the "space" to actually select
>> a demo.
>>
>> Cause: The SwingSet2 has JToggleButtons added to a ButtonGroup. Each
>> JToggleButton has an AbstractAction set on it, which loads the demo.
>> Earlier, when the user pressed Left/Right button, only the selected button
>> used to change. The Action set on JToggleButton used to perform only on
>> pressing the "Space" button. Now, the Action is performed on navigating the
>> JToggleButtons using Left/Right keys without the need to press the "space"
>> key.
>>
>> This issue is a side effect of fix done for
>> https://bugs.openjdk.java.net/browse/JDK-8249548. The issues fixed in
>> JDK-8249548 were not present in JRadioButton as there was code available to
>> handle it in AquaButtonRadioUI and BasicRadioButtonUI. The fix done for
>> JDK-8249548 moved duplicate code from AquaButtonRadioUI and
>> BasicRadioButtonUI and moved it to BasicButtonUI, so this code is available
>> for JToggleButton and JRadioButton for all L This was wrong as there is
>> a difference in behaviour for JRadioButtons added to ButtonGroup for AquaL
>> and other L In AquaL, the AbstractAction set on JRadioButton is not
>> performed on button selection and user has to press "space". In other L,
>> the AbstractAction is performed on selection of Button itself without the
>> need to press "space".
>>
>> Fix: The current change fixes the issue in present bug and keeps the fixes
>> done in JDK-8249548. Following is the behaviour of JToggleButton and
>> JRadioButton for different L before JDK-8249548 fix, after JDK-8249548
>> fix and after current fix.
>>
>> Before fix for JDK-8249548
>> JToggleButton:
>> For all L, user can not navigate/select the buttons added to ButtonGroup
>> properly as mentioned in the JDK-8249548.
>> JRadioButton:
>> For Synth L (Nimbus L), user can not navigate/select the buttons added
>> to ButtonGroup.
>> For AquaL, user can select the buttons added to ButtonGroup by pressing
>> left/right key and needs to press the "space" to perform the set Action.
>> For Other L, user can select/navigate the buttons and the set Action is
>> also performed without pressing "space"
>>
>> After fix for JDK-8249548
>> JToggleButton:
>> For all L, user can navigate/select the buttons added to ButtonGroups and
>> the AbstractAction set is performed without the need to press "space".
>> JRadioButton:
>> For all L, user can navigate/select the buttons added to ButtonGroups and
>> the AbstractAction set is performed without the need to press "space".
>>
>> After current fix:
>> JToggleButton:
>> For all L, user can navigate/select the buttons added to ButtonGroups.
>> User needs to press "space" to perform the set AbstractAction.
>> JRadioButton:
>> For AquaL, the behaviour before JDK-8249548 is restored, so user needs to
>> press the "space" to perform the set Action.
>> For all other L (including Synth L), the behaviour is same. User can
>> navigate/select the buttons added to ButtonGroups and set Action is
>> performed without pressing "space"
>>
>> I have run mach5 jobs with full jtreg/jck and all looks good. Links in JBS.
>> The test TestButtonGroupFocusTraversal.java is modified such that it fails
>> without current fix and passes after the fix. The fix also should be
>> verified by running SwingSet2.
>
> src/java.desktop/macosx/classes/com/apple/laf/AquaButtonUI.java line 228:
>
>> 226: if (listener != null) {
>> 227: b.removeMouseListener(listener);
>> 228: b.removeMouseListener(listener);
>
> Why are we removing same listener twice?
I just rolled back the changes I did in this file in JDK-8249548. Looks like
this was has been there for some time. See the file in commit made in this file
before JDK-8249548
https://github.com/openjdk/jdk/blob/c18080fef714949221c8ddabc4e23d409575c3d3/src/java.desktop/macosx/classes/com/apple/laf/AquaButtonUI.java.
But yes, it should not be done and I will remove it and update the PR.
-
PR: https://git.openjdk.java.net/jdk16/pull/99
Re: [OpenJDK 2D-Dev] RFR: 8255710: Opensource unit/regression tests for CMM
On Sat, 9 Jan 2021 07:28:20 GMT, Sergey Bylokhov wrote: > A few tests for the CMM moved to the open repo. Marked as reviewed by pbansal (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2012
[OpenJDK 2D-Dev] [jdk16] RFR: 8259237: Demo selection changes with left/right arrow key. No need to press space for selection.
Please review a fix for jdk16 Issue: In SwingSet2, if the user navigates through the demos, the demo gets selected/starts on pressing left/right key only. There is no need to press the "space" key. Earlier, on pressing the left/right key, only demo icon used to get focused and user needed to press the "space" to actually select a demo. Cause: The SwingSet2 has JToggleButtons added to a ButtonGroup. Each JToggleButton has an AbstractAction set on it, which loads the demo. Earlier, when the user pressed Left/Right button, only the selected button used to change. The Action set on JToggleButton used to perform only on pressing the "Space" button. Now, the Action is performed on navigating the JToggleButtons using Left/Right keys without the need to press the "space" key. This issue is a side effect of fix done for https://bugs.openjdk.java.net/browse/JDK-8249548. The issues fixed in JDK-8249548 were not present in JRadioButton as there was code available to handle it in AquaButtonRadioUI and BasicRadioButtonUI. The fix done for JDK-8249548 moved duplicate code from AquaButtonRadioUI and BasicRadioButtonUI and moved it to BasicButtonUI, so this code is available for JToggleButton and JRadioButton for all L This was wrong as there is a difference in behaviour for JRadioButtons added to ButtonGroup for AquaL and other L In AquaL, the AbstractAction set on JRadioButton is not performed on button selection and user has to press "space". In other L, the AbstractAction is performed on selection of Button itself without the need to press "space". Fix: The current change fixes the issue in present bug and keeps the fixes done in JDK-8249548. Following is the behaviour of JToggleButton and JRadioButton for different L before JDK-8249548 fix, after JDK-8249548 fix and after current fix. Before fix for JDK-8249548 JToggleButton: For all L, user can not navigate/select the buttons added to ButtonGroup properly as mentioned in the JDK-8249548. JRadioButton: For Synth L (Nimbus L), user can not navigate/select the buttons added to ButtonGroup. For AquaL, user can select the buttons added to ButtonGroup by pressing left/right key and needs to press the "space" to perform the set Action. For Other L, user can select/navigate the buttons and the set Action is also performed without pressing "space" After fix for JDK-8249548 JToggleButton: For all L, user can navigate/select the buttons added to ButtonGroups and the AbstractAction set is performed without the need to press "space". JRadioButton: For all L, user can navigate/select the buttons added to ButtonGroups and the AbstractAction set is performed without the need to press "space". After current fix: JToggleButton: For all L, user can navigate/select the buttons added to ButtonGroups. User needs to press "space" to perform the set AbstractAction. JRadioButton: For AquaL, the behaviour before JDK-8249548 is restored, so user needs to press the "space" to perform the set Action. For all other L (including Synth L), the behaviour is same. User can navigate/select the buttons added to ButtonGroups and set Action is performed without pressing "space" I have run mach5 jobs with full jtreg/jck and all looks good. Links in JBS. The test TestButtonGroupFocusTraversal.java is modified such that it fails without current fix and passes after the fix. The fix also should be verified by running SwingSet2. - Commit messages: - 8259237: Demo selection changes with left/right arrow key. No need to press space for selection. Changes: https://git.openjdk.java.net/jdk16/pull/99/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk16=99=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8259237 Stats: 375 lines in 4 files changed: 350 ins; 20 del; 5 mod Patch: https://git.openjdk.java.net/jdk16/pull/99.diff Fetch: git fetch https://git.openjdk.java.net/jdk16 pull/99/head:pull/99 PR: https://git.openjdk.java.net/jdk16/pull/99
Re: [OpenJDK 2D-Dev] RFR: 8254798: Deprecate for removal an empty finalize() methods in java.desktop module
On Thu, 15 Oct 2020 05:29:42 GMT, Sergey Bylokhov wrote: > The java.desktop module has a few implementations of the finalize() method > which do nothing. We can remove such methods > if the class is not public API, and could mark these methods in the public > classes as "forRemoval = true" Marked as reviewed by pbansal (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/675
Re: [OpenJDK 2D-Dev] RFR: 8252817: Cleanup the classes in the java.awt.color package
On Sat, 5 Sep 2020 21:32:42 GMT, Sergey Bylokhov wrote: > - Class declarations reformat > - Docs cleanup +1 - Marked as reviewed by pbansal (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/22
Re: [OpenJDK 2D-Dev] RFR: 8251469 Better cleanup for test/jdk/javax/imageio/SetOutput.java
looks good to me -Pankaj On 16/08/20 9:18 AM, Sergey Bylokhov wrote: Hello. Please review the fix for jdk/client. Bug: https://bugs.openjdk.java.net/browse/JDK-8251469 Fix: http://cr.openjdk.java.net/~serb/8251469/webrev.00 One more test which leaks the temp file, because it does not close the IOS stream.
Re: [OpenJDK 2D-Dev] RFR: 8240518: Incorrect JNU_ReleaseStringPlatformChars in Windows Print
Hi Toshio, I will commit this for you. Regards, Pankaj On 06/03/20 6:10 AM, Toshio 5 Nakamura wrote: Hi Sergey, Prasanta, Pankaj, Thank you for the review. May I ask a sponsor of the fix as well? I'm not a committer. Best Regards, Toshio Nakamura Pankaj Bansal wrote on 2020/03/05 18:22:43: > From: Pankaj Bansal > To: Toshio 5 Nakamura , awt- > d...@openjdk.java.net, 2d-dev <2d-dev@openjdk.java.net> > Date: 2020/03/05 18:22 > Subject: [EXTERNAL] RE: RFR: 8240518: Incorrect > JNU_ReleaseStringPlatformChars in Windows Print > > Looks good to me > > -Pankaj > > -Original Message- > From: Sergey Bylokhov > Sent: Thursday, March 5, 2020 2:35 PM > To: Toshio 5 Nakamura ; awt- > d...@openjdk.java.net; 2d-dev <2d-dev@openjdk.java.net> > Subject: Re: RFR: 8240518: Incorrect > JNU_ReleaseStringPlatformChars in Windows Print > > (CC) 2d-dev, > > Looks fine. > > On 3/4/20 4:48 am, Toshio 5 Nakamura wrote: > > Hi, > > > > I'd like to ask review and sponsor of this fix. > > > > Issue: https://bugs.openjdk.java.net/browse/JDK-8240518 > > Fix: http://cr.openjdk.java.net/~tnakamura/8240518/webrev.00 <http://cr.openjdk.java.net/%7Etnakamura/8240518/webrev.00> > > > > Eclipse OpenJ9 VM detects two errors about > JNU_ReleaseStringPlatformChars in WPrinterJob.cpp. > > Then, I checked similar situation in the folder, and found one > lacking of the release method in awt_PrintControl.cpp. > > > > Best regards, > > > > Toshio Nakamura > > > > > -- > Best regards, Sergey. >
Re: [OpenJDK 2D-Dev] RFR: 8240518: Incorrect JNU_ReleaseStringPlatformChars in Windows Print
Looks good to me -Pankaj -Original Message- From: Sergey Bylokhov Sent: Thursday, March 5, 2020 2:35 PM To: Toshio 5 Nakamura ; awt-...@openjdk.java.net; 2d-dev <2d-dev@openjdk.java.net> Subject: Re: RFR: 8240518: Incorrect JNU_ReleaseStringPlatformChars in Windows Print (CC) 2d-dev, Looks fine. On 3/4/20 4:48 am, Toshio 5 Nakamura wrote: > Hi, > > I'd like to ask review and sponsor of this fix. > > Issue: https://bugs.openjdk.java.net/browse/JDK-8240518 > Fix: http://cr.openjdk.java.net/~tnakamura/8240518/webrev.00 > > Eclipse OpenJ9 VM detects two errors about JNU_ReleaseStringPlatformChars in > WPrinterJob.cpp. > Then, I checked similar situation in the folder, and found one lacking of the > release method in awt_PrintControl.cpp. > > Best regards, > > Toshio Nakamura > -- Best regards, Sergey.
Re: [OpenJDK 2D-Dev] [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java
Hi Prahalad, Thanks for the review. Yes, the test fails at 1.75 scale value both before and after the fix on GDI, OpenGL and D3D pipelines with exceptions similar to this. Failed. Execution failed: `main' threw exception: java.lang.RuntimeException: Test failed at x=10 y=10 (expected=0x actual=0x6060) In this bug, I was working to remove the artifact in the image as shown in the image attached in the JBS. The issue is reproducible at scale 2.5 and similar on only OpenGL pipeline. I mostly worked to remove the artifact at mentioned scale. The proposed fix removes that artifact from the image. I am not very sure if it’s a Robot issue or not, but I have some reasons to believe that. I will work on it and let you know. Regards, Pankaj Bansal -Original Message- From: Prahalad Kumar Narayanan Sent: Tuesday, November 14, 2017 3:15 PM To: Sergey Bylokhov; Pankaj Bansal; awt-...@openjdk.java.net; 2d-dev@openjdk.java.net; Philip Race Subject: RE: [OpenJDK 2D-Dev] [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java Hello Pankaj Good day to you. I imported your changes and checked the build with the JTreg test file. . I tried random uiScales using -Dsun.java2d.uiScale and by adjusting DPI scale on Windows 10. . The test failed at uiScale set at 1.75 on all the pipelines- D3D, OpenGL and GDI. . As you explained offline, this could be due to Robot running on HiDPI tests. Kindly check this out. . Sergey's suggestion to include "@run main/othervm DrawImageBilinear" will enable the test-case use the DPI scale as available on host machine. . Hence any uiScale (1.75) is possible & makes test vulnerable for failures. I 've not reviewed the code yet. I 'm working on a similar D3D bug and will need some time to review code. If other reviewers approve of the change, kindly proceed with the check-in. Thank you Have a good day Prahalad -Original Message- From: Sergey Bylokhov Sent: Tuesday, November 14, 2017 3:57 AM To: Pankaj Bansal; awt-...@openjdk.java.net; 2d-dev@openjdk.java.net; Philip Race Subject: Re: [OpenJDK 2D-Dev] [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java I have only one comment: @run main/othervm DrawImageBilinear Should we preserve this tag and run the test in the common-default config? On 09/11/2017 08:43, Pankaj Bansal wrote: > Hi Sergey, > > Do these changes look good? Please give your feedback. > > Regards, > Pankaj Bansal > > -----Original Message- > From: Pankaj Bansal > Sent: Wednesday, November 8, 2017 8:09 PM > To: Sergey Bylokhov; awt-...@openjdk.java.net; > 2d-dev@openjdk.java.net; Philip Race > Subject: RE: [10] Review Request: JDK-8159142 : Visible > artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java > > + 2d-dev, Phil. > > Regards, > Pankaj Bansal > > -Original Message- > From: Pankaj Bansal > Sent: Friday, November 3, 2017 3:11 PM > To: Sergey Bylokhov; awt-...@openjdk.java.net > Subject: RE: [10] Review Request: JDK-8159142 : Visible > artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java > > Hi Sergey, > > > XRSurfaceData, GLXSurfaceData:On linux, only integer scale is > supported. If we give non-integer, the the uiScale is truncated to integer > and used. So the ceil or round does not matter. In both XRSurfaceData and > GLXSurfaceData, ceil function or direct multiplication is used arbitrarily at > different places. So, we can make changes for making things more consistent, > but it is not necessary as of now. > PS: There are artifacts in same test case on Linux and there is a separate > bug https://bugs.openjdk.java.net/browse/JDK-8189957. But these don't seem to > be result of this precision error. I will look into this bug later. > > WGLSurfaceData: In openGL, ceil function is used in WGLSurfaceData, so it > will create WGLOffScreenSurfaceData and in turn create native resource. Then > in DrawImage class, the clipRound is used, and these sizes are sent to native > size. Then in OGLBlitTextureToSurface function inside OGLBlitLoops file, the > texture coordinates are calculated by dividing the sizes sent from Java side > by the native resource size. So the texture coordinates are wrong. For > instance, in DrawImageBilinear, the size is 5x5 and uiScale is 2.5, so the > native resource is of size, 13x13. But the dimensions sent to native are > 12x12, so the textures are (0,0) and (12/13, 12/13). So artifacts in final > image. > > D3DSurfaceData: In D3D, everything is similar to OpenGL, but in > D3DBlitTextureToSurface function inside D3DBlitLoops file, the calculated > texture coordinates are sent to D3DDrawTextureWithHint and
Re: [OpenJDK 2D-Dev] [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java
Hi Phil, I think we discussed that there are many artifacts in both Windows and Linux and we should try to fix one at a time. So we should try to remove all artifacts from Windows first and then move on to something else. Also, the linux contains artifacts even without non-integer scaling https://bugs.openjdk.java.net/browse/JDK-8189957. So I think we should first try to solve these issues first in linux before moving ahead. So I will fix these issues when I work on the listed bug. So for time being, I am working on the current bug to remove issue on Windows and want a feedback on the changes done for the same. I have changed the test and added a @run to run with -Dsun.java2d.d3d=false. I have tested this on my machine (Intel chipset) and another machine which supports D3D. It passes on all. Webrev: http://cr.openjdk.java.net/~pbansal/8159142/webrev.02/ Regards, Pankaj Bansal From: Phil Race Sent: Friday, November 10, 2017 3:26 AM To: Pankaj Bansal; Sergey Bylokhov; awt-...@openjdk.java.net; 2d-dev@openjdk.java.net Subject: Re: [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java Wasn't it suggested to have the Linux code stop truncating the uiScale if it was explicitly set to a non-integer value ? Or at least have another property to tell it not to truncate. This would facilitate ongoing testing of fixing Linux so we can support fractional values. To test the software pipeline it may be that there should be one more line added to these to disable the default accelerated pipeline 29 * @run main/othervm -Dsun.java2d.uiScale=2.5 DrawImageBilinear 30 * @run main/othervm -Dsun.java2d.uiScale=2.5 -Dsun.java2d.opengl=True -Dsun.java2d.opengl.fbobject=true DrawImageBilinear @run main/othervm -Dsun.java2d.uiScale=2.5 -Dsun.java2d.3d=false -Dsun.java2d.xrender=false That will ensure that on Windows we test the GDI pipeline and on Linux we test the X11 pipeline -phil. On 11/09/2017 08:43 AM, Pankaj Bansal wrote: Hi Sergey, Do these changes look good? Please give your feedback. Regards, Pankaj Bansal -Original Message- From: Pankaj Bansal Sent: Wednesday, November 8, 2017 8:09 PM To: Sergey Bylokhov; HYPERLINK "mailto:awt-...@openjdk.java.net"awt-...@openjdk.java.net; HYPERLINK "mailto:2d-dev@openjdk.java.net"2d-dev@openjdk.java.net; Philip Race Subject: RE: [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java + 2d-dev, Phil. Regards, Pankaj Bansal -Original Message- From: Pankaj Bansal Sent: Friday, November 3, 2017 3:11 PM To: Sergey Bylokhov; HYPERLINK "mailto:awt-...@openjdk.java.net"awt-...@openjdk.java.net Subject: RE: [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java Hi Sergey, XRSurfaceData, GLXSurfaceData: On linux, only integer scale is supported. If we give non-integer, the the uiScale is truncated to integer and used. So the ceil or round does not matter. In both XRSurfaceData and GLXSurfaceData, ceil function or direct multiplication is used arbitrarily at different places. So, we can make changes for making things more consistent, but it is not necessary as of now. PS: There are artifacts in same test case on Linux and there is a separate bug https://bugs.openjdk.java.net/browse/JDK-8189957. But these don't seem to be result of this precision error. I will look into this bug later. WGLSurfaceData: In openGL, ceil function is used in WGLSurfaceData, so it will create WGLOffScreenSurfaceData and in turn create native resource. Then in DrawImage class, the clipRound is used, and these sizes are sent to native size. Then in OGLBlitTextureToSurface function inside OGLBlitLoops file, the texture coordinates are calculated by dividing the sizes sent from Java side by the native resource size. So the texture coordinates are wrong. For instance, in DrawImageBilinear, the size is 5x5 and uiScale is 2.5, so the native resource is of size, 13x13. But the dimensions sent to native are 12x12, so the textures are (0,0) and (12/13, 12/13). So artifacts in final image. D3DSurfaceData: In D3D, everything is similar to OpenGL, but in D3DBlitTextureToSurface function inside D3DBlitLoops file, the calculated texture coordinates are sent to D3DDrawTextureWithHint and then data is copied from texture to surface. Here if the native resource size and dimensions sent from Java are not same, the data is copied in parts. This is somehow handling the wrong texture coordinates. So this test does not produce any artifacts in D3D pipeline. But it can in some other case. So this needs to be fixed. GDIWindowSurfacedata: Here the volatile image creates a BufImageSurfaceData as backing resource using the ceil function and while copying from BugImageSurfaceData to a GDIWindowSurface, the Java_sun_java2d_l
Re: [OpenJDK 2D-Dev] [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java
Hi Sergey, Do these changes look good? Please give your feedback. Regards, Pankaj Bansal -Original Message- From: Pankaj Bansal Sent: Wednesday, November 8, 2017 8:09 PM To: Sergey Bylokhov; awt-...@openjdk.java.net; 2d-dev@openjdk.java.net; Philip Race Subject: RE: [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java + 2d-dev, Phil. Regards, Pankaj Bansal -Original Message- From: Pankaj Bansal Sent: Friday, November 3, 2017 3:11 PM To: Sergey Bylokhov; awt-...@openjdk.java.net Subject: RE: [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java Hi Sergey, XRSurfaceData, GLXSurfaceData:On linux, only integer scale is supported. If we give non-integer, the the uiScale is truncated to integer and used. So the ceil or round does not matter. In both XRSurfaceData and GLXSurfaceData, ceil function or direct multiplication is used arbitrarily at different places. So, we can make changes for making things more consistent, but it is not necessary as of now. PS: There are artifacts in same test case on Linux and there is a separate bug https://bugs.openjdk.java.net/browse/JDK-8189957. But these don't seem to be result of this precision error. I will look into this bug later. WGLSurfaceData: In openGL, ceil function is used in WGLSurfaceData, so it will create WGLOffScreenSurfaceData and in turn create native resource. Then in DrawImage class, the clipRound is used, and these sizes are sent to native size. Then in OGLBlitTextureToSurface function inside OGLBlitLoops file, the texture coordinates are calculated by dividing the sizes sent from Java side by the native resource size. So the texture coordinates are wrong. For instance, in DrawImageBilinear, the size is 5x5 and uiScale is 2.5, so the native resource is of size, 13x13. But the dimensions sent to native are 12x12, so the textures are (0,0) and (12/13, 12/13). So artifacts in final image. D3DSurfaceData: In D3D, everything is similar to OpenGL, but in D3DBlitTextureToSurface function inside D3DBlitLoops file, the calculated texture coordinates are sent to D3DDrawTextureWithHint and then data is copied from texture to surface. Here if the native resource size and dimensions sent from Java are not same, the data is copied in parts. This is somehow handling the wrong texture coordinates. So this test does not produce any artifacts in D3D pipeline. But it can in some other case. So this needs to be fixed. GDIWindowSurfacedata: Here the volatile image creates a BufImageSurfaceData as backing resource using the ceil function and while copying from BugImageSurfaceData to a GDIWindowSurface, the Java_sun_java2d_loops_TransformHelper_Transform in native side is able to handle the difference in sizes. So the artifacts are not visible. But I think this should be fixed. So should make changes to getBackupImage function in SunVolatileImage to use clipRound instead of ceil. BufImageSurfaceData : It uses the image raster size to create the native resource. The raster size is already scaled according to the uiScale. So no need to change this. So in a nutshell, I feel we may not change the code in XRSurfaceData, GLXSurfaceData as linux only support integer scaling. We can do it later if required. We should change code for D3DSurfaceData, GDIWindowSurfacedata and WGLSurfacedata as it may cause artifacts. I have updated the webrev for the same. Please review. Webrev: http://cr.openjdk.java.net/~pbansal/8159142/webrev.01/ Regards, Pankaj Bansal -Original Message- From: Sergey Bylokhov Sent: Wednesday, November 1, 2017 11:46 PM To: Pankaj Bansal; awt-...@openjdk.java.net Subject: Re: [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java Hi, Pankaj. D3DSurfaceData, XRSurfaceData and probably other SurfaceData use simple multiplication or ceil(). Should we update them also? and the test to catch the errors there as well. On 30/10/2017 03:39, Pankaj Bansal wrote: > Hi All, > > Please review the fix for JDK 10. > > Bug: > > https://bugs.openjdk.java.net/browse/JDK-8159142 > > Webrev: > > http://cr.openjdk.java.net/~pbansal/8159142/webrev.00/ > > Issue: > > The test runs with D3D and OpenGL both. The issue is with the OpenGL > run as there aee some visual artifacts in image when the HiDPI scale > is set to non-integer value like 2.5. > > Fix: > > The issue is due to precision error. In WGLSurfaceData.java, the width > and height is calculated by multiplying the width and height with scale. > But the ceil function was being used instead of round. It should be > using round as at most of the places, the round function is being used. > > Also changed the test to run at uiScale of 2.5 to catch any further errors. > > Regards, > > Pankaj Bansal > -- Best regards, Sergey.
Re: [OpenJDK 2D-Dev] [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java
+ 2d-dev, Phil. Regards, Pankaj Bansal -Original Message- From: Pankaj Bansal Sent: Friday, November 3, 2017 3:11 PM To: Sergey Bylokhov; awt-...@openjdk.java.net Subject: RE: [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java Hi Sergey, XRSurfaceData, GLXSurfaceData:On linux, only integer scale is supported. If we give non-integer, the the uiScale is truncated to integer and used. So the ceil or round does not matter. In both XRSurfaceData and GLXSurfaceData, ceil function or direct multiplication is used arbitrarily at different places. So, we can make changes for making things more consistent, but it is not necessary as of now. PS: There are artifacts in same test case on Linux and there is a separate bug https://bugs.openjdk.java.net/browse/JDK-8189957. But these don't seem to be result of this precision error. I will look into this bug later. WGLSurfaceData: In openGL, ceil function is used in WGLSurfaceData, so it will create WGLOffScreenSurfaceData and in turn create native resource. Then in DrawImage class, the clipRound is used, and these sizes are sent to native size. Then in OGLBlitTextureToSurface function inside OGLBlitLoops file, the texture coordinates are calculated by dividing the sizes sent from Java side by the native resource size. So the texture coordinates are wrong. For instance, in DrawImageBilinear, the size is 5x5 and uiScale is 2.5, so the native resource is of size, 13x13. But the dimensions sent to native are 12x12, so the textures are (0,0) and (12/13, 12/13). So artifacts in final image. D3DSurfaceData: In D3D, everything is similar to OpenGL, but in D3DBlitTextureToSurface function inside D3DBlitLoops file, the calculated texture coordinates are sent to D3DDrawTextureWithHint and then data is copied from texture to surface. Here if the native resource size and dimensions sent from Java are not same, the data is copied in parts. This is somehow handling the wrong texture coordinates. So this test does not produce any artifacts in D3D pipeline. But it can in some other case. So this needs to be fixed. GDIWindowSurfacedata: Here the volatile image creates a BufImageSurfaceData as backing resource using the ceil function and while copying from BugImageSurfaceData to a GDIWindowSurface, the Java_sun_java2d_loops_TransformHelper_Transform in native side is able to handle the difference in sizes. So the artifacts are not visible. But I think this should be fixed. So should make changes to getBackupImage function in SunVolatileImage to use clipRound instead of ceil. BufImageSurfaceData : It uses the image raster size to create the native resource. The raster size is already scaled according to the uiScale. So no need to change this. So in a nutshell, I feel we may not change the code in XRSurfaceData, GLXSurfaceData as linux only support integer scaling. We can do it later if required. We should change code for D3DSurfaceData, GDIWindowSurfacedata and WGLSurfacedata as it may cause artifacts. I have updated the webrev for the same. Please review. Webrev: http://cr.openjdk.java.net/~pbansal/8159142/webrev.01/ Regards, Pankaj Bansal -Original Message- From: Sergey Bylokhov Sent: Wednesday, November 1, 2017 11:46 PM To: Pankaj Bansal; awt-...@openjdk.java.net Subject: Re: [10] Review Request: JDK-8159142 : Visible artifacts in sun/java2d/SunGraphics2D/DrawImageBilinear.java Hi, Pankaj. D3DSurfaceData, XRSurfaceData and probably other SurfaceData use simple multiplication or ceil(). Should we update them also? and the test to catch the errors there as well. On 30/10/2017 03:39, Pankaj Bansal wrote: > Hi All, > > Please review the fix for JDK 10. > > Bug: > > https://bugs.openjdk.java.net/browse/JDK-8159142 > > Webrev: > > http://cr.openjdk.java.net/~pbansal/8159142/webrev.00/ > > Issue: > > The test runs with D3D and OpenGL both. The issue is with the OpenGL > run as there aee some visual artifacts in image when the HiDPI scale > is set to non-integer value like 2.5. > > Fix: > > The issue is due to precision error. In WGLSurfaceData.java, the width > and height is calculated by multiplying the width and height with scale. > But the ceil function was being used instead of round. It should be > using round as at most of the places, the round function is being used. > > Also changed the test to run at uiScale of 2.5 to catch any further errors. > > Regards, > > Pankaj Bansal > -- Best regards, Sergey.
[OpenJDK 2D-Dev] [10] Review Request JDK - 8164811 : [hidpi]Tests fail with OpenGL Rendering
Hi All, Please review the fix for JDK 10. Bug: https://bugs.openjdk.java.net/browse/JDK-8164811 Webrev: http://cr.openjdk.java.net/~pbansal/8164811/webrev.00/ Issue: The tests given in the bug were failing when run with OpenGL and GDI. This bug is related to https://bugs.openjdk.java.net/browse/JDK-8189257 which states that that HIDPI does not work with swing components when Translucent window is used. Because of which all the tests in the bug were failing. Fix: The TranslucentWindowPainter class was creating BufferedImage for OpenGL (when forceOpt is false) and GDI pipeline, but it is not considering the device HiDPI scale. There is no way to create a scaled BufferedImage because of which the scale value in BufImgSurfaceData is always 1. Made changes to store graphics config in Buffered image, so that the BufImgSurfaceManager can create BufImgSurfaceData with scale set properly. This fix also fixes https://bugs.openjdk.java.net/browse/JDK-8189257 Regards, Pankaj Bansal
