Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix rendering of overlay on some graphic chipsets. #286

Merged
merged 1 commit into from
Feb 11, 2020

Conversation

wmvanvliet
Copy link
Contributor

@wmvanvliet wmvanvliet commented Feb 11, 2020

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
Copy link

codecov bot commented Feb 11, 2020

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
Copy link
Contributor

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
Copy link
Contributor

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

IIRC it's older versus newer PyQt5 versions

@wmvanvliet
Copy link
Contributor Author

I think so!

@larsoner
Copy link
Contributor

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 larsoner merged commit dd9f3b5 into nipy:master Feb 11, 2020
@larsoner
Copy link
Contributor

Thanks for the awesome investigation @wmvanvliet !

@wmvanvliet
Copy link
Contributor Author

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: enthought/mayavi#969

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Problem with overlay rendering when visualizing the inverse problem
2 participants