search

pyblish / pyblish-qml

Pyblish QML frontend for Maya 2013+, Houdini 11+, Nuke 8+ and more
GNU Lesser General Public License v3.0
114 stars 44 forks source link

Hot fix for v1.8.0 #295

Closed davidlatwe closed 6 years ago

davidlatwe commented 6 years ago

Fix event filter install outside supported hosts.

darkvertex commented 6 years ago

I confirm this fixes issue #294 on my end and the UI appears as it normally would.

I haven't yet played with the foster mode. (If it's broken in Linux I'll make a separate ticket.)

davidlatwe commented 6 years ago

The error was because install_event_filter() get called under unsupported host, and vesselParent is the main window of the host, which will only be set in supported hosts.

Thanks @darkvertex :)

darkvertex commented 6 years ago

I see! The explanation is appreciated.

When you speak of "supported hosts" do you mean DCCs like Maya, Houdini, etc? Does "Python" (standalone Python) count as a supported host in this context? I mean, I know Pyblish works with standalone Python, but I guess the hosts concept is for running from inside another software, correct?

davidlatwe commented 6 years ago

When you speak of "supported hosts" do you mean DCCs like Maya, Houdini, etc?

Yes, but to be more specific, I mean the host with GUI :)

davidlatwe commented 6 years ago

I added one more commit, for the cases like running inside mayapy. Sorry for the trouble guys.

mottosso commented 6 years ago

Oh! I didn't notice there was an event filter on the main window involved; that is a problem.

An event filter on the main window is a major performance bottleneck, as it would funnel every single Qt event - from user input to draw calls - through Python. There are hundreds of thousands of them, sometimes per second, and Python just isn't up for it. It affects things such as just clicking in the viewport; you may notice a slight 10-50 ms delay with an event filter installed.

The worst thing about it is that there is no way for the end-user to know why things have suddenly gotten slow. No way to debug.

It looks like it's only used for the foster child feature, could we make sure this is only installed when that feature is used? Preferably we'd find another way of accomplishing whatever it is doing without using an event filter.

davidlatwe commented 6 years ago

It looks like it's only used for the foster child feature

Actually, it's used by the fake child approach. ;) Surprise !

The reason it's fake is because we use event filter to tap the host main window's window state and the QML window response with stay-on-top/minimize/raise to accomplish.

The foster mode did not use event filter since it's already inside a container window which spawned by the host. But in order to avoid host's busy main thread during validation and publish, the QML window parent back to subprocess, in other words, back to fake child mode, so the event filter is involved again.

Can the event filter be addressed as not-a-threat ? Since it's quietly served us a while :P

mottosso commented 6 years ago

Oh no! Thanks for confirming, I'll file an issue for it so we can have a look at resolving it.

In that case, this PR looks good.

davidlatwe commented 6 years ago

Thanks @mottosso :) Then I guess it's okay to bump the version to 1.8.1 for merge ?

mottosso commented 6 years ago

Yes please. :)

davidlatwe commented 6 years ago

Ok, it's done.