openscenegraph / OpenSceneGraph

OpenSceneGraph git repository
http://www.openscenegraph.org
Other
3.23k stars 1.42k forks source link

osgText fails to use VBOs when it should #992

Open jpabst0 opened 3 years ago

jpabst0 commented 3 years ago

I have observed the issue using OpenSceneGraph 3.6.5. I have set up scene where I render a Text object after a Geometry object. The Text object is set up to use VBOs while the Geometry object is set up to not use VBOs. VAOs are not used.

Now the Text object fails to use it's VBO when dispatching the _coords / _texcoords array. The corresponding log messages are:

VertexArrayDispatch::enable_and_dispatch(108) TexCoordArrayDispatch::enable_and_dispatch(108) unit=0

If the Geometry object is changed to also use VBOs, suddenly the Text object uses its VBO properly:

VertexArrayDispatch::enable_and_dispatch(184, vbo=000001B8457C07C0) TexCoordArrayDispatch::enable_and_dispatch(184, vbo=000001B8457C07C0) unit=0

After some debugging I found out what's wrong. In the State object, the global VertexArrayState object is set current. It isn't changed throughout all drawing as VAOs are not used. When the Geometry object is drawn, it disables VBO dispatching in the global VertexArrayState object. Here is the corresponding code:

State& state = renderInfo.getState(); bool usingVertexBufferObjects = state.useVertexBufferObject(_supportsVertexBufferObjects && _useVertexBufferObjects); osg::VertexArrayState vas = state.getCurrentVertexArrayState(); vas->setVertexBufferObjectSupported(usingVertexBufferObjects);

The Text object also dispatches it's arrays using the global VertexArrayState object. It doesn't make a call to vas->setVertexBufferObjectSupported however. So the boolean flag that tells whether to use VBOs is left disabled from the Geometry object drawn before.

A possible fix might be adding the line

vas->setVertexBufferObjectSupported(usingVertexBufferObjects);

also to both, Text and Text3d, classes. I wonder however, if there arn't better (less viral) solutions.

openscenegraph commented 3 years ago

I have no clue why you are using the word "FBO" in this context.

FBO is my world means Frame Buffer Object. It's really not something that Text knows about or uses, or should use other than to render to if you are doing some high level render to texture effect.

jpabst0 commented 3 years ago

I'm sorry. I mixed up VBO and FBO in the text. Of course I meant VBO. I fixed title and description.

robertosfield commented 3 years ago

Could you try the OpenSeneGrah-3.6 branch head, it has a few fixes over 3.6.5. This I one made back in January might be relevant:

commit 605821e655d97b7ad5ab6098b3ec26e2f9ad2ff7 Author: Robert Osfield robert@openscenegraph.com Date: Tue Jan 7 11:12:18 2020 +0000

Implemented TextBase::compileGLObjects() with handling of VAO/VBOs to address bugs associated with VAO usage of Text.
jpabst0 commented 3 years ago

I got the same behaviour with OpenSceneGraph-3.6 head and the relevant code seems to be the same in the trunk too. Note that the suspicious call

vas->setVertexBufferObjectSupported(usingVertexBufferObjects);

is only made in the Geometry class. The flag is set in no other place.

robertosfield commented 3 years ago

I haven't look in depth yet, but adding the vas setting to Text.cpp and Text3D.cpp does look like a sensible thing to add. I don't have a usage case illustrates the problem you are seeing so if I made changes I won't know whether it has any effect or not. Could you apply the changes that you think that are required and fix the issue you are seeing and the generate PR for these changes. I can then review them with the knowledge that fix the problem rather than taking a punt. Thanks.

jpabst0 commented 3 years ago

See PR #1000.

Investigating this issue further, I noticed that adding the calls vas->setVertexBufferObjectSupported to the Text classes is not enough. All Drawables are affected. The only sane solution I found is to set up the vas object properly in the Drawable class and move the relevant code there.

I discovered another subtle issue concerning the VertexArrayState class. VertexArrayState stores current VBO and EBO bindings as members. According to to some OpenGL specification however, VBO binding is global and not part of the VAO state.

Now, there is a problem lurking in this code:

        else if (vbo != _currentVBO)
        {
            vbo->bindBuffer();
            _currentVBO = vbo;
        }

VertexArrayState does a check if if the object to bind changes. But _currentVBO only represents the last bound VBO by the Drawable (when VAOs are used) and not the global one. The code may miss to bind the object when it falsely assumes it has not changed.

I'll attach a program that provokes the bug. The program obviously sets up a scene to draw two small squares and one large square. The result of executing the program however is, that only one small square and two large squares get drawn.

As a workaround, I just unbind the VBO after using it - like it is done when VAOs are not enabled. But this is maybe a design flaw that needs to be addressed separately.

main.txt

openscenegraph commented 3 years ago

Thanks for the example. I've begun looking the PR and am using your provided example. I originally got a crash on a different Sorry for the slow reply, been busy with client work and pushing the VSG towards it's 1.0.

OSG branch (MultiView branched from 3.6) with this example, but when I did a clean build of OSG master I couldn't provoke a crash again. I'm about to test 3.6, takes rather long for the OSG to build so.... have to wait.

Do you see any crashes for any OSG branches? What platform are you testing with.

openscenegraph commented 3 years ago

I've rebuilt both 3.6 and MultiView branch debug and release and don't get the crash now. I do get the two big squares and one small one as you describe. If I comment out the calls to ->dirty calls then it works correctly.

openscenegraph commented 3 years ago

I've done some more experimenting, calling dirty on just one of the arrays leads to no errors, I only see failure when both arrays are dirtied. If I use dirtyGLObjects() to the r0,r1 and r2 everything works fine too - two small squares and one large one.

For sure it's a peculiar combination of operations that are provoking the error, it's great that we have an example of the problem happening as it'd be very hard to track down without it.

Above you say: "According to to some OpenGL specification however, VBO binding is global and not part of the VAO state."

Could you point me in the direction of the OpenGL spec references you are thinking of?

jpabst0 commented 3 years ago

The frame() and dirty() calls were meant to provoke the error. If I remember correctly, setting a new VertexArray to the Geometry had the same effect as dirty().

VBO binding is not part of the VAO state, I would expect every OpenGL spec to agree on that. I just looked it up in one I found.

https://www.khronos.org/registry/OpenGL/specs/gl/glspec33.core.pdf

Look under 6.2 "State Tables". Here ELEMENT_ARRAY_BUFFER_BINDING is mentioned.

This link might give a good explanation:

https://gamedev.stackexchange.com/questions/99236/what-state-is-stored-in-an-opengl-vertex-array-object-vao-and-how-do-i-use-the

I think OSG just got it wrong with storing the bound VBO in VertexArrayState. Class State would maybe have been the correct place. As OSG so far has the habit of unbinding VBOs and EBOs (when VAOs are not used) at the end of each call to Drawable::draw, it doesn't matter much. My fix just unbinds the VBO in case of using VAOs too. I regard it as open issue fixing the design here. I would then see no need to unbind VBOs and EBOs anymore.