Re: [osg-users] Removing objects with shared GL state from scene graph

[Chris Djali]

Hi,

Did my last message disappear because of the forum issues? It's been a while and there's no reply yet...

Cheers,
Chris

------------------
Read this topic online here:
http://forum.openscenegraph.org/viewtopic.php?p=76596#76596





_______________________________________________
osg-users mailing list
osg-...@lists.openscenegraph.org
http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org

[Robert Osfield]

Hi Chris,

On Mon, 19 Aug 2019 at 22:58, Chris Djali <kriz...@gmail.com> wrote:

Did my last message disappear because of the forum issues? It's been a while and there's no reply yet...

How should we know if what your last message was if it might have disappeared :-)

Robert.

 

[Chris Djali]

Here it is again:


AnyOldName3 wrote:
> Hi,
>
> Hopefully, after that two-week hiatus, we can get this sorted.
>
>
> > You can always subclass from Camera and override the destruct if you want to add this "fix" if it suits your usage case.
>
>
> I'm already going to have to include a separate fix for older OSG versions (and I have a good idea of how I'll implement it), as we're supposed to support as far back as 3.4.x, so if we're abandoning the idea of actually fixing the issue in OSG itself, I've already got a workaround figured out.
>
> I imagine, though, that until everyone who writes applications knows how to use Vulkan and all devices have Vulkan support, removing the footgun from OSG is still your preferred option.
>
>
> > The approach I have taken with the VSG is for the GPU objects to hold a ref_ptr<> to the VkInstance wrapper preventing it being deleted when the objects are still in use. It's thread safe, robust for a lots of different usage cases and straight forward. Alas OpenGL can't be managed like this, or at least I don't think you can keep a graphics context around after a window has been deleted. If I was to writing an OpenGL scene graph from scratch I'd probably try to resolve this topic in a more flexible way than the OSG which has evolve from narrower usage cases to more and more general usage cases as users have pushed and pulled it in various directions.
>
>
> That does sound like a much nicer way of doing things. The only thing similar I can think of with OpenGL is to have a sharing context with access to the same objects as the main window, and to only destroy this extra context once everything has been released, but then I'm still discovering surprising new OpenGL features that can do new things.

>
> Cheers,
> Chris


------------------
Read this topic online here:

http://forum.openscenegraph.org/viewtopic.php?p=76598#76598

[Robert Osfield]

Hi Chris,

I read that post but didn't reply as I couldn't see anything that required any feedback from myself. 

Robert.

[Chris Djali]

Hi,

Is the plan still to have OSG itself release GL objects in the object cache when the contexts they're associated with are destroyed, and if so, have you had any further thoughts about how this might be accomplished?

The problem is still that there's nothing in osgViewer that has the same lifetime as an osg::GraphicsContext, and osg isn't supposed to be aware of osgDB, so it has to be osgViewer that releases things, right?

Cheers,
Chris

------------------
Read this topic online here:

http://forum.openscenegraph.org/viewtopic.php?p=76601#76601

[Robert Osfield]

Hi Chris,

On Wed, 21 Aug 2019 at 20:18, Chris Djali <kriz...@gmail.com> wrote:

Is the plan still to have OSG itself release GL objects in the object cache when the contexts they're associated with are destroyed, and if so, have you had any further thoughts about how this might be accomplished?

The problem is still that there's nothing in osgViewer that has the same lifetime as an osg::GraphicsContext, and osg isn't supposed to be aware of osgDB, so it has to be osgViewer that releases things, right?

I have already added a osgDB::Registry::instance()->releaseGLObjects(state); to the osgViewer::Renader::releaseGLObjects():

void Renderer::releaseGLObjects(osg::State* state) const
{
    osgDB::Registry::instance()->releaseGLObjects(state);

    if (_sceneView[0].valid()) _sceneView[0]->releaseGLObjects(state);
    if (_sceneView[1].valid()) _sceneView[1]->releaseGLObjects(state);
}

This is in master and the 3.6 branch.  This should clear the ObjectCache.  It's why I added this.

I can't keep running around in circles on this.  I've put in a lot of effort to try and resolve what I can, but now I need to get on with other work.

Robert.

 

[Chris Djali]

Hi,

As we've discussed, that only occasionally helps. In the boring single-viewer, single-view case, it works, but no one would notice there was a problem there anyway as nothing would actually render incorrectly.

My application just uses the camera created for it by OSG when an osgViewer::View is created, and I think it's reasonable to expect OSG to clear up things it's created itself. I'm under the impression that you do, too.

