[vtkusers] VTK incorrectly assumes in several places that there is a current rendering context

[Marc Ruiz Altisent]

Hi, I'm a developer working with VTK and I have found that in several
places VTK incorrectly assumes that there is current OpenGL rendering
context after calling vtkRenderWindow::MakeCurrent(), and this can lead to
a crash sometimes. Now follows the long explanation and afterwards the
proposed solutions.

We are developing and application that uses VTK and have found that it
crashes under certain circumstances. Debugging has shown that it crashes
because it's calling a null function pointer inside VTK code. Before every
crash, there have been errors in wglMakeCurrent(), although often
MakeCurrent() fails but there is no crash, just a window that is not
refreshed anymore.

In our application we have one or more viewers that are vtkRenderWindows.
In VTK, before each render and before other OpenGL operations the method
MakeCurrent() is called on the specific render window, which in turn calls
wglMakeCurrent(). Usually all goes well and wglMakeCurrent() succeeds, but
sometimes, with our configuration and after a known set of actions,
wglMakeCurrent() will fail where it previously succeeded during the same
run. This is detected by MakeCurrent() and it runs the vtkErrorMacro.
There's also the method IsCurrent() to check if the associated rendering
context is current, so it could be checked after a call to MakeCurrent() to
check from the outside if it has succeeded. But often this is not done, and
instead it is assumed that MakeCurrent() has succeeded.

In particular, in vtkRenderer::Render() a texture is loaded. At some point
in vtkOpenGLTexture::Load() the method MakeCurrent() is called:

// make the new context current before we mess with opengl
this->RenderWindow->MakeCurrent();

It is assumed that it has succeeded, no check. Later, a
vtkPixelBufferObject is created and its method SetContext() called. That
method in turn calls LoadRequiredExtensions(). This method first checks
whether the required extensions are supported:

bool gl15=mgr->ExtensionSupported("GL_VERSION_1_5")==1;

The first time this is executed it loads the extensions string and then
looks for the extension name there. On subsequent calls it just looks for
the extension name in the cached extensions string. Thus, if
wglMakeCurrent() succeeds the first time but fails later (so then there is
no current context) ExtensionSupported() can return true, as it happens in
our case. Later, the extension is loaded:

mgr->LoadExtension("GL_VERSION_1_5");

This method will succeed when there is a current rendering context but fail
and run vtkErrorMacro otherwise, but again it is always assumed that it has
succeeded. The big problem is that when it fails the function pointers in
vtkgl are null.

Later in vtkOpenGLTexture::Load() the method Upload2D() is called on the
pbo, which in turn calls Upload3D(), which at some point calls
CreateBuffer():

void vtkPixelBufferObject::CreateBuffer()
{
  this->Context->MakeCurrent();
  if (!this->Handle)
    {
    GLuint ioBuf;
    vtkgl::GenBuffers(1, &ioBuf);
    vtkGraphicErrorMacro(this->Context,"after GenBuffers");
    this->Handle = ioBuf;
    }
}

Again, MakeCurrent() is assumed to succeed. More importantly, later
vtkgl::GenBuffers() is called and in our failing scenario this results in a
call to a null function pointer and therefore a crash.

I hope you have followed the explanation. Now, I have some ideas to fix
this in VTK:

- One possibility would be to always check if the expected rendering
context is actually current by calling vtkRenderWindow::IsCurrent(), and
continue only if it returns true.
- Another option would be to use observers to react to the ErrorEvent fired
by the vtkErrorMacro in the required places, and then do whatever is
necessary to avoid problems.
- A third option, more specific to this particular problem, is to check
that vtkgl function pointers are not null before calling them.
- Another alternative, also specific, is to replace the call to
LoadExtension() in vtkPixelBufferObject::
LoadRequiredExtensions() by a call to LoadSupportedExtension() and check
the required value to update the result variable.

This could also be partially fixed in our application code by adding an
observer of ErrorEvent to vtkOutputWindow, which would throw an exception
that would be captured in our code, effectively aborting the execution of
the method that assumed MakeCurrent() to work, but this is cumbersome and
prone to memory leaks, so I think the proper fix should be in VTK.

Sorry for the very long mail, but it was hard to explain without all this
text ;)

PS: The problem has been discovered with VTK 5.6.1 but the cited code
fragments are the same in the master branch. The context fail happens on a
WinXP64 machine with two graphics cards and three monitors, maybe due to a
driver bug (the error string is "The operation completed successfully.").
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.vtk.org/pipermail/vtkusers/attachments/20130301/80097a79/attachment.htm>