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

Implemented eventFilter removal and improved Foster stability #297

Merged
merged 28 commits into from
Sep 8, 2018

Conversation

davidlatwe
Copy link
Collaborator

@davidlatwe davidlatwe commented Sep 1, 2018

In this PR, should have:

  • eventFilter auto remove after quitting under any conditions
    • No matter foster is on or not, usual window close or force quit, should remove evenFilter in the end.
  • Foster mode should work under any versions of Maya
    • Behavior may differ by the Qt version on host, in my observation, host which running Qt4 could not render QML window properly while window setParent back to subprocess (window detach-attach), which meant for avoiding main thread busy. So in this PR, in order to guaranty the visual feedback, those Qt4 host will not be able to avoid main thread busy (lag better then blank).
    • For test propose, you can turn detach-attach on or off by setting environment variable PYBLISH_QML_FOSTER_NINJA=True or False (Yes/No 1/0)
  • In any condition, window should able to ignore closeEvent while processing, unless force quitting
    • This was broken in my previous PR. :(
  • Host main window should blink alert on validation/publish complete when in foster mode
    • In foster mode, the alert can only be shown on host main window.
  • Fake child approach's OS limitation removed
    • Was only enabled on Windows, limitation is gone now.

Here are some Foster mode running screen gif:

This is Maya 2018.0 on Windows

qml_foster_windows

This is Maya 2015 on Windows

qml_foster_2015

As you can see, it's really laggy.

This is Maya 2017 update 5 on CentOS 7 (Run in Hyper-V)

qml_foster_linux

Foster mode works on CentOS !
One thing to be noticed is that, while in my first test on CentOS, I installed Maya 2017 and encountered an error says:
FosterVessel(QtWidgets.QDialog) has no attribute 'winId'
This is a bug, the error was gone after I installed the Maya 2017 update.

Now, I wouldn't say the Foster mode is 100% safe to use, but should at least stable as Fake child approach.

Any feedback are welcome :)
Thanks !

@davidlatwe
Copy link
Collaborator Author

Here's my plugins Gist that used in tests.

@mottosso
Copy link
Member

mottosso commented Sep 2, 2018

Stellar effort @davidlatwe! I've given this a go on Windows 10, Maya 2018 and all went well. There's some flickering in the window when it goes from being a child to its own window, but I can't see a way around that.

I've added some docstrings, but was hoping you could fill in what "ninja" meant, here.

@mottosso
Copy link
Member

mottosso commented Sep 2, 2018

Looks like something is off for Maya 2015 and 2016, presumably related to Qt 4 vs. Qt 5.

image

It sits there, nothing happening. This is with PYBLISH_QML_FOSTER=1

@mottosso
Copy link
Member

mottosso commented Sep 2, 2018

This produces the same result, not sure if it helps.

from pyblish_qml import show
show(foster=False)

@mottosso
Copy link
Member

mottosso commented Sep 2, 2018

Also to confirm that the current master, and the master before merging the foster update, works in 2015. So there's something in this PR specifically causing it not to run.

@davidlatwe
Copy link
Collaborator Author

Thanks for quick spin @mottosso !

Looks like something is off for Maya 2015 and 2016, presumably related to Qt 4 vs. Qt 5.

Yes, it's related to Qt version, but the reason it freeze was from the change in commit 7b3277c .
It's about ninja, too. ninja is foster mode's window detach-attach transition process, which I could not come up with a better name.
I would like to elaborate more, but I need to hit the road now, I'll come back later :)

self.vessel.show()
return self._dispatch("show", args=[settings or {},
self._winId,
self.ninja])

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This spot, is the reason that make Maya 2015 and 16 freeze at startup.
ninja is disabled if host running Qt4, and if ninja is disabled, it need to call vessel.show before _dispatch("show"), or the window will not show up.

@davidlatwe
Copy link
Collaborator Author

davidlatwe commented Sep 2, 2018

Okay, just tested on 2015, it should wok now :)

I renamed the param ninja to foster_fixed, should be clear enough I think. It's to control whether the GUI to avoid main thread busy by parent back to subprocess or not.

If host's Qt version==4, GUI will become blank white after window setParent back to subprocess from host (tested on Windows). So I made this extra switch foster_fixed, to let the fostered window always stay inside the host, although this will produce laggy GUI when host main thread is busy, but it's better then a blank window.

By default, the bool value of foster_fixed is set from host's Qt version, one can override it through additional environment variable PYBLISH_QML_FOSTER_FIXED=True.
Here's the code that define the value (describe better then my words) :

if not foster:  # If Foster mode is not turned on, `foster_fixed` is False
    return False

value = os.environ.get("PYBLISH_QML_FOSTER_FIXED", "").lower()
return value in ("true", "yes", "1") or QtCore.qVersion()[0] == "4"

There's some flickering in the window when it goes from being a child to its own window, but I can't see a way around that.

If everything works well, this window flickering and the taskbar alert change to main window should be the last two trade off. :P

One other buggy appearance was on CentOS, the window did not resize properly when it parent back to subprocess. But I am going to let it slide for now, will back to fix this one after a while. :)

Does foster_fixed works for you ? @mottosso

@mottosso
Copy link
Member

mottosso commented Sep 8, 2018

Works!

Great work @davidlatwe. If you're ready, I'm happy to merge and release. Have you incremented the version too?

@davidlatwe
Copy link
Collaborator Author

Bump !
Thanks @mottosso :D

@mottosso mottosso merged commit 5d5fc0c into pyblish:master Sep 8, 2018
@mottosso
Copy link
Member

mottosso commented Sep 8, 2018

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.

2 participants