-
-
Notifications
You must be signed in to change notification settings - Fork 248
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
gl2shim for core/gles2+ contexts #1446
Conversation
…gles context support (SLOW, still needs ffp for matrix operations)
…, wrap gles functions in gl2wrap when XASH_GLES enabled
…g dumb array object
…on on core context
…o/world array render work
…s generates GL Errors, but renders fine on mesa
…ngs, Draw QUADS with TRIFAN when possible
…fer works now on core contexts
…ent quering extensions on core profile
@@ -812,8 +812,13 @@ qboolean VID_CreateWindow( int width, int height, window_mode_t window_mode ) | |||
{ | |||
if( !glw_state.initialized ) | |||
{ | |||
if( !GL_CreateContext( )) | |||
return false; | |||
while( !GL_CreateContext( )) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't get it. There is already function VID_CreateWindowWithSafeGL
that iterates over safegl options.
Why there is a second loop?
@@ -1070,7 +1070,7 @@ static void GL_TextureImageRAW( gl_texture_t *tex, GLint side, GLint level, GLin | |||
} | |||
else if( tex->target == GL_TEXTURE_2D_MULTISAMPLE ) | |||
{ | |||
#if !defined( XASH_GLES ) && !defined( XASH_GL4ES ) | |||
#if !defined( XASH_GL_STATIC ) || (!defined( XASH_GLES ) && !defined( XASH_GL4ES )) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the point for checking for XASH_GL_STATIC here? It doesn't seem to change anything for MSAA.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you're updating macros, don't forget to update comments after #else and #endif
@@ -40,6 +40,9 @@ int VGL_ShimInit( void ); | |||
void VGL_ShimShutdown( void ); | |||
void VGL_ShimEndFrame( void ); | |||
#endif | |||
#if !defined(XASH_GL_STATIC) | |||
#include "gl2_shim/gl2_shim.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think #if !defined(XASH_GL_STATIC) really needed here. Including gl2shim header won't break anything.
|
||
static dllfunc_t shaderobjectsfuncs_gles[] = | ||
{ | ||
{ "glDeleteShader" , (void **)&pglDeleteObjectARB }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use GL_CALL macro here, it's very easy to pass wrong pointer here. You can create a second macro that will include ARB postfix.
ref/gl/gl_opengl.c
Outdated
|
||
static dllfunc_t vaofuncs[] = | ||
{ | ||
{ "glBindVertexArray" , (void **)&pglBindVertexArray }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use GL_CALL here.
ref/gl/gl_opengl.c
Outdated
// one or more functions are invalid, extension will be disabled | ||
GL_SetExtension( r_ext, false ); | ||
// HACK: fix ARB names | ||
char *str = Q_strstr(func->name, "ARB"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add spaces in parentheses.
ref/gl/gl_opengl.c
Outdated
GL_SetExtension( extid, true ); // required to be supported by wrapper | ||
if(!GL_CheckExtension( "multitexture", multitexturefuncs, "gl_arb_multitexture", GL_ARB_MULTITEXTURE, 1.0) && glConfig.wrapper == GLES_WRAPPER_NONE ) | ||
#ifndef XASH_GL_STATIC | ||
if( !GL_CheckExtension( "multitexture_es1", multitexturefuncs_es, "gl_arb_multitexture", GL_ARB_MULTITEXTURE, 1.0 ) ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you just use && here instead of nested ifs?
ref/gl/gl_opengl.c
Outdated
break; | ||
case GL_ARB_VERTEX_ARRAY_OBJECT_EXT: | ||
if(!GL_CheckExtension( "GL_OES_vertex_array_object", vaofuncs, "gl_vertex_array_object", extid, 3.0 )) | ||
!GL_CheckExtension( "GL_EXT_vertex_array_object", vaofuncs, "gl_vertex_array_object", extid, 3.0 ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused NOT operator here.
|
||
// get our various GL strings | ||
glConfig.vendor_string = (const char *)pglGetString( GL_VENDOR ); | ||
glConfig.renderer_string = (const char *)pglGetString( GL_RENDERER ); | ||
glConfig.version_string = (const char *)pglGetString( GL_VERSION ); | ||
glConfig.extensions_string = (const char *)pglGetString( GL_EXTENSIONS ); | ||
|
||
pglGetIntegerv(GL_MAJOR_VERSION, &major); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spaces in parentheses please.
|
||
pglGetIntegerv(GL_MAJOR_VERSION, &major); | ||
pglGetIntegerv(GL_MINOR_VERSION, &minor); | ||
if( !major && glConfig.version_string ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this even happen? O_o?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. GL_MAJOR_VERSION was intoduced in gl 3.0
#pragma once | ||
|
||
// max verts in a single frame | ||
#define GL2_MAX_VERTS 32768 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's pretty low for our frame.
net_graph for example, easily exceeds this
…ernary ops, remove singleline multiple assignments, use bitset macros
e0d66c1
to
8c46357
Compare
Because it is not called when GL_CreateContext failed. It is only called when it failed create window, but does not work in case window created, but context not. It just runs engine without valid context and it crashes on bad gl function calls.
14 октября 2023 г. 5:46:20 GMT+03:00, Alibek Omarov ***@***.***> пишет:
…
@a1batross commented on this pull request.
> @@ -812,8 +812,13 @@ qboolean VID_CreateWindow( int width, int
height, window_mode_t window_mode )
{
if( !glw_state.initialized )
{
- if( !GL_CreateContext( ))
- return false;
+ while( !GL_CreateContext( ))
I don't get it. There is already function `VID_CreateWindowWithSafeGL`
that iterates over safegl options.
Why there is a second loop?
--
Reply to this email directly or view it on GitHub:
#1446 (review)
You are receiving this because you authored the thread.
Message ID: ***@***.***>
|
I do not want to have declaration of missing functtion present. It wont be present if gl2shim part not compiled at all (it have macro in source file), so why include this header?
14 октября 2023 г. 7:42:07 GMT+03:00, Alibek Omarov ***@***.***> пишет:
…
@a1batross commented on this pull request.
> @@ -40,6 +40,9 @@ int VGL_ShimInit( void );
void VGL_ShimShutdown( void );
void VGL_ShimEndFrame( void );
#endif
+#if !defined(XASH_GL_STATIC)
+#include "gl2_shim/gl2_shim.h"
I don't think #if !defined(XASH_GL_STATIC) really needed here.
Including gl2shim header won't break anything.
--
Reply to this email directly or view it on GitHub:
#1446 (review)
You are receiving this because you authored the thread.
Message ID: ***@***.***>
|
gl changed function names, so we need macro including two names in this case, ARB does not call all cases
14 октября 2023 г. 7:45:03 GMT+03:00, Alibek Omarov ***@***.***> пишет:
…
@a1batross commented on this pull request.
> + { GL_CALL( glDisableVertexAttribArrayARB ) },
+ { GL_CALL( glBindAttribLocationARB ) },
+ { GL_CALL( glGetActiveAttribARB ) },
+ { GL_CALL( glGetAttribLocationARB ) },
+ { GL_CALL( glVertexAttrib2fARB ) },
+ { GL_CALL( glVertexAttrib2fvARB ) },
+// { GL_CALL( glVertexAttrib3fv ) },
+// { GL_CALL( glVertexAttrib4f ) },
+// { GL_CALL( glVertexAttrib4fv ) },
+// { GL_CALL( glVertexAttrib4ubv ) },
+{ NULL, NULL }
+};
+
+static dllfunc_t shaderobjectsfuncs_gles[] =
+{
+ { "glDeleteShader" , (void **)&pglDeleteObjectARB },
Let's use GL_CALL macro here, it's very easy to pass wrong pointer
here. You can create a second macro that will include ARB postfix.
--
Reply to this email directly or view it on GitHub:
#1446 (review)
You are receiving this because you authored the thread.
Message ID: ***@***.***>
|
&& inside macro? I not sure if it would be better
14 октября 2023 г. 7:46:57 GMT+03:00, Alibek Omarov ***@***.***> пишет:
…
@a1batross commented on this pull request.
> break;
case GL_ARB_MULTITEXTURE:
- GL_SetExtension( extid, true ); // required to be supported by
wrapper
+ if(!GL_CheckExtension( "multitexture", multitexturefuncs,
"gl_arb_multitexture", GL_ARB_MULTITEXTURE, 1.0) && glConfig.wrapper ==
GLES_WRAPPER_NONE )
+#ifndef XASH_GL_STATIC
+ if( !GL_CheckExtension( "multitexture_es1", multitexturefuncs_es,
"gl_arb_multitexture", GL_ARB_MULTITEXTURE, 1.0 ) )
Can you just use && here instead of nested ifs?
--
Reply to this email directly or view it on GitHub:
#1446 (review)
You are receiving this because you authored the thread.
Message ID: ***@***.***>
|
It seems to be some bug inside net_graph, it should not draw to many quads, but i'll add workaround for big GL_QUADS cases.
14 октября 2023 г. 8:13:09 GMT+03:00, Alibek Omarov ***@***.***> пишет:
…
@a1batross commented on this pull request.
> +
+This program is free software: you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation, either version 3 of the License, or
+(at your option) any later version.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+GNU General Public License for more details.
+*/
+
+#pragma once
+
+// max verts in a single frame
+#define GL2_MAX_VERTS 32768
It's pretty low for our frame.
net_graph for example, easily exceeds this
--
Reply to this email directly or view it on GitHub:
#1446 (review)
You are receiving this because you authored the thread.
Message ID: ***@***.***>
|
It's better to be moved out of the scope of this patchset then.
It will be detected at the link time. I don't see a problem here.
Yeah, we discussed this already. It's very unfortunate, so let's keep it as is then.
I think this code is not an example of a good code. It's not very obvious at which level |
This might be availiable in gles context without static gl. I just rechecked all XASH_GLES leftovers because many of them refered to XASH_GL_STATIC really, not to GLES context itself. But GLES without wrappers was not supported before. This commit first time add launching on pure gles not completely wrapped by nanogl/wes/gl4es, only having ffp implemented with gl2shim
14 октября 2023 г. 7:41:22 GMT+03:00, Alibek Omarov ***@***.***> пишет:
…
@a1batross commented on this pull request.
> @@ -1070,7 +1070,7 @@ static void GL_TextureImageRAW( gl_texture_t
*tex, GLint side, GLint level, GLin
}
else if( tex->target == GL_TEXTURE_2D_MULTISAMPLE )
{
-#if !defined( XASH_GLES ) && !defined( XASH_GL4ES )
+#if !defined( XASH_GL_STATIC ) || (!defined( XASH_GLES ) && !defined(
XASH_GL4ES ))
What's the point for checking for XASH_GL_STATIC here? It doesn't seem
to change anything for MSAA.
--
Reply to this email directly or view it on GitHub:
#1446 (review)
You are receiving this because you authored the thread.
Message ID: ***@***.***>
|
Alibek Omarov писал(а) 2023-10-15 02:41:
> Because it is not called when GL_CreateContext failed. It is only
> called when it failed create window, but does not work in case window
> created, but context not. It just runs engine without valid context
> and it crashes on bad gl function calls.
It's better to be moved out of the scope of this patchset then.
Message ID: ***@***.***>
I need to test if i did not break older GL versions and found that i
cannot even create context before 3.x with compatibility flag enabled
and safegl was not set automaticly. I searched other places where it
might be set, but it was unrelated: this function only calls
SDL_CreateWindow. Even when this safegl level enough to create window,
it might not enough to create context, so we need to increment it later
--=_f98e57ced455552daa412d819c95ca55
Content-Transfer-Encoding: quoted-printable
Content-Type: text/html; charset=UTF-8
<html><head><meta http-equiv=3D"Content-Type" content=3D"text/html; charset=
=3DUTF-8" /></head><body style=3D'font-size: 10pt; font-family: Verdana,Gen=
eva,sans-serif'>
<p id=3D"reply-intro">Alibek Omarov =D0=BF=D0=B8=D1=81=D0=B0=D0=BB(=D0=B0) =
2023-10-15 02:41:</p>
<blockquote type=3D"cite" style=3D"padding: 0 0.4em; border-left: #1010ff 2=
px solid; margin: 0">
<div id=3D"replybody1">
<p><br /></p>
<blockquote type=3D"cite" style=3D"padding: 0 0.4em; border-left: #1010ff 2=
px solid; margin: 0">
<p dir=3D"auto">Because it is not called when GL_CreateContext failed. It i=
s only called when it failed create window, but does not work in case windo=
w created, but context not. It just runs engine without valid context and i=
t crashes on bad gl function calls.</p>
</blockquote>
<p dir=3D"auto">It's better to be moved out of the scope of this patchset t=
hen.</p>
<p style=3D"font-size: small; -webkit-text-size-adjust: none; color: #666;"=
<span style=3D"color: transparent; font-size: 0; display: none; visibility=
: hidden; overflow: hidden; opacity: 0; width: 0px; height: 0px; max-width:=
0; max-height: 0; mso-hide: all;">Message ID: <span><FWGS/xash3d-fwgs/p=
ull/1446/c1763221270</span><span>@</span><span>github</span><span>.</span><=
span>com></span></span></p>
</div>
</blockquote>
<p>I need to test if i did not break older GL versions and found that i can=
not even create context before 3.x with compatibility flag enabled and safe=
gl was not set automaticly. I searched other places where it might be set, =
but it was unrelated: this function only calls SDL_CreateWindow. Even when =
this safegl level enough to create window, it might not enough to create co=
ntext, so we need to increment it later</p>
</body></html>
…--=_f98e57ced455552daa412d819c95ca55--
--=_4bdf773d7ffda6467a4f2456ec038f46
Content-Transfer-Encoding: base64
Content-Type: image/gif;
name=blocked.gif
Content-Disposition: attachment;
filename=blocked.gif;
size=118
R0lGODlhZAAyAIAAAPrOzgAAACH5BAAAAAAALAAAAABkADIAAAJNhI+py+0Po5y02ouz3rz7D4bi
SJbmiabqyrbuC8fyTNf2jef6zvf+DwwKh8Si8YhMKpfMpvMJjUqn1Kr1is1qt9yu9wsOi8fksvls
KwAAOw==
--=_4bdf773d7ffda6467a4f2456ec038f46--
|
GitHub didn't close it automatically AGAIN. Closing manually, merged. |
Limited FFP emulation with streaming buffers for gles/core contexts
Please check on nvidia/amd/intel proprietary drivers before merging, maybe need change some feature detection/blacklisting logic