Glew to epoxy #1681

sobkas · 2024-05-25T20:17:17Z

Switch from ancient GLEW to libepoxy

AMS21 · 2024-05-26T12:27:47Z

libepoxys last release is 2 years old so why not use the still actively developed glad library, which also supports Vulkan out of the box?

sobkas · 2024-05-26T16:50:26Z

libepoxys last release is 2 years old so why not use the still actively developed glad library, which also supports Vulkan out of the box?

GLAD2 depends on generated code and this code is separated for each API.
So for each API there are files(*.c and *.h) that would be included into OpenXRay.
Epoxy needs only #include to be added.

Xottab-DUTY · 2024-05-26T17:44:27Z

libepoxys last release is 2 years old so why not use the still actively developed glad library, which also supports Vulkan out of the box?

GLEW's last release is 3 years old :)

Though, there's a bad sign there: libepoxy doesn't have any fresh commits since the last release, while glew has.

sobkas · 2024-05-26T18:17:36Z

libepoxys last release is 2 years old so why not use the still actively developed glad library, which also supports Vulkan out of the box?

GLEW's last release is 3 years old :)

Though, there's a bad sign there: libepoxy doesn't have any fresh commits since the last release, while glew has.

libepoxy is used by much more recent software, while glew usage is ancient.

Xottab-DUTY

Thanks!
I've left mostly code style and some other small improvement request!

Xottab-DUTY · 2024-05-26T18:50:00Z

src/Layers/xrRenderGL/glHW.cpp

+#include <epoxy/gl.h>
 #include "xrEngine/XR_IOConsole.h"



Suggested change

#include <epoxy/gl.h>

#include "xrEngine/XR_IOConsole.h"

#include "xrEngine/XR_IOConsole.h"

#include <epoxy/gl.h>

Xottab-DUTY · 2024-05-26T18:50:12Z

src/Layers/xrRenderGL/glHW.cpp

+        mode.format = SDL_PIXELFORMAT_RGBA8888;
+        // Apply the pixel format to the device context


Suggested change

mode.format = SDL_PIXELFORMAT_RGBA8888;

// Apply the pixel format to the device context

mode.format = SDL_PIXELFORMAT_RGBA8888;

// Apply the pixel format to the device context

Xottab-DUTY · 2024-05-26T18:50:19Z

src/Layers/xrRenderGL/glHW.cpp

@@ -103,6 +104,7 @@ void CHW::CreateDevice(SDL_Window* hWnd)
        const Uint32 flags = SDL_WINDOW_BORDERLESS | SDL_WINDOW_HIDDEN | SDL_WINDOW_OPENGL;

        m_helper_window = SDL_CreateWindow("OpenXRay OpenGL helper window", 0, 0, 1, 1, flags);
+


Suggested change

Xottab-DUTY · 2024-05-26T18:51:05Z

src/Layers/xrRenderGL/glHW.h

+    bool GLKHRdebugSupported;
+    bool GLARBtexturefloatSuppoted;
+    bool GLARBvertexattribbindingSupported;


Suggested change

bool GLKHRdebugSupported;

bool GLARBtexturefloatSuppoted;

bool GLARBvertexattribbindingSupported;

bool DebugSupported;

bool TextureFloatSuppoted;

bool VertexAttribBindingSupported;

Xottab-DUTY · 2024-05-26T18:54:38Z

src/Layers/xrRenderGL/glState.h

@@ -36,6 +36,7 @@ class glState
    D3D_DEPTH_STENCIL_STATE m_pDepthStencilState;
    D3D_BLEND_STATE m_pBlendState;
    float m_uiMipLODBias;
+    bool GLEXTtexturefilteranisotropicSupported;


Let's move it to HW with TextureFilterAnisotropicSupported name.
It's generally better to keep such stuff in one place, and also glState can be created on-demand during the game run – that means it would always call epoxy_has_gl_extension for each glState instance (there can be many of them) which is suboptimal.

Xottab-DUTY · 2024-05-26T18:56:36Z

src/Layers/xrRenderGL/glBufferUtils.cpp

@@ -162,14 +162,14 @@ void SetVertexDeclaration(const VertexElement* dxdecl)
    });
 }

-void ConvertVertexDeclaration(const VertexElement* dxdecl, SDeclaration* decl)
+void ConvertVertexDeclaration(const VertexElement* dxdecl, SDeclaration* decl, bool GLARBvertexattribbindingSupported)


This looks less good.
It's better to just use HW.GLARBvertexattribbindingSupported (after renaming it should be HW.VertexAttribBindingSupported) directly.

Xottab-DUTY · 2024-05-26T18:57:31Z

src/Layers/xrRender/ShaderResourceTraits.h

