Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add custom get proc support #343

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Jupeyy
Copy link

@Jupeyy Jupeyy commented Jan 15, 2022

Motvation:
OpenGL isn't really well supported under windows (e.g. borderless fullscreen causes weird black flushes, or doesn't really load the taskbar correctly and stuff like that)

So in order to let the user decide if they need borderless fullscreen over "real" fullscreen, we want to support glondirect12 mesa drivers.
They are, as of jan. 2022, slower than the native OpenGL drivers. So we want to have support for both.

Since GCC for windows doesn't support tricks like lazy dll loading, the easiest is to not being required to link OpenGL and add custom handlers for that.

SDL2 comes with SDL_GL_LoadLibrary which makes it easy to select a different opengl driver.

Minimal code example:
e.g. by having mesa's opengl32.dll in a directory called "mesa" and all other required libs(for mesa opengl driver) where the working directory is(the .exe)

extern "C" void *GLEW_customGetProcAddress(const GLchar *name)
{
	return SDL_GL_GetProcAddress(name);
}

static void LoadOpenGLLibrary()
{
	const auto *pOpenGLLibName = SDL_getenv("SDL_OPENGL_LIBRARY");
	bool HasCustomOpenGLLib = pOpenGLLibName != nullptr;
	if(HasCustomOpenGLLib)
	{
		// message("sdl", "loading custom opengl32.dll: %s", pOpenGLLibName);
	}
	if(SDL_GL_LoadLibrary(nullptr) != 0)
	{
		// message("sdl", "loading opengl32.dll failed, please remove any start parameter. (error %s)", SDL_GetError());
	}
}

[...] somewhere in the backend
	SDL_InitSubSystem(SDL_INIT_VIDEO);
	LoadOpenGLLibrary();
        [...]
        glewContextInit(); // use context init, NOT glewInit

[...] somewhere at program start
      // optinal
      SDL_setenv("SDL_OPENGL_LIBRARY", "mesa/opengl32.dll", 1);
      SDL_setenv("GALLIUM_DRIVER", "d3d12", 1);

Important:
This moves GL 1.1 functions into GLEWs loading mecanism, even on windows. I dunno if this can potentially break something, or what the exact reason was, this wasnt done before.

This probably only works with glewContextInit, because there were code paths that used wglGetCurrentDC. Maybe this can be improved for easier usage

Currently it requires GLEW_STATIC for the custom symbol

