jfx
jfx copied to clipboard
openjdk
8329820: [Linux] Prefer EGL over GLX
Wayland implementation will require EGL.
EGL works with Xorg as well. The idea is to be EGL first and if it fails, fallback to GLX. A force flag prism.es2.forceGLX=true is available.
See: Switching the Linux graphics stack from GLX to EGL Prefer EGL to GLX for the GL support on X11
Progress
- [x] Change must not contain extraneous whitespace
- [x] Commit message must refer to an issue
- [ ] Change must be properly reviewed (3 reviews required, with at least 1 Reviewer, 2 Authors)
Issue
- JDK-8329820: [Linux] Prefer EGL over GLX (Enhancement - P4)
Reviewing
Using git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jfx.git pull/1381/head:pull/1381
$ git checkout pull/1381
Update a local copy of the PR:
$ git checkout pull/1381
$ git pull https://git.openjdk.org/jfx.git pull/1381/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 1381
View PR using the GUI difftool:
$ git pr show -t 1381
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/1381.diff
Webrev
:wave: Welcome back tsayao! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.
![bridgekeeper[bot] avatar](https://avatars.githubusercontent.com/u/41768318?v=4 "Published by a pub.dev verified publisher"){kind=small}
@tsayao this pull request can not be integrated into master due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:
git checkout egl
git fetch https://git.openjdk.org/jfx.git master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push
❗ This change is not yet ready to be integrated. See the Progress checklist in the description for automated requirements.
Webrevs
- 14: Full (f0efd654)
- 13: Full (0a8307bf)
- 12: Full (7bdd2963)
- 11: Full (d904e8c4)
- 10: Full - Incremental (47985493)
- 09: Full (51c35579)
- 08: Full - Incremental (8618dd77)
- 07: Full - Incremental (619ae9c4)
- 06: Full - Incremental (0c78b749)
- 05: Full - Incremental (f8c61efc)
- 04: Full - Incremental (4d62744a)
- 03: Full - Incremental (aa321efb)
- 02: Full - Incremental (30203eab)
- 01: Full - Incremental (7ae1f275)
- 00: Full (e8330501)
This will need a fair bit of testing and review. I would like a chance to review it, but won't be able to get to it for a couple weeks.
Reviewers: @kevinrushforth @lukostyra @johanvos @arapte /reviewers 3
@kevinrushforth The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 3 (with at least 1 Reviewer, 2 Authors).
I have to figure out how to test Monocle, since I have changed the logic of defines on PrismES2Defs.h.
I changed it to use EGL_DEFAULT_DISPLAY in an attempt to make no assumptions on the platform. It works, but I'm not sure if it's correct. I'm looking at Mesa egl source to be sure.
I also changed the dummy window to use PBUFFER (also to make no assumptions on the platform, so there's no need to create a XWindow). I'm also not sure about that.
It is possible to set the platform with EGL_PLATFORM=x11 environment variable.
Using EGL_LOG_LEVEL=debug currently outputs that the platform was detected in build time - I think it's wrong.
I'll have to fix it
I got a reply explaining it (the docs where not very clear) https://lists.freedesktop.org/archives/mesa-users/2024-April/001727.html
I changed it to use platform specific way to get EGLDisplay so eglGetDisplay does not have to guess the platform (it can guess wrong).
It can be seen by setting EGL_LOG_LEVEL=debug (with eglGetDisplay it will output the guessing part).
I also reverted the Dummy window code change.
@tsayao Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. See OpenJDK Developers’ Guide for more information.
Testing 8332108 with this branch seems to have a significance in favor of EGL:
Using this branch, compare the attached example with and without -Dprism.es2.forceGLX=true
It seems to make a difference when display refresh rate is lower (60Hz). With higher refresh (160hz), no difference.
EGL:
GLX:
@tsayao This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!
Would it be better to still default to GLX and have a flag to use EGL (and switch within a few releases), or to go with EGL and have a flag to fallback to GLX?
Currently (on this PR) it defaults to EGL first, and if it fails, try GLX (I don't know a scenario where EGL won't work, on Xorg o XWayland).
SetSceneScalingTest currently fails with EGL_BAD_ALLOC on nCreateDrawable. I still don't know why - by the file name I would suspect something related to scaling - but there's no scaling on the test (not that I can see).
by the file name I would suspect something related to scaling - but there's no scaling on the test (not that I can see).
It is related to system's DPI setting. We had a problem with UI elements sometimes being incorrectly rendered/shifted when scaling setting in the system was set to more than 100% - see JDK-8299968