On Thu, 25 Apr 2024 12:19:19 GMT, Lukasz Kostyra <lkost...@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