I've been having a poke around, and it seems to me that one possible option would be to call releaseGLObjects on a camera being removed from an osg::GraphicsContext when it's the only one attached. In such a situation, we already call it on the camera's non-shared child nodes (which is all of them) and its rendering cache, so this only adds the renderer and callbacks as extra things being released. It's my belief that this could only cause unwanted releasing in the case where all the cameras were removed from a context and then new ones were added. I think such a situation is probably unlikely (but you'd know better) and not much would be different to how things are now, as any attached nodes are already being released. There's also an added bonus that the function can return early as there'd be no need to work out which child nodes may or may not be shared.

The diff that fixes my use case is

Code:

diff --git a/src/osg/GraphicsContext.cpp b/src/osg/GraphicsContext.cpp
index 1a35497d0..e6113eb9a 100644
--- a/src/osg/GraphicsContext.cpp
+++ b/src/osg/GraphicsContext.cpp
@@ -741,6 +741,12 @@ void GraphicsContext::addCamera(osg::Camera* camera)

void GraphicsContext::removeCamera(osg::Camera* camera)
{
+ if (_cameras.size() == 1 && camera == _cameras.front())
+ {
+ _cameras.clear();
+ camera->releaseGLObjects(_state.get());
+ return;
+ }
Cameras::iterator itr = std::find(_cameras.begin(), _cameras.end(), camera);
if (itr != _cameras.end())
{




How does this look to you?

Cheers,
Chris

------------------
Read this topic online here:

http://forum.openscenegraph.org/viewtopic.php?p=76605#76605

[Robert Osfield]

Hi Chris,

As far as I'm aware the issues you have are pretty unusual, in most usage cases the OSG looks to be doing the right thing, it's niche usage cases that are the issue.  I've tried to help plug the gaps with the usage cases you've come up with, but can't do anymore without concrete usage cases to study that illustrate in runnable code the problem you have, trying to parse explanations for this type of thing is fraught with problems - what might be clear in your head but you have a whole bunch of extra information relevant to your application that I don't have.  I have lots of different topics that need my attention so I have to jump between, each one I have to try to get up to speed and work out what is going on, having threads that meander across months of posts makes this whole process even harder to follow.

Concrete code cuts through it all, a succinct example that I can read work out what is going on, run it, see the problem first hand, then apply and fix and confirm that it fixes it.  It's the gold standard way to get things resolved.

On Sat, 24 Aug 2019 at 01:18, Chris Djali <kriz...@gmail.com> wrote:

The diff that fixes my use case is

>...

How does this look to you?

Looks like a hack that would be workaround for a specific usage case not a general solution.

Robert.

[Chris Djali]

Hi,

I'm trying to build a stripped-down version of the same behaviour as my application, but it's got extra issues because things that are handled by what I'm stripping out aren't being handled when they're gone.

What's the correct process for adding and removing views to and from an osgViewer::CompositeViewer while it's running? I was under the impression I just needed osgViewer::CompositeViewer::stopThreading and osgViewer::CompositeViewer::startThreading either side of it being added and removed, but that's not working (access violation) when doing it with an osgGA::GUIEventHandler attached to one of the views. There's pretty much nothing else at all in the demo application, just a text node added to all of the views.

Cheers,
Chris

------------------
Read this topic online here:

http://forum.openscenegraph.org/viewtopic.php?p=76610#76610

[Chris Djali]

Hi,

The reason I couldn't add or remove views during the event traversal was that it was invalidating iterators that iterated over the views. Instead, I'm adding an update operation to add or remove the viewer.

I now have a fairly minimal example that replicates my use case and bug:


Code:

#include <osg/AutoTransform>

#include <osgText/Text>

#include <osgGA/TrackballManipulator>

#include <osgViewer/ViewerEventHandlers>

#include <osgViewer/CompositeViewer>

class AddViewOperation : public osg::Operation
{
public:
AddViewOperation(osg::ref_ptr<osgViewer::View> view)
: osg::Operation("AddView", false)
, _view(view)
{}

void operator() (osg::Object * compositeViewer) override
{
OSG_NOTICE << "AddView operator()" << std::endl;
osgViewer::CompositeViewer * viewer = dynamic_cast<osgViewer::CompositeViewer *>(compositeViewer);
viewer->stopThreading();
viewer->addView(_view);
viewer->startThreading();
}

protected:
osg::ref_ptr<osgViewer::View> _view;
};

class RemoveViewOperation : public osg::Operation
{
public:
RemoveViewOperation(osg::ref_ptr<osgViewer::View> view)
: osg::Operation("RemoveView", false)
, _view(view)
{}

void operator() (osg::Object * compositeViewer) override
{
OSG_NOTICE << "RemoveView operator()" << std::endl;
osgViewer::CompositeViewer * viewer = dynamic_cast<osgViewer::CompositeViewer *>(compositeViewer);
viewer->stopThreading();
viewer->removeView(_view);
viewer->startThreading();
}

protected:
osg::ref_ptr<osgViewer::View> _view;
};

class ViewAdder : public osgGA::GUIEventHandler
{
public:
ViewAdder(osgViewer::CompositeViewer * viewer)
: _viewer(viewer)
, _view(nullptr)
{}

bool handle(const osgGA::GUIEventAdapter& ea, osgGA::GUIActionAdapter& aa)
{
if (ea.getEventType() == osgGA::GUIEventAdapter::KEYUP && (ea.getKey() == 'v' || ea.getKey() == 'V'))
{
if (_view)
{
OSG_NOTICE << "Existing view, remove it" << std::endl;
// parts of the scene get removed before the view gets destroyed.
// normally this is fine as things get handled by destructors.
// however, things that are still cached require the cache to be released
_view->setSceneData(nullptr);
// We need to remove the view after the event traversal is done to avoid invalidating iterators
_viewer->addUpdateOperation(new RemoveViewOperation(_view));
_view = nullptr;
}
else
{
OSG_NOTICE << "No existing view, create one" << std::endl;
_view = new osgViewer::View;
_view->setName("View two");

_view->setUpViewOnSingleScreen(1);


osg::ref_ptr<osgText::Text> text2 = new osgText::Text();
text2->setText("Here's some other text. It appears in the dynamically-added view. It ensures the default font gets used with a context that goes away, and that lives in the cache.");
osg::ref_ptr<osg::AutoTransform> autoTransform2 = new osg::AutoTransform();
autoTransform2->setAutoRotateMode(osg::AutoTransform::ROTATE_TO_SCREEN);
autoTransform2->addChild(text2);
osg::ref_ptr<osg::Node> scene2 = autoTransform2;

_view->setSceneData(scene2.get());
_view->setCameraManipulator(new osgGA::TrackballManipulator);

_viewer->addUpdateOperation(new AddViewOperation(_view));
}
return true;
}
return false;
}

protected:
// raw pointer because it's stack-allocated
osgViewer::CompositeViewer * _viewer;
osg::ref_ptr<osgViewer::View> _view;
};

int main(int argc, char **argv)
{

// use an ArgumentParser object to manage the program arguments.
osg::ArgumentParser arguments(&argc, argv);

osg::ref_ptr<osgText::Text> text = new osgText::Text();
text->setText("Here's some text. It doesn't have to be text, and in fact we don't need anything in the main view as it just exists to keep the application alive.");
osg::ref_ptr<osg::AutoTransform> autoTransform = new osg::AutoTransform();
autoTransform->setAutoRotateMode(osg::AutoTransform::ROTATE_TO_SCREEN);
autoTransform->addChild(text);
osg::ref_ptr<osg::Node> scene = autoTransform;

if (!scene)
{
OSG_NOTICE << argv[0] << ": requires filename argument." << std::endl;
return 1;
}

// construct the viewer.
osgViewer::CompositeViewer viewer(arguments);

// single-threaded to prove a threading bug isn't responsible for the symptoms
viewer.setThreadingModel(osgViewer::CompositeViewer::SingleThreaded);

// view one
{
osg::ref_ptr<osgViewer::View> view = new osgViewer::View;
view->setName("View one");
viewer.addView(view);

view->setUpViewOnSingleScreen(0);
view->setSceneData(scene.get());
view->setCameraManipulator(new osgGA::TrackballManipulator);

view->addEventHandler(new ViewAdder(&viewer));
}

// run the viewer's main frame loop
return viewer.run();
}





This started life as the Composite Viewer example, and I deleted chunks of it and then added the new behaviour I needed, so I don't think I've done anything invalid.

Upon starting the application, you'll see a view with some text on one monitor. This isn't important, but I believe I need a living viewer to be able to have an event handler.

When you press the V key, a second view with some different text will appear on the other monitor. Pressing V again will remove it, and then again will add it again, and so on and so forth.

The second appearance of the second view will have a white box instead of the text, though, as the object cache doesn't have its GL objects released when the view was destroyed, so OSG tries using the same object names, which are no longer valid with the new context. This happens because the text isn't attached to the scene graph any more when the context is destroyed, so the default font is only referenced by the object cache, and the camera and its renderer never have releaseGLObjects called.

The osg::GraphicsContext::removeCamera diff I posted earlier solves the issue.

Cheers,
Chris

------------------
Read this topic online here:

http://forum.openscenegraph.org/viewtopic.php?p=76611#76611

[Robert Osfield]

Hi Chris,

Thanks for the test program.  I've created a CMake file and got it compiling and running on my Linux system.  I tweaked the window creation so it uses a multiple windows on a single screen as that suits by desktop configuration better.  On pressing 'V' multiple times I get the white text and GL errors reported on the second time the extra view creation happens.

I've put debugging code into the Registry and Renderer to see if my addition to Renderer::releaseGLObjects() was working and found that when the view gets removed this isn't called.  However, if I press escape and the both view+windows get deleted then the Renderer::releaseGLObjects() gets called.  I don't know why this isn't working as intended yet.

I have other work to get on with today so I'll need to return to investigating this later, my guess is that we just need to tighten up the handling of the removeView.

Robert.

[Chris Djali]

Hi,

It's been a week, so I'm just checking this hasn't been forgotten.

Cheers,
Chris

------------------
Read this topic online here:

http://forum.openscenegraph.org/viewtopic.php?p=76638#76638

[Robert Osfield]

Hi Chris,

On Tue, 3 Sep 2019 at 18:08, Chris Djali <kriz...@gmail.com> wrote:

It's been a week, so I'm just checking this hasn't been forgotten.

Not forgotten, but I'm working flat out to get database paging functionality into the VSG right now so don't have time or spare brain capacity to go chasing other topics.  The time I had available to work on the was July, during my holiday, now an OSG work has to be squeezed in when I'm not work on intense stuff.

Robert.