Here is the patch on DDNet, where we want to use it
(ddnet/ddnet#4600)

@nigels-com
Copy link
Owner

There is a build failure in GLEW_STATIC mode:

cc -DGLEW_NO_GLU -DGLEW_STATIC -O2 -Wall -W -Wshadow -pedantic -Iinclude -fPIC -Wcast-qual -ansi -pedantic -fno-stack-protector  -o tmp/linux/default/static/glew.o -c src/glew.c
--
967 | src/glew.c:18868:38: warning: redefinition of typedef ‘PFNGLGETSTRINGPROC’ [-Wpedantic]
968 | 18868 \| typedef const GLubyte* (GLAPIENTRY * PFNGLGETSTRINGPROC) (GLenum name);
969 | \|                                      ^~~~~~~~~~~~~~~~~~
970 | In file included from src/glew.c:34:
971 | include/GL/glew.h:1006:39: note: previous declaration of ‘PFNGLGETSTRINGPROC’ was here
972 | 1006 \| typedef const GLubyte * (GLAPIENTRY * PFNGLGETSTRINGPROC) (GLenum name);
973 | \|                                       ^~~~~~~~~~~~~~~~~~
974 | src/glew.c:18869:28: warning: redefinition of typedef ‘PFNGLGETINTEGERVPROC’ [-Wpedantic]
975 | 18869 \| typedef void (GLAPIENTRY * PFNGLGETINTEGERVPROC) (GLenum pname, GLint *params);
976 | \|                            ^~~~~~~~~~~~~~~~~~~~
977 | In file included from src/glew.c:34:
978 | include/GL/glew.h:993:28: note: previous declaration of ‘PFNGLGETINTEGERVPROC’ was here
979 | 993 \| typedef void (GLAPIENTRY * PFNGLGETINTEGERVPROC) (GLenum pname, GLint *params);
980 | \|                            ^~~~~~~~~~~~~~~~~~~~
981 | ar rv lib/libGLEW.a tmp/linux/default/static/glew.o
982 | ar: creating lib/libGLEW.a
983 | a - tmp/linux/default/static/glew.o
984 | strip -x lib/libGLEW.a
985 | cc -DGLEW_NO_GLU -O2 -Wall -W -Wshadow -pedantic -Iinclude -fPIC -Wcast-qual -ansi -pedantic -fno-stack-protector  -o tmp/linux/default/shared/glewinfo.o -c src/glewinfo.c
986 | src/glewinfo.c:131:13: error: redefinition of ‘_glewInfo_GL_VERSION_1_1’
987 | 131 \| static void _glewInfo_GL_VERSION_1_1 (void)
988 | \|             ^~~~~~~~~~~~~~~~~~~~~~~~
989 | src/glewinfo.c:122:13: note: previous definition of ‘_glewInfo_GL_VERSION_1_1’ was here
990 | 122 \| static void _glewInfo_GL_VERSION_1_1 (void)
991 | \|             ^~~~~~~~~~~~~~~~~~~~~~~~
992 | src/glewinfo.c:122:13: warning: ‘_glewInfo_GL_VERSION_1_1’ defined but not used [-Wunused-function]
993 | make: *** [Makefile:195: tmp/linux/default/shared/glewinfo.o] Error 1
994 |  
995 | [Container] 2022/01/15 10:41:28 Command did not exit successfully make exit status 2
996 | [Container] 2022/01/15 10:41:28 Phase complete: BUILD State: FAILED
997 | [Container] 2022/01/15 10:41:28 Phase context status code: COMMAND_EXECUTION_ERROR Message: Error while executing command: make. Reason: exit status 2
998 | [Container] 2022/01/15 10:41:28 Entering phase POST_BUILD

@Jupeyy Jupeyy force-pushed the pr_custom_get_proc branch from 0268962 to 414966c Compare January 15, 2022 13:08
@nigels-com
Copy link
Owner

I can understand the motivation and practicality of this proposed change.
There is some history here of MX mode which was unsupportable for core contexts for some reason I don't recall too clearly.

I'm not decided if GLEW can reasonably accommodate this sort of change, but I will give it some consideration and engagement.
As a lone maintainer I am initially a bit daunted with how to maintain, test and support this additional mode of operation.
As a cross-platform library I would also want to be confident about the viability on Linux and Apple.

The additional wglew complexity is also a bit daunting from a maintainer perspective.

#if defined(GLEW_REGAL)
#if defined(GLEW_CUSTOM_GET_PROC)
# ifndef GLEW_STATIC
# error GLEW_CUSTOM_GET_PROC currently requires GLEW_STATIC and defining the symbol GLEW_customGetProcAddress
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps glewGetProcAddress just ought to be used as-is, if already defined to something.
(A thought, not a firm opinion)

Copy link
Author

@Jupeyy Jupeyy Jan 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah sounds reasonable (or did you mean use glewGetProcAddress as macro directly) :D

@Jupeyy Jupeyy force-pushed the pr_custom_get_proc branch from 414966c to 9b9e668 Compare January 15, 2022 13:33
@Jupeyy Jupeyy force-pushed the pr_custom_get_proc branch from 9b9e668 to 3ee0ae8 Compare January 17, 2022 08:58
@Jupeyy
Copy link
Author

Jupeyy commented Jan 17, 2022

This probably only works with glewContextInit, because there were code paths that used wglGetCurrentDC. Maybe this can be improved for easier usage

I load wglGetCurrentDC dynamically now too to minimize the ifdefs and allow glewInit
Also I tested it on windows XP to make sure it doesn't break any legacy things, but it seems to work

@Jupeyy
Copy link
Author

Jupeyy commented Jan 17, 2022

As a cross-platform library I would also want to be confident about the viability on Linux and Apple.

I'd say it is defenitely less useful, as atleast Linux uses mesa anyway and mesa supports MESA_LOADER_DRIVER_OVERRIDE environment flag to overload the GL driver(or atleast a vendor neutral dispatcher that can be forced to load smth different probably). But theoretically I'd say it could also work there if someone wants to write a custom opengl wrapper like mesa does it for windows

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants