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

QPainter::begin: Paint device returned engine == 0, type: 1 #196

Open
adam-grant-hendry opened this issue Jul 10, 2022 Discussed in #194 · 21 comments · May be fixed by #200
Open

QPainter::begin: Paint device returned engine == 0, type: 1 #196

adam-grant-hendry opened this issue Jul 10, 2022 Discussed in #194 · 21 comments · May be fixed by #200

Comments

@adam-grant-hendry
Copy link
Contributor

Discussed in #194

Originally posted by adam-grant-hendry July 8, 2022
When using QtInteractor (Windows 10 with PySide6)

from pyvistaqt import MainWindow, QtInteractor
from qtpy import QtWidgets

class Window(MainWindow):
    def __init__(self):
        super().__init__()
        self.setWindowTitle('Example')

        self.layout_ = QtWidgets.QVBoxLayout()

        self.container = QtWidgets.QFrame()
        self.container.setLayout(self.layout_)
        self.setCentralWidget(self.container)

        self.plotter = QtInteractor(parent=self.container)
        self.layout_.addWidget(self.plotter)
        self.plotter.show()

        self.signal_close.connect(self.plotter.close)


app = QtWidgets.QApplication([])
window = Window()
window.show()
app.exec_()

The error:

QPainter::begin: Paint device returned engine == 0, type: 1

appears every time the screen is updated (even if plotter.show() is placed after window.show().

From the PySide6 QPaintEngine docs, the error is saying Qt thinks I am on X11:

  • QPaintEngine.X11 == 0
  • QPainEngine.Windows == 1

After debugging in VSCode with the following launch.json so the debugger goes into packages:

{
    "configurations": [
        {
            "name": "Python: Current File",
            "type": "python",
            "request": "launch",
            "program": "${file}",
            "console": "integratedTerminal",
            "justMyCode": false
        }
    ],
}

I found the error happens when rwi.py::QVTKRenderWindowInteractor.paintEngine() is called in response to a Paint event:

def paintEngine(self):
    return None

I also noticed pyvistaqt.rwi::QVTKRenderWindowInteractor.__init__() sets widget attribute

self.setAttribute(WidgetAttribute.WA_PaintOnScreen)

From the PySide6 docs, this flag is only supported on X11 and requires returning NULL (None) from paintEngine:

image

Hence, an alternative is needed for non-X11 systems (Windows, Mac) running PySide for OpenGL to be possible.

@adam-grant-hendry
Copy link
Contributor Author

This same issue first appeared in the mayavi issue Problem when using QGLWidget #969.

@adam-grant-hendry
Copy link
Contributor Author

adam-grant-hendry commented Jul 10, 2022

@adeak @tkoyama010 @akaszynski @larsoner Do you have any idea on how to approach this one? FYI, this might be a great one to debug at SciPy this week in a sprint.

@adam-grant-hendry
Copy link
Contributor Author

adam-grant-hendry commented Jul 10, 2022

@banesullivan @GuillaumeFavelier Also shamelessly pinging you for your help with the above issue as well 😆

@adam-grant-hendry
Copy link
Contributor Author

Linking as well the VTK issue I posted for this: QPainter::begin: Paint device returned engine == 0, type: 1 #18606

@adam-grant-hendry
Copy link
Contributor Author

@akaszynski @larsoner @adeak @tkoyama010 @banesullivan @GuillaumeFavelier Copying here as this affects us as well:

An important note on differences between QGLWidget and QOpenGLWidget:

The key takeaway is this:

While the API is very similar, there is an important difference between the two: QOpenGLWidget always renders offscreen, using framebuffer objects. QGLWidget on the other hand uses a native window and surface. The latter causes issues when using it in complex user interfaces since, depending on the platform, such native child widgets may have various limitations, regarding stacking orders for example. QOpenGLWidget avoids this by not creating a separate native window.

@wmvanvliet I suspect this is why switching back to QGLWidget worked for you. The QVTKRenderWindowInteractor appears designed around this old architecture.

@prabhuramachandran @larsoner After investigating, it looks like QVTKRenderWindowInteractor will require a rewrite to support the newer QOpenGLWidget. pyvistaqt has already pulled in the vtk upstream changes, and these do not solve the problem (at least on my Windows 10 machine with an NVIDIA gpu).

Since the QGL classes are obsolete or near deprecation and the vtk folks have previously explained they don't have time to maintain QVTKRenderWindowInteractor, I think it will become more important for mayavi, pyvista, and other packages using Qt to work together towards a solution.

@adam-grant-hendry
Copy link
Contributor Author

@akaszynski @larsoner @adeak @tkoyama010 @banesullivan @GuillaumeFavelier See Problem when using QGLWidget #969 for more details (my troubleshooting steps), but in summary:

If I add

window_ = QtGui.QWindow.fromWinId(self.plotter.winId())
self.container = self.createWindowContainer(window_, self)

to my Window.__init__() method, this resolves the errors:

QWindowsGLContext::makeCurrent: SetPixelFormat() failed (The pixel format is invalid.)
qt.qpa.backingstore: composeAndFlush: makeCurrent() failed

so the final MRE is now

Final MRE
from __future__ import annotations

from pyvistaqt import MainWindow, QtInteractor
from qtpy import QtGui, QtWidgets


class Window(MainWindow):
    def __init__(self, parent=None):
        MainWindow.__init__(self, parent)
        self.setWindowTitle('Example')

        self.layout().setContentsMargins(0, 0, 0, 0)
        self.layout().setSpacing(0)

        self.plotter = QtInteractor()
        self.plotter.add_axes()

        window_ = QtGui.QWindow.fromWinId(self.plotter.winId())
        self.container = self.createWindowContainer(window_, self)

        self.setCentralWidget(self.container)
        self.signal_close.connect(self.plotter.close)


format_ = QtGui.QSurfaceFormat()
format_.setMajorVersion(4)
format_.setMinorVersion(6)
format_.setDepthBufferSize(16)
QtGui.QSurfaceFormat.setDefaultFormat(format_)

app = QtWidgets.QApplication([])
window = Window()
window.show()
app.exec_()

I'm unsure if there is a better way. I'm open to suggestions though.

This and simply adding this check in QVTKRenderWindowInteractor.__init__()

if os.name == 'posix':
    self.setAttribute(WidgetAttribute.WA_PaintOnScreen)

solve 100% of the problems for me.

@tkoyama010
Copy link
Member

Sorry for the delay in responding. All the maintainers have been focused on preparing tutorials for Scipy2022 for the past few days. Have you solved the problem, and we are waiting for your PullRequest.

@larsoner
Copy link
Contributor

Sorry for the slow response, I was out for a bit. There have been a lot of messages about this, @adam-grant-hendry can you try to distill to what changes pyvistaqt needs to make? It seems like maybe it's just the if os.name == 'posix' conditional, is that right? Then maybe some documentation somewhere about how to specify formats in case of incompatibility...?

@adam-grant-hendry
Copy link
Contributor Author

@tkoyama010 @larsoner Yes, no problem. I purposefully left many comments since I can’t make the sprints.

mayavi will need to make these changes as well

The issue stems from two things:

  1. WA_PaintOnScreen is for X11 systems only.

  2. Qt introduced backwards incompatible changes when transitioning from QGLWidgets to QOpenGLWidgets. (Only QOpenGLWidget is available in PySide6 and PyQt6, and it is the recommended widget for new code in newer versions of Qt5.)

To fix the errors from this:

  • Check if system is posix before setting the attribute
  • Add the following lines after setting WindowInfo
window_ = QtGui.QWindow.fromWinId(WId)
self.container = self.createWindowContainer(window_, self)

@larsoner
Copy link
Contributor

@adam-grant-hendry can you add a PR for this to pyvistaqt? Then we can see if CIs are happy. If they are, then a PR to vtk and then mayavi as well would be good -- the former because we (pyvistaqt) really want our .py file to just be a copy of the upstream vearsion, and the latter because it would be good to fix Mayavi as well!

@adam-grant-hendry
Copy link
Contributor Author

@larsoner Yes, but I might not be able to by the sprints (I’ll try though). I’m AFK right now. Otherwise, I can on Monday.

One thing I would ask - if possible, at the sprints, please discuss with the CI’s any ramifications from using native widgets over alien widgets.

There are performance costs to using native widgets.

@larsoner
Copy link
Contributor

please discuss with the CI’s any ramifications

I assume you mean "maintainers" here... when I say "Then we can see if CIs are happy" I am anthopromorphizing the continuous integration / unit tests that we run, not talking about actual people :)

any ramifications from using native widgets over alien widgets

You mean the currently used QVTKRenderWindowInteractor versus the unused QVTKOpenGLWidget? It would be great to use the latter if/when we can, but my guess is that in needs to be compiled against a specific version of Qt. So you couldn't for example have one vtk wheel that supported Qt 5 and Qt6 for example, whereas currently you can use pyvistaqt with either of these because we subclass QWidget directly in python via QVTKRenderWindowInteractor. I'd love to be wrong on this point, though, and haven't looked to see if it's actually the case regarding QVTKOpenGLWidget's portability

@adam-grant-hendry
Copy link
Contributor Author

I assume you mean "maintainers" here

Ya, my bad

You mean the currently used QVTKRenderWindowInteractor versus the unused QVTKOpenGLWidget.

No, we use QOpenGLWidget. QVTKRenderWindowInteractor already subclasses QVTKOpenGLWidget if you're on PyQt6 or PySide6 (check out QVTKRWIBase in rwi.py). In these bindings, one has no other option (QGLWidget is unavailable to them), and in fact the QGL widgets are slowly becoming deprecated:

The legacy QtOpenGL module (classes prefixed with QGL) provides a widget called QGLWidget. QOpenGLWidget is intended to be a modern replacement for it. Therefore, especially in new applications, the general recommendation is to use QOpenGLWidget.

I mean consider all the important changes made when Qt transitioned from QGLWidget to QOpenGLWidget. These are laid out here. I really do recommend reading through all the notes I made on the mayavi issue board, but the two categories of changes are:

  • Differences between platforms and graphics cards (this seems important to your mayavi users)
  • Differences between offscreen and onscreen rendering (which it seems pyvista has gone to great lengths to implement)

I think it is mandatory for all of us updating and contributing to QVTKRenderWidgetInteractor to fully read the QOpenGLWidget docs.

@adam-grant-hendry
Copy link
Contributor Author

adam-grant-hendry commented Jul 14, 2022

I assume you mean "maintainers" here

Ya, my bad

You mean the currently used QVTKRenderWindowInteractor versus the unused QVTKOpenGLWidget.

No, we use QOpenGLWidget. QVTKRenderWindowInteractor already subclasses QVTKOpenGLWidget if you're on PyQt6 or PySide6 (check out QVTKRWIBase in rwi.py). In these bindings, one has no other option (QGLWidget is unavailable to them), and in fact the QGL widgets are slowly becoming deprecated:

The legacy QtOpenGL module (classes prefixed with QGL) provides a widget called QGLWidget. QOpenGLWidget is intended to be a modern replacement for it. Therefore, especially in new applications, the general recommendation is to use QOpenGLWidget.

I mean consider all the important changes made when Qt transitioned from QGLWidget to QOpenGLWidget. These are laid out here. I really do recommend reading through all the notes I made on the mayavi issue board, but the two categories of changes are:

  • Differences between platforms and graphics cards (this seems important to your mayavi users)
  • Differences between offscreen and onscreen rendering (which it seems pyvista has gone to great lengths to implement)

I think it is mandatory for all of us updating and contributing to QVTKRenderWidgetInteractor to fully read the QOpenGLWidget docs.

Sorry, I think you may have meant QVTKOpenGLWidget, when I was speaking of QOpenGLWidget. (The names are so similar)

My point in requesting the docs be mulled over was that I don’t know if there’s a better way to achieve what I did or if continuing to use native widgets is okay.

Trying to rewrite rwi.py sounds like a big task, Im not fully sure what’s involved, and I just want to be cognizant of the differences because we may be able to get performance benefits from a thoughtful rewrite.

At first glance, from reading the Qt docs, it seems there may be performance benefits from a refactor. However, I’m not seasoned enough with the remainder of pyvista and mayavi to know if it’s worth the effort.

@adam-grant-hendry
Copy link
Contributor Author

@larsoner Sorry for my last two comments. I didn’t read your response clearly enough).

I submitted a PR, but tests only passed for PyQt5 on my machine (Windows, Python 3.8). If you or other team members have time to get the other tests to pass, feel free to push directly to my branch.

I put createWindowContainer at the end of QVTKRenderWindowInteractor, but the interactive doesn’t resize to the window, so the user might have to place it manually in MainWindow (that works for me).

Also, I can’t declare self.container in QVTKRenderWindowInteractor because that causes __getattr__ to crash.

We could look at refactoring for performance improvements when using QOpenGLWidget (if that’s possible) in a separate PR. I think it might be worthwhile, but I’ll need some help and guidance from you and the other team members.

