RFR: 8329820: [Linux] Prefer EGL over GLX [v3]

Thiago Milczarek Sayao tsayao at openjdk.org
Sun Apr 28 15:32:23 UTC 2024


On Thu, 25 Apr 2024 12:19:19 GMT, Lukasz Kostyra <lkostyra at openjdk.org> wrote:

>> Thiago Milczarek Sayao has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Forgot debug message
>
> 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`

You're right. Fixed.

> 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

Removed this call as it needs a surface.

> 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

Fixed.

> 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

Fixed. `eglSwapInterval` fails without a surface, so added a check.

> 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.

Added.

-------------

PR Review Comment: https://git.openjdk.org/jfx/pull/1381#discussion_r1582202507
PR Review Comment: https://git.openjdk.org/jfx/pull/1381#discussion_r1582202270
PR Review Comment: https://git.openjdk.org/jfx/pull/1381#discussion_r1582202434
PR Review Comment: https://git.openjdk.org/jfx/pull/1381#discussion_r1582202110
PR Review Comment: https://git.openjdk.org/jfx/pull/1381#discussion_r1582202475


More information about the openjfx-dev mailing list