@@ -61,7 +61,7 @@ inline std::pair<char, GLuint> GLCompileShader(pcstr* buffer, size_t size, pcstr

    const GLuint program = glCreateProgram();
    R_ASSERT(program);
-    if (GLEW_VERSION_4_3)
+    if (epoxy_gl_version() >= 43)


Let's check for HW.OpenGLVersion instead of calling external function.

Xottab-DUTY · 2024-05-26T19:03:40Z

cmake/Findepoxy.cmake

Please, try to use this find module: https://github.com/WebKit/WebKit/blob/main/Source/cmake/FindEpoxy.cmake
It provides Epoxy::Epoxy CMake target which can be used.

Xottab-DUTY · 2024-05-26T19:04:31Z

src/Layers/xrRenderPC_GL/CMakeLists.txt

@@ -396,6 +396,7 @@ target_include_directories(xrRender_GL
    "${CMAKE_SOURCE_DIR}/Externals"
    "${CMAKE_SOURCE_DIR}/Externals/gli"
    "${CMAKE_SOURCE_DIR}/Externals/gli/external"
+    "${epoxy_INCLUDE_DIR}"


Can be removed after switching to Epoxy::Epoxy (#1681 (comment)).

Xottab-DUTY · 2024-05-26T19:04:33Z

src/Layers/xrRenderPC_GL/CMakeLists.txt

@@ -408,8 +409,7 @@ target_link_libraries(xrRender_GL
    xrParticles
    xrScriptEngine
    xrImGui
-    OpenGL::GL
-    GLEW::GLEW
+    "${epoxy_LIBRARY}"


Try to switch to Epoxy::Epoxy.

AMS21 · 2024-05-27T09:12:23Z

libepoxys last release is 2 years old so why not use the still actively developed glad library, which also supports Vulkan out of the box?

GLEW's last release is 3 years old :)

Though, there's a bad sign there: libepoxy doesn't have any fresh commits since the last release, while glew has.

Maybe I'm being missunderstood here I'm all for replacing the ancient GLEW library. My point was rather to maybe consider using the glad library I linked which latest release is about 1 Month old and still actively developed while also supporting all OpenGL extensions as well as Vulkan (which we might want to use in the future). Also glad directly parses the official OpenGL and Vulkan spec to generate it's glue code so even if the library is ever abandoned the parser should still be able to generate bindings for never versions of OpenGL and Vulkan (at least untill they make fundamental changes to the spec).

Xottab-DUTY · 2024-05-28T17:02:11Z

@AMS21 the @sobkas's concern is this:

GLAD2 depends on generated code and this code is separated for each API.
So for each API there are files(*.c and *.h) that would be included into OpenXRay.
Epoxy needs only #include to be added.

I also had a concern that if you select OpenGL 4.6, then you won't be able to use e.g. OpenGL 3.3 or OpenGL 2.0.
But I just checked the generated files and found that we were (or I was) wrong.

GLAD2 supports working with multiple APIs simultaneously. On it's website you just need to select maximal OpenGL version and any lower version would be included.
It also support header-only option.

Xottab-DUTY · 2024-05-29T17:07:58Z

So main concern with libepoxy is that it's author hadn't been active for two years. No commits, not even PR merges, etc.
It also doesn't have simple global variables indicating if an extension is available. Honestly, they are pretty handy to use.

Xottab-DUTY · 2024-06-01T15:14:34Z

Superseded with #1684.

github-actions bot added the Renderer label May 25, 2024

sobkas force-pushed the glew_to_epoxy branch from d3af903 to 562958a Compare May 26, 2024 18:12

github-actions bot added the Infrastructure label May 26, 2024

OpenGL: Replace glew with libepoxy

a1c7df0

sobkas force-pushed the glew_to_epoxy branch from 562958a to a1c7df0 Compare May 26, 2024 18:29

Xottab-DUTY requested changes May 26, 2024

View reviewed changes

Xottab-DUTY added Portability OpenGL External (3rd party) This issue is related to external component used by our project. and removed Infrastructure labels May 29, 2024

Xottab-DUTY closed this Jun 1, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Glew to epoxy #1681

Glew to epoxy #1681

sobkas commented May 25, 2024

AMS21 commented May 26, 2024

sobkas commented May 26, 2024

Xottab-DUTY commented May 26, 2024

sobkas commented May 26, 2024

Xottab-DUTY left a comment

Xottab-DUTY May 26, 2024

Xottab-DUTY May 26, 2024

Xottab-DUTY May 26, 2024

Xottab-DUTY May 26, 2024

Xottab-DUTY May 26, 2024

Xottab-DUTY May 26, 2024

Xottab-DUTY May 26, 2024

Xottab-DUTY May 26, 2024

Xottab-DUTY May 26, 2024 •

edited

Loading

Xottab-DUTY May 26, 2024 •

edited

Loading

AMS21 commented May 27, 2024

Xottab-DUTY commented May 28, 2024 •

edited

Loading

Xottab-DUTY commented May 29, 2024 •

edited

Loading

Xottab-DUTY commented Jun 1, 2024

		mode.format = SDL_PIXELFORMAT_RGBA8888;
		// Apply the pixel format to the device context

		@@ -103,6 +104,7 @@ void CHW::CreateDevice(SDL_Window* hWnd)
		const Uint32 flags = SDL_WINDOW_BORDERLESS \| SDL_WINDOW_HIDDEN \| SDL_WINDOW_OPENGL;

		m_helper_window = SDL_CreateWindow("OpenXRay OpenGL helper window", 0, 0, 1, 1, flags);

Glew to epoxy #1681

Glew to epoxy #1681

Conversation

sobkas commented May 25, 2024

AMS21 commented May 26, 2024

sobkas commented May 26, 2024

Xottab-DUTY commented May 26, 2024

sobkas commented May 26, 2024

Xottab-DUTY left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Xottab-DUTY May 26, 2024 • edited Loading

Choose a reason for hiding this comment

Xottab-DUTY May 26, 2024 • edited Loading

Choose a reason for hiding this comment

AMS21 commented May 27, 2024

Xottab-DUTY commented May 28, 2024 • edited Loading

Xottab-DUTY commented May 29, 2024 • edited Loading

Xottab-DUTY commented Jun 1, 2024

Xottab-DUTY May 26, 2024 •

edited

Loading

Xottab-DUTY May 26, 2024 •

edited

Loading

Xottab-DUTY commented May 28, 2024 •

edited

Loading

Xottab-DUTY commented May 29, 2024 •

edited

Loading