8388849: javax/swing/JInternalFrame/8069348/bug8069348.java should ignore execution in non-Windows platform if sun.java2d.d3d=true#32024
Conversation
|
👋 Welcome back psadhukhan! A progress list of the required criteria for merging this PR into |
|
@prsadhuk This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be: You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been 24 new commits pushed to the
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details. ➡️ To integrate this PR with the above commit message to the |
|
The total number of required reviews for this PR has been set to 2 based on the presence of this label: |
Webrevs
|
| String d3d = System.getProperty("sun.java2d.d3d"); | ||
| System.out.println("d3d " + d3d); | ||
| return !Boolean.getBoolean(d3d) || getOSType() == OSType.WINDOWS; | ||
| return !Boolean.parseBoolean(d3d) || getOSType() == OSType.WINDOWS; |
There was a problem hiding this comment.
The method looked weird, partly because of the name "isSupported() and I wondered it is trying to achieve
The only thing it does is prevent this invocation from running on mac & linux
@run main/othervm -Dsun.java2d.d3d=true -Dsun.java2d.uiScale=2 bug8069348
presumably because it duplicates this invocation :
@run main/othervm -Dsun.java2d.uiScale=2 bug8069348
But it wasn't stopping the same duplication on macOS .. in the ORIGINAL version of the test
44 * @run main/othervm -Dsun.java2d.opengl=true -Dsun.java2d.uiScale=2 bug8069348
was the default until metal came along
It was removed by https://hg.openjdk.org/jdk/jdk/rev/3b820b878ebe
So some of what isSupported() was trying to achieve is obsoleted.
And anywatd3d is the default on windows where we can enable it and specifying the property doesn't change anything.
i.e it does not force-enable it.
I think to be useful it should have been
@run main/othervm -Dsun.java2d.d3d=false -Dsun.java2d.uiScale=2 bug8069348
then WHERE SUPPORTED this default line would be running the d3d case
@run main/othervm -Dsun.java2d.uiScale=2 bug8069348
So I suggest to just ditch the isSupported() method and use just the default
@run main/othervm -Dsun.java2d.uiScale=2 bug8069348
There was a problem hiding this comment.
Removed isSupported and used the default pipeline
|
@azvegint Can you please re-review it? |
|
/integrate |
|
Going to push as commit 3d44c66.
Your commit was automatically rebased without conflicts. |
Test should not run in non-WIndows platform if
sun.java2d.d3d=truebutisSupportedcall in the test checks forBoolean.getBoolean(d3d)which returns false as it treats "true" as the name of another system propertyso we need to use
Boolean.parseBooleanwhich returns true andisSupportedin that case returns falseso that test should be ignored for non-Windows platform if d3d is true
Since 1st @run execution already tests for default pipeline in non-Windows platform, there's no need to run it again if d3d is true
Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/32024/head:pull/32024$ git checkout pull/32024Update a local copy of the PR:
$ git checkout pull/32024$ git pull https://git.openjdk.org/jdk.git pull/32024/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 32024View PR using the GUI difftool:
$ git pr show -t 32024Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/32024.diff
Using Webrev
Link to Webrev Comment