@adam-grant-hendry
Copy link
Contributor Author

Sorry for the delay in responding. All the maintainers have been focused on preparing tutorials for Scipy2022 for the past few days. Have you solved the problem, and we are waiting for your PullRequest.

@tkoyama010 No worries! I knew everyone was busy and I would be mostly AFK for the latter half of the week, so I purposefully left a lot of comments to help you and the team see my debugging process.

I submitted a PR, but many tests are still not passing. If you and other team members have time to take a look during the sprints, please feel free to push directly to my branch.

Thank you and @larsoner for all your help! You guys are amazing!

@adam-grant-hendry
Copy link
Contributor Author

adam-grant-hendry commented Jul 20, 2022

@larsoner @tkoyama010 Please note that my "fix" only works when QVTKRenderWidgetInteractor subclasses from QWidget. You must manually change QVTKRWIBase in vtmodules/qt/init.py to change the base class it derives from

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

and you can also set the python Qt binding by setting PyQtImpl to one of ["PySide6", "PyQt6", "PyQt5", "PySide2", "PyQt4", "PySide"]:

# PyQtImpl can be set by the user
PyQtImpl = None

QVTKRenderWindowInteractor.py then pulls in the modified information

# Check whether a specific PyQt implementation was chosen
try:
    import vtkmodules.qt
    PyQtImpl = vtkmodules.qt.PyQtImpl
except ImportError:
    pass

# Check whether a specific QVTKRenderWindowInteractor base
# class was chosen, can be set to "QGLWidget" in
# PyQt implementation version lower than Qt6,
# or "QOpenGLWidget" in Pyside6 and PyQt6
QVTKRWIBase = "QWidget"
try:
    import vtkmodules.qt
    QVTKRWIBase = vtkmodules.qt.QVTKRWIBase
except ImportError:
    pass

This situation isn't ideal because if the user changes or updates versions of VTK, manual changes in the __init__.py get overridden, so the changes would have to be remade. It would be much nicer if the user were to set this dynamically in their code:

import vtkmodules.qt
vtkmodules.qt.PyQtImpl = 'PySide6'
vtkmodules.qt.QVTKRWIBase = 'QOpenGLWidget'
from vtkmodules.qt.QVTKRenderWindowInteractor import QVTKRenderWindowInteractor

I think the above should be added to the pyvistaqt documentation, since most users won't know to manually modify vtkmodules/qt/__init__.py. However, see below (I think QWidget should be the default).

Important

From the What's New in Qt4, Qt4 split QWidget's WFlags into Qt::WindowFlags and Qt::WidgetAttribute flags and introduced WA_PaintOnScreen for backwards-compatibility. In Qt4, all widgets are double-buffered, so the WA_PaintOnScreen allowed setting a widget to use single buffering.

Even though the raster paint engine is the primary paint engine for QtWidget-based classes on Windows, X11, and macOS, I don't that there's necessarily a huge performance difference suing QWidget vs QOpenGLWidget.

A good reference w.r.t. using the QVTKOpenGLWidget variants is this discourse discussion:

How to decide between QVTKOpenGL{Native}Widget?

We could try to implement them using the developer tips from here:

VTK Python Wrappers Notes

@adam-grant-hendry
Copy link
Contributor Author

@akaszynski @tkoyama010 @larsoner I have submitted MR 9443 on VTK to attempt to solve this issue. Would you please kindly review the MR? (The VTK team has asked I bring in reviewers from mayavi and pyvista).

Thanks in advance!

@prabhuramachandran
Copy link

@adam-grant-hendry -- I am sorry I got COVID at SciPy the sprints were a washout for me and thereafter I had to travel a long way and am still a bit under the weather. What time I do have is taken up by the semester which is now in full swing. I will try and get to this either this weekend or the next. Thanks for your patience.

@adam-grant-hendry
Copy link
Contributor Author

@adam-grant-hendry -- I am sorry I got COVID at SciPy the sprints were a washout for me and thereafter I had to travel a long way and am still a bit under the weather. What time I do have is taken up by the semester which is now in full swing. I will try and get to this either this weekend or the next. Thanks for your patience.

No problem @prabhuramachandran . Thank you for your help!

@adam-grant-hendry
Copy link
Contributor Author

@banesullivan @GuillaumeFavelier If either of you can review MR 9443 on VTK as well, I would also appreciate your help!

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 a pull request may close this issue.

4 participants