Buffer.GetData() GLES fix

[k4G17eYWI]

There is no glMapBuffer() on GLES but it has the glMapBufferRange() as a more modern way to do the same.
So I replaced the function and it is working. I've tested it on Windows 11 desktop and on Android phone with GLES 3.2

[cpt-max]

Great, thanks for that. I'll check it out.
On a quick glance, most of the changed lines seem to be whitespace changes coming from an auto-formatter. It's best to not commit those, makes it difficult to see the actual changes.

[cpt-max]

glMapBufferRange was introduced with OpenGL 3. At this point OpenGL 2 is still supported by MonoGame. It's OK to use this function, but there must be a fallback to glMapBuffer when it's not available

[k4G17eYWI]

As I can see at the time your PR is merged they will drop this legacy opengl support, won't they?

[cpt-max]

I think the idea is to drop OpenGL completely and replace it with Vulkan.
It doesn't make sense for me to drop OpenGL 2 support over this. The fallback is so easy to implement.

[k4G17eYWI]

I've fixed indirect drawing on Android by adding this to PlatformInitialize():

            // Create VAO
            // Indirect drawing does not work without VAO on some platforms (Android for instance)
            // (for some reason this does not work for a scalar, works only for an array)
            var a = new uint[1];
            var dataPtr = GCHandle.Alloc(a, GCHandleType.Pinned);
            var vao = (UIntPtr) dataPtr.AddrOfPinnedObject();
            GL.GenVertexArrays(1, vao);
            GL.BindVertexArray(a[0]);

PR updated. Maybe we should dispose it on application exit? Not sure about this.

[cpt-max]

Thanks for looking into this. So you are creating one global VAO that's constantly bound. I'm a bit worried about inefficiency here. Every time the vertex attribute configuration changes this VAO will be updated. I think generally you want to have different VAO's for different vertex attributes. Using the same VAO with different vertex buffers sounds OK, I'm skeptical about using it with different vertex attributes.

[k4G17eYWI]

I don't think will be a performance degradation here because driver do the same as without this VAO, only store data in different place. But without any measurements it's only my speculation.

If you think we need approach with different VAO's, could you explain it a bit more? Does it match an existing architecture?

[k4G17eYWI]

Btw: what is the proper way to use these changes in my project? Should I build a nuget? When I use build on in my fork repo I get this:
MonoGame\MonoGame.Framework\Graphics\BufferResource.cs(48,13): error CS0103: The name 'PlatformConstruct' does not exist in the current context [...\MonoGame\MonoGame.Framework\MonoGame.Framework.Native.csproj]

It seems to be related with my changes but I don't understand exactly how to deal with it.

[cpt-max]

When a VAO is bound, every time you change vertex attributes or the vertex buffer, the VAO will update . The whole point of the VAO is to remember this state, so when you draw an object, you only need to set the corresponding VAO, instead of setting vertex attributes and buffers independently every time.

If the VAO usage was limited to Android and only to indirect draw calls, then a quick fix solution like this might be more acceptable, but as it is, it will probably add some overhead to every draw call. At least that's my expectation.

You don't need the MonoGame.Framework.Native project. Just remove it from the solution. Eventually those missing methods should probably be added. This was introduced with the latest merge of the MG develop branch into the compute branch.

EDIT: and no, you don't need to build a Nuget, you can use the MonoGame.Framework.Android project directly.

[k4G17eYWI]

If you have time please check out this discussion https://hero.handmade.network/forums/code-discussion/t/896-opengl_vbo_without_vao

They said that default zero VAO is still VAO. And it is deprecated:

Client vertex and index arrays - all vertex array attribute and element array
index pointers must refer to buffer objects. The default vertex array object
(the name zero) is also deprecated. Calling VertexAttribPointer when no
buffer object or no vertex array object is bound will generate an INVALID_-
OPERATION error, as will calling any array drawing command when no vertex
array object is bound.

[k4G17eYWI]

So what's the plan? I will remove unnecessary formatting from this PR but should we keep my solution or switch to another like you suggest. Will you merge it in the near future or it's better to use my own fork for my game?

And, honestly, I don't think monogame maintainers will merge you PR anytime. It is assigned to 5.0 version which suppose to drop OpenGL and use Vulkan. So they (or we) have to completely redo all the code. What do you think about it?

[cpt-max]

If you have time please check out this discussion https://hero.handmade.network/forums/code-discussion/t/896-opengl_vbo_without_vao

There's this statement in there

The easiest way to do things is to just create a VAO when your program starts and then just use VBOs as normal, without messing around with the VAO stuff. Last I heard it is actually faster to do things that way than using VAOs the "correct" way.

That's exactly what you did. If this is true then your solution is fine. I simply don't know if it's true. I'm not aware of any indirect draw problems on the desktop, so I'd prefer to keep things as they are on dektop, and only apply this fix for Android (either use #if ANDROID or #if GLES).

[cpt-max]

So what's the plan? I will remove unnecessary formatting from this PR but should we keep my solution or switch to another like you suggest. Will you merge it in the near future or it's better to use my own fork for my game?

I'll merge it. The remaining TODO's are

• remove formatting changes
• check if MapBufferRange function exists, if not use MapBuffer function
• limit VAO fix to ANDROID or GLES, or come up with more solid proof that it won't have a negative impact on performance (so far it's just a statement from some guy on the internet)
• move MapBufferRange, GenVertexArrays, BindVertexArray from LoadEntryPoints() to LoadExtensions(). Potentially unavailable functions are loaded in LoadExtensions

And, honestly, I don't think monogame maintainers will merge you PR anytime.

Neither do I. I don't understand how that's relevant for the discussion here. Is it about whether to implement the fallback or not? The options are:

• put in an if statement and stay compatible with current MG
• save the if statement and loose compatibility

That's an obvious decision for me. If there was some big gain from dropping compatibility, we can talk.

It is assigned to 5.0 version which suppose to drop OpenGL and use Vulkan. So they (or we) have to completely redo all the code. What do you think about it?

When MG switches to Vulkan there's definitely some work to do to get the compute branch updated. I don't know yet if I have the time to make that happen. That's a decision for another day.

[cpt-max]

I just pushed the missing functions for the native project. It should compile now.

[cpt-max]

@k4G17eYWI are you going to update your PR?

[k4G17eYWI]

I'll do it within a few days.

[orosbogdan]

Is this related to the build failing for the effect in the android sample ?

[cpt-max]

No, this is the issue you are facing here: cpt-max/Docs#3