Removed GLES version check for glBufferStorage, allowing for any device that supports it.

[dankcushions]

Tested on RPI4 - please see profile comparison of Goldeneye 64:
https://docs.google.com/spreadsheets/d/1kQ15ra9rFnzVx6Plukck8cEPczf446eRlf1rCKm21FI/edit?usp=sharing

When glBufferStorage is permitted calls to glDrawArraysUnbuffered and glDrawElementsUnbuffered (highlighted in yellow in first 'before' tab) become calls to glDrawArrays, glDrawRangeElementsBaseVertex and glBufferStorage (highlighted in yellow in the second 'after' tab). The total duration of said calls within each profiling interval (ends highlighted in red) are reduced in the 'after' scenario:

1.009887 (before) vs 0.7056322 (after)
0.739989 vs 0.565734

Please verify that I am comparing the right calls! This is based on conversations starting here: #2329 (comment) - tagging @fzurita, @loganmc10

I have only checked the intro of Goldeneye as it's a game where there's visible slowdown for the pi4. Please let me know if you want me to profile anything else. There was some concern that bufferStorage could cause slowdown, but I searched through the history and that line was introduced here: a625225, and there's some related comments, but nothing that specifically indicates this will cause slowdown in devices that support it.

Thanks :)

[loganmc10]

I believe that check is there because glDrawRangeElementsBaseVertex is only supported on GLES3.2+, the raspberry Pi probably supports GL_EXT_draw_elements_base_vertex which is an extension that enables it.

So the proper thing to do would be to keep the GLES3.2 check, but add an OR for GL_EXT_draw_elements_base_vertex

[dankcushions]

the pi4 supports bufferStorage directly as far as i can tell: https://gitlab.freedesktop.org/mesa/mesa/-/blob/master/docs/features.txt#L397

doesn't && m_glInfo.bufferStorage check for existence of this extension directly? if so, a gles3.2 device would pass this check? maybe i'm misunderstanding the check!

[loganmc10]

We are talking about 2 different extensions, GL_EXT_draw_elements_base_vertex isn't the same as the buffer storage extension, though they are used together.

I'm saying a device could support glBufferStorage, but not support glDrawRangeElementsBaseVertex, in which case we can't use glBufferStorage

[dankcushions]

ah I see, so i guess we could add it to GLInfo in https://github.com/gonetz/GLideN64/blob/master/src/Graphics/OpenGLContext/opengl_GLInfo.cpp like:

	drawElementsBaseVertex = (!isGLESX && (numericVersion >= 32)) || Utils::isExtensionSupported(*this, "
`GL_ARB_draw_elements_base_vertex`") ||
			Utils::isExtensionSupported(*this, "GL_EXT_draw_elements_base_vertex");

and then change the check to

if ((m_glInfo.isGLESX && (m_glInfo.bufferStorage && m_glInfo.drawElementsBaseVertex)) || !m_glInfo.isGLESX)

?

[Jj0YzL5nvJ]

The problem is that currently it's immediately assumed that if "bufferStorage" is available, then the GL/GLES version is X or higher. But such logic is totally wrong for Mesa drivers in general. #2048

If said behavior is not changed, this change will create more serious issues than those that are supposed to be fixed.

[dankcushions]

Yes I guess the presumption of the availability of GL_ARB_draw_elements_base_vertex is an example of this.

However isn't what GLInfo.cpp is supposed to address, by checking for the presence of individual extensions for certain things? Perhaps it should do that for ALL extensions, ratehr than relying on version numbers, but that's a large change!

bufferStorage doesn't seem to be used much in the code so I am hoping that it's not such a huge change for just this one. If I add the check for drawElementsBaseVertex is it not enough?

[gonetz]

@loganmc10 , @Jj0YzL5nvJ please review that fix and if it is wrong, please suggest how it can be corrected.

[fzurita]

I think if he does what @loganmc10 says, it should be ok.

[dankcushions]

I have put the additional check in and everything seems to work for me, but only have RPI4 to check with.

[loganmc10]

Given that the minimum OpenGL version for the plugin is 3.3, you don't really need:

(!isGLESX && (numericVersion >= 32)) || Utils::isExtensionSupported(*this, "GL_ARB_draw_elements_base_vertex")

It could just be:
drawElementsBaseVertex = !isGLESX || Utils::isExtensionSupported(*this, "GL_EXT_draw_elements_base_vertex");

[loganmc10]

Looks good to me now!

[Jj0YzL5nvJ]

The specific case I referenced only affected ATI/AMD Mesa drivers limited to OpenGL 3.3. Testing on such hardware is no longer possible ...at least from my side.
This change looks harmless to PC users.

[gonetz]

The condition looks crazy now. Should not it be
if (!m_glInfo.isGLESX || (m_glInfo.bufferStorage && m_glInfo.drawElementsBaseVertex))
?

[gonetz]

I corrected the condition and merged this PR: cb76d2f

[fzurita]

Well, it's never a free lunch. This broke power VR devices that say they support the extension.