On Sat, 24 Feb 2024 17:54:47 GMT, Thiago Milczarek Sayao <tsa...@openjdk.org> wrote:
> GLX is X only while EGL is need for Wayland and also works with X.org. > > I know there are limitations on Xorg and I still need to investigate it. > > GLX replacement for Xorg is not mandatory - this can be further discussed. > > This is a work in progress and it's on the first step (making it work). > > 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) I took a brief look at your changes and I shared some comments. Overall the changes look good. The EGL/GLX loading in GLFactory feels a bit hacky, but other than making EGL a completely separate Prism backend (which would duplicate/reorganize a lot of common EGL/GLX code - definitely a no-go IMO) I don't have a better idea how to solve this. Maybe with an ES2-specific property like `prism.es2.forceglx=true`? This brings me to a question - other than some form of debugging/fallback for when EGL is still new to JFX, do we even need GLX implementation when we have a working EGL implementation? I'm fairly sure that current distros (especially ones officially supported by JFX) have EGL support available at this point in time, and since it's an intermediary layer between application and runtime it shouldn't depend on used GPU. Also a side note - the PR title does not match the JBS issue. modules/javafx.graphics/src/main/java/com/sun/prism/es2/LinuxEGLContext.java line 2: > 1: /* > 2: * Copyright (c) 2012, 2013, Oracle and/or its affiliates. All rights > reserved. Copyright dates should be updated (here and in other files) modules/javafx.graphics/src/main/native-prism-es2/linux/egl/LinuxGLContext.c line 140: > 138: /* allocate the structure */ > 139: ctxInfo = (ContextInfo *) malloc(sizeof (ContextInfo)); > 140: if (ctxInfo == NULL) { Shouldn't this also `eglDestroyContext`? modules/javafx.graphics/src/main/native-prism-es2/linux/egl/LinuxGLContext.c line 158: > 156: /* set function pointers */ > 157: ctxInfo->glActiveTexture = (PFNGLACTIVETEXTUREPROC) > 158: dlsym(RTLD_DEFAULT, "glActiveTexture"); Since you're using EGL, it would be better to use EGL's own `eglGetProcAddress` here and below. ------------- PR Review: https://git.openjdk.org/jfx/pull/1381#pullrequestreview-1973486376 PR Comment: https://git.openjdk.org/jfx/pull/1381#issuecomment-2031952274 PR Review Comment: https://git.openjdk.org/jfx/pull/1381#discussion_r1547718158 PR Review Comment: https://git.openjdk.org/jfx/pull/1381#discussion_r1547684895 PR Review Comment: https://git.openjdk.org/jfx/pull/1381#discussion_r1547688868
