On Fri, 19 Apr 2024 14:42:23 GMT, Thiago Milczarek Sayao <tsa...@openjdk.org> wrote:
>> 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](https://mozillagfx.wordpress.com/2021/10/30/switching-the-linux-graphics-stack-from-glx-to-egl/) >> [Prefer EGL to GLX for the GL support on >> X11](https://gitlab.gnome.org/GNOME/gtk/-/merge_requests/3540) > > Thiago Milczarek Sayao has updated the pull request incrementally with one > additional commit since the last revision: > > Forgot debug message I left a few comments, mostly for adjusting error-checking. Once those are fixed and others have their input I'll test those changes. modules/javafx.graphics/src/main/native-prism-es2/linux/egl/LinuxGLContext.c line 252: > 250: eglGetProcAddress("glGetProgramInfoLog"); > 251: ctxInfo->glTexImage2DMultisample = (PFNGLTEXIMAGE2DMULTISAMPLEPROC) > 252: dlsym(RTLD_DEFAULT,"glTexImage2DMultisample"); I think these three functions should also be available via `eglGetProcAddress` modules/javafx.graphics/src/main/native-prism-es2/linux/egl/LinuxGLContext.c line 258: > 256: dlsym(RTLD_DEFAULT,"glBlitFramebuffer"); > 257: > 258: eglSwapInterval(eglDisplay, 0); `eglSwapInterval` can potentially fail, we should check for errors here modules/javafx.graphics/src/main/native-prism-es2/linux/egl/LinuxGLContext.c line 267: > 265: > 266: // Release context once we are all done > 267: eglMakeCurrent(eglDisplay, EGL_NO_SURFACE, EGL_NO_SURFACE, > EGL_NO_CONTEXT); Similar as above, `eglMakeCurrent` can also fail modules/javafx.graphics/src/main/native-prism-es2/linux/egl/LinuxGLContext.c line 301: > 299: > 300: if (!eglMakeCurrent(ctxInfo->eglDisplay, dInfo->eglSurface, > dInfo->eglSurface, ctxInfo->context)) { > 301: fprintf(stderr, "Failed in eglMakeCurrent"); I think you might be missing a `return` statement here. modules/javafx.graphics/src/main/native-prism-es2/linux/egl/LinuxGLContext.c line 310: > 308: interval = (vSyncNeeded) ? 1 : 0; > 309: ctxInfo->state.vSyncEnabled = vSyncNeeded; > 310: eglSwapInterval(ctxInfo->eglDisplay, interval); Check for errors like above modules/javafx.graphics/src/main/native-prism-es2/linux/egl/LinuxGLDrawable.c line 58: > 56: } > 57: > 58: EGLSurface eglSurface = eglCreateWindowSurface(pfInfo->eglDisplay, > pfInfo->eglConfig, I would add a check to make sure the surface was created successfully, as it can potentially return `EGL_NO_SURFACE`. Some additional information from `eglGetError` could also come in handy if `eglCreateWindowSurface` does fail. modules/javafx.graphics/src/main/native-prism-es2/linux/egl/LinuxGLDrawable.c line 119: > 117: > 118: eglSwapBuffers(eglGetCurrentDisplay(), dInfo->eglSurface); > 119: return JNI_TRUE; eglSwapBuffers can fail, so I'd `return (eglSwapBuffers(...) == EGL_TRUE);` or something similar ------------- PR Review: https://git.openjdk.org/jfx/pull/1381#pullrequestreview-2022336712 PR Review Comment: https://git.openjdk.org/jfx/pull/1381#discussion_r1579372523 PR Review Comment: https://git.openjdk.org/jfx/pull/1381#discussion_r1579386732 PR Review Comment: https://git.openjdk.org/jfx/pull/1381#discussion_r1579387499 PR Review Comment: https://git.openjdk.org/jfx/pull/1381#discussion_r1579431452 PR Review Comment: https://git.openjdk.org/jfx/pull/1381#discussion_r1579387989 PR Review Comment: https://git.openjdk.org/jfx/pull/1381#discussion_r1579376454 PR Review Comment: https://git.openjdk.org/jfx/pull/1381#discussion_r1579379362
