nipy / PySurfer

Cortical neuroimaging visualization in Python
https://pysurfer.github.io/
BSD 3-Clause "New" or "Revised" License
239 stars 98 forks source link

Fix rendering of overlay on some graphic chipsets. #286

Closed wmvanvliet closed 4 years ago

wmvanvliet commented 4 years ago

Ok. More digging and potential solutions.

An undocumented workaround for various rendering bugs in some graphics drivers is to hack your local site-packages/vtkmodules/qt/__init__.py file and make this change:

# QVTKRWIBase, base class for QVTKRenderWindowInteractor,
# can be altered by the user to "QGLWidget" in case
# of rendering errors (e.g. depth check problems, readGLBuffer
# warnings...)
QVTKRWIBase = "QGLWidget"  # Changed from "QWidget" -> "QGLWidget"

Code relying on vtkmodules, such as Mayavi and pyvista, will check the QVTKRWIBase variable and try to do the right thing accordingly. Unfortunately, for some reason, it decided to do this:

if PyQtImpl == "PyQt5":
    if QVTKRWIBase == "QGLWidget":
        try:
            from PyQt5.QtWidgets import QOpenGLWidget as QGLWidget
        except:
            from PyQt5.QtOpenGL import QGLWidget

I don't know where the QOpenGLWidget/QGLWidget confusion comes from, but in my case, it needs to be QGLWidget, otherwise everything crashes. Removing this try/catch construction and just importing QGLWidget fixes this at my end.

This gets me up to this point:

fig2

Depth ordering bugs are fixed! And now I face the a similar rendering bug as @filip-halemba reported in #284. Sadly, turning on backface culling on the overlay surface had no effect for me. However, turning on force_opaque did work! See changes in this PR.

fig3

Perfection at last!

EDIT:

Closes #284

codecov[bot] commented 4 years ago

Codecov Report

Merging #286 into master will increase coverage by 0.17%. The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #286      +/-   ##
==========================================
+ Coverage   74.52%   74.69%   +0.17%     
==========================================
  Files           7        7              
  Lines        2575     2577       +2     
  Branches      512      513       +1     
==========================================
+ Hits         1919     1925       +6     
+ Misses        480      477       -3     
+ Partials      176      175       -1     
larsoner commented 4 years ago

I don't know where the QOpenGLWidget/QGLWidget confusion comes from, but in my case, it needs to be QGLWidget, otherwise everything crashes.

And so to fix the bug we need:

  1. This PR
  2. Be on VTK 8.2+
  3. Fix an upstream Mayavi bug (PR forthcoming?)

Is this correct?

larsoner commented 4 years ago

don't know where the QOpenGLWidget/QGLWidget confusion comes from

IIRC it's older versus newer PyQt5 versions

wmvanvliet commented 4 years ago

I think so!

larsoner commented 4 years ago

Okay let's go ahead and merge this. Can you tag me in the Mayavi PR when you make it? I'd like to follow

larsoner commented 4 years ago

Thanks for the awesome investigation @wmvanvliet !

wmvanvliet commented 3 years ago

A PR is not so simple. QGLWidget is deprecated, so we should be using QOpenGLWidget, but that is crashing and I'm out of my depth trying to debug this. I've raised the issue in the mayavi repo: https://github.com/enthought/mayavi/issues/969