Invalid VertexArrayState usage

[bylee20]

As far as I can tell, VertexArrayState wraps vertex array state such as vertex attribute and VAO.
Current implementation create VertexArrayState per contextID for shared context, but you should not share vertex state between different context instances even though they are shared.

From https://www.khronos.org/opengl/wiki/Vertex_Specification#Vertex_Array_Object:

Note: VAOs cannot be shared between OpenGL contexts.

So, you need unique VertexArrayState instance for each unique context instance, not for each unique contextID.

In my case, sharing VertexArrayState instances causes crash for multiple viewers with multiple shared contexts(one context per viewer) case when VAO is enabled.

[mp3butcher]

Interesting use case. Please give us a simple sample code reproducing the issue

[bylee20]

It's related with external GUI toolkit such as Qt.

When you integrate OSG with other GUI toolkit, you should follow its event loop, otherwise you will mess up the graphics stack (z-index or drawing order of windows). This restriction makes osg::CompositeViewer useless for multiple viewer usecase because it renders all subviews at once.

In this case, your only available option for multiple viewer is context per viewer.

I am not sure I am capable of making simple example, but I'll try it.

[openscenegraph]

This does sounds like a design flaw.

Generally I recommend against shared contexts as they can be so problematic - you can use multi-treading with them, and drivers can introduce their own problems as well. Qt aids a few more caveats to running things as it has a few dumb constraints to how it can be used.

So as a workaround I'd suggest just not sharing the contexts for now. A full solution would probably be to have a ContextID for shared graphics objects and a StateID for per osg::State/Window objects like VAO's. The OSG doens't have a concept of a StateID so it'd be a new feature that would need to be implemented.

[bylee20]

I am handling a scene with multiple buildings and I doubt it shows good performance without sharing for multiple viewers.

I have not received real data yet, so I cannot test right now, but duplication of graphics resources could hurt the performance a lot especially for large scene with many objects/vertices.

[openscenegraph]

On Thu, 19 Mar 2020 at 08:28, Lee, Byoung-young ***@***.***> wrote: I am handling a scene with multiple buildings and I doubt it shows good performance without sharing for multiple viewers. I have not received real data yet, so I cannot test right now, but duplication of graphics resources could hurt the performance a lot especially for large scene with many objects/vertices.

You can try introducing an osg::State StateID and use this in the VAO to enable sharing of context. I haven't tried it myself, but another approach might be to just have a single off screen context that you render to a series of FBO's then copy the results to each of the windows you want.

[bylee20]

@openscenegraph

You can try introducing an osg::State StateID and use this in the VAO to
enable sharing of context.

Do you mean I should try to fix it by implementing StateID?
Well, I am not very familiar with CMake-based project and it is quite a burden to write and debug CMake-based project.

@mp3butcher

Here's sample code to reproduce the problem:

#include <osg/Camera>
#include <osg/Geometry>
#include <osg/Program>
#include <osgViewer/CompositeViewer>

inline auto createGeometry() -> osg::ref_ptr<osg::Geometry>
{
    const char *vertexSource = R"(
        #version 130
        uniform mat4 osg_ModelViewProjectionMatrix;
        in vec4 osg_Vertex;
        in vec3 osg_Normal;
        out vec4 color;
        void main()
        {
            gl_Position = osg_ModelViewProjectionMatrix * osg_Vertex;
        };
    )";
    const char *fragmentSource = R"(
        #version 130
        out vec4 fragColor;
        void main()
        {
            fragColor = vec4(1.0, 1.0, 1.0, 1.0);
        };
)";
    osg::Program* program = new osg::Program;
    program->addShader(new osg::Shader{osg::Shader::VERTEX, vertexSource});
    program->addShader(new osg::Shader{osg::Shader::FRAGMENT, fragmentSource});

    osg::ref_ptr<osg::Geometry> g = new osg::Geometry();
    osg::ref_ptr<osg::Vec3Array> vertices = new osg::Vec3Array;
    vertices->push_back({-1, 0, -1});
    vertices->push_back({ 1, 0, -1});
    vertices->push_back({ 0, 0,  1});
    g->setVertexArray(vertices);
    g->addPrimitiveSet(new osg::DrawArrays(osg::PrimitiveSet::TRIANGLES, 0,
                                           static_cast<int>(vertices->size())));
    g->getOrCreateStateSet()->setAttribute(program);

    return g;
}

auto createContext(int x, int y, int w, int h,
                   osg::observer_ptr<osg::GraphicsContext> share)
    -> osg::ref_ptr<osg::GraphicsContext>
{
    osg::ref_ptr<osg::GraphicsContext::Traits> traits = new osg::GraphicsContext::Traits;
    traits->screenNum = 0;
    traits->x = 50 + x;
    traits->y = 50 + y;
    traits->width = w;
    traits->height = h;
    traits->windowDecoration = true;
    traits->doubleBuffer = true;
    traits->glContextVersion = "3.0";
    traits->sharedContext = share;
    auto context = osg::GraphicsContext::createGraphicsContext(traits.get());
    context->getState()->setUseModelViewAndProjectionUniforms(true);
    context->getState()->setUseVertexAttributeAliasing(true);
    return context;
}

auto main() -> int
{
    auto &settings = osg::DisplaySettings::instance();
    settings->setVertexBufferHint(osg::DisplaySettings::VERTEX_ARRAY_OBJECT);

    osgViewer::CompositeViewer viewer;

    const auto g = createGeometry();
    const auto c1 = createContext(0, 0, 300, 200, nullptr);
    const auto c2 = createContext(100, 100, 300, 200, c1);

    auto addView = [&] (auto context) {
        const double w = context->getTraits()->width;
        const double h = context->getTraits()->height;
        osg::ref_ptr<osg::Camera> camera = new osg::Camera;
        camera->setGraphicsContext(context);
        camera->setViewport(new osg::Viewport{0, 0, w, h});
        camera->setProjectionMatrixAsPerspective(30, 1, 1, 10000);

        osg::ref_ptr<osgViewer::View> view = new osgViewer::View;
        view->setCamera(camera.get());
        view->setSceneData(g);
        viewer.addView(view);
    };

    addView(c1);
    addView(c2);

    return viewer.run();
}

This code causes crash on Windows 10 64 bit with nvidia GTX 1660.

[mp3butcher]

perhaps gc->getNumContexts() could be used in VertexArrayState in order to index PCVAS[contextid*numContexts+subcontexid]
However it involve to store a new subcontextid in ContextData and increment it for shared gc
@robertosfield Could this be a proper solution?

[openscenegraph]

As the VAS functionality is used very intently during the draw traversal it's critical that we add no extra steps in getting the offset. Right now we have the state.getContextID() which is a pointer to an int, so we'd want to keep it no more complex than this. Which is why I suggested a getStateID(). In terms of actual meaning have one ContextID per Context makes sense, it's just being cheaply reused to handle shared contexts by them sharing the same ContextID. If we wanted to be really through we'd introduce something like a ContextDataID that is used for all GL objects that can be shared and ContextID for everything that is specific to a ContextID and can't be reused. However, the OSG has the current context for two decades, going changing something that touches many part of the OSG just for niche usage is really inappropriate, have my suggestion of the StateID(), it's not ideal by it's lightweight and able to make this specific usage case possible. I have lots of work on my plate right now so I'll not chasing up any of the possibilities. Others are welcome to jump in.

…

On Thu, 19 Mar 2020 at 13:20, Julien Valentin ***@***.***> wrote: perhaps gc->getNumContexts() could be used in VertexArrayState in order to index PCVAS[contextid*numContexts+subcontexid] However it involve to store a new subcontextid in ContextData and increment it for shared gc @robertosfield <https://github.com/robertosfield> Could this be a proper solution? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#927 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABUA2YMPGCWIYQN63PJREDRIILZHANCNFSM4LPBWP7A> .

[mp3butcher]

perhaps gc->getNumContexts() could be used in VertexArrayState in order to index PCVAS[contextid*numContexts+subcontexid]
However it involve to store a new subcontextid in ContextData and increment it for shared gc

@xylosper you still could try to manage VertexArrayState behavior in a createVASCallback on your drawables and set the stored one to NULL in custom drawimplementation
very hacky but perhaps could work