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

Parenting tools #472

Merged
merged 17 commits into from
Nov 7, 2019
Merged

Parenting tools #472

merged 17 commits into from
Nov 7, 2019

Conversation

davidlatwe
Copy link
Collaborator

@davidlatwe davidlatwe commented Nov 5, 2019

Problem

Avalon tools like Loader, Creator, Scene Manager, Context Manager, Workfile were not parented in Houdini and Nuke's main window.

And the Workfile app's show() wasn't accept parent arg at all.

Changes

  • Implemented main window getter function for Nuke (Update: Also Houdini and Maya)
  • Add parent flag to avalon.tools.workfiles.show()
  • Parenting tools for Nuke and Houdini
  • Fix tools style after parented in Houdini

While I was working on Houdini, I encountered two problems:

  1. The Qt style sheet was overridden by Houdini.
  2. Calling Context Manager raised Segmentation fault fatal error.

On problem 1, the Loader has become:

See Image

image

And the Context Manager:

See Image

image

Which looks cool but I love Avalon flavor more.
Interestingly, Creator and Scene Manager did not have this issue, not sure why :(

Edit: ☝️ Actually they do, see comments below.

See Image

image

On problem 2, it was because of directly connecting QtWidgets.QTreeView.selectionModel() to the handler, and leads to a Segmentation fault error and crash Houdini.

The only useful info I could find was this tweet.

@davidlatwe davidlatwe requested review from mottosso and BigRoy November 5, 2019 11:02
avalon/nuke/pipeline.py Outdated Show resolved Hide resolved
@@ -42,6 +42,7 @@ def __init__(self, parent=None):
task_view.setIndentation(0)
task_model = TasksModel()
task_view.setModel(task_model)
task_view_selection = task_view.selectionModel()
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

By keeping the QItemSelectionModel object before connecting signal, avoids the Segmentation fault crash in Houdini which described above.

Copy link
Collaborator

@tokejepsen tokejepsen left a comment

Choose a reason for hiding this comment

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

Awesome stuff!

Code looks good, but cant test.

@davidlatwe
Copy link
Collaborator Author

Thanks @tokejepsen ❤️
For testing, hope that @BigRoy can have a test on Houdini, and @jasperges or @jezscha able to test on Nuke ☺️

Also, I don’t have fusion here, so the tools still unparented in fusion.

@BigRoy
Copy link
Collaborator

BigRoy commented Nov 5, 2019

Also, I don’t have fusion here, so the tools still unparented in fusion.

There's no Qt in Fusion so I think that will have to remain as is unfortunately.

For testing, hope that @BigRoy can have a test on Houdini

Very nice! I wonder how I missed doing this at the time. Anyway, nice work!

Houdini avalon tools font difference

I did notice the styling felt a bit off as it seems to be using the Houdini font as opposed or something? For example:

Creator

afbeelding

I wonder if there's a way to patch that up too.

Houdini Work files error

The Work files tool however pops up this error on opening:

Traceback (most recent call last):
  File "D:\git_pipeline\avalon\development\git\core\avalon\tools\workfiles\app.py", line 344, in <lambda>
    QtCore.QTimer.singleShot(100, lambda: self.list.scrollToItem(item))
RuntimeError: Internal C++ object (PySide2.QtWidgets.QListWidgetItem) already deleted.

Additionally it seems that the Work Files tool does not have the Avalon styling whatsoever. Or does it look so different solely due to the font being so different?

Workfiles App

afbeelding


Also I did recall this comment in Shotgun's toolkit regarding styling Qt widgets inside houdini which calls the method here. Maybe we should try and just apply the stylesheet globally in Houdini? Sounds a bit hacky though.

Or `RuntimeError: Internal C++ object (PySide2.QtWidgets.QListWidgetItem) already deleted.` will raised and hangs Houdini.
@davidlatwe
Copy link
Collaborator Author

davidlatwe commented Nov 5, 2019

Maybe we should try and just apply the stylesheet globally in Houdini? Sounds a bit hacky though.

Haha, and it says here..
NOTE: Except for 16+. It's no longer safe and causes lots of styling problems in Houdini's UI globally.

So hacky indeed !

Anyway, I finally resolved the font size and QSplitter issue by adding some entry in style.qss, the Loader and Creator tools should now look almost the same as in other DCC.

Here's the before and after:

before

image

Notice that the splitter in Loader is a thick gray line even mouse is not hover on top of them.

after

image

🎉

Furthermore, I also moved the line window.setStyleSheet after window.show in every tool, so their appearance should all aligned now.

Keeping main window instance in Avalon session to avoid child widgets being garbage collected before entering the deferred scope.
Remove duplicated main window getting code
@davidlatwe
Copy link
Collaborator Author

davidlatwe commented Nov 6, 2019

Houdini Work files error

The Work files tool however pops up this error on opening:

Traceback (most recent call last):
  File "D:\git_pipeline\avalon\development\git\core\avalon\tools\workfiles\app.py", line 344, in <lambda>
    QtCore.QTimer.singleShot(100, lambda: self.list.scrollToItem(item))
RuntimeError: Internal C++ object (PySide2.QtWidgets.QListWidgetItem) already deleted.

Originally, this issue was fixed in 1b2e9ad, but after a few more search, I find that keeping the Houdini's main window instance in a place which can have a longer lifespan is a better solution, which fixes in a higher scope (based on this post). So I revert the previous commit and add 5907a80.

Note that the Segmentation fault error described in top comment can still exists after this change.

And for removing duplicated main window getting logic plus having same function for each host, I also add get_main_window in maya.pipeline in 49687fe.

@tokejepsen
Copy link
Collaborator

Just noticed as well that the Loader is not parent to the Maya window.

@davidlatwe
Copy link
Collaborator Author

@tokejepsen, really !? Loader did get parented in Maya here in my end. 😲

@tokejepsen
Copy link
Collaborator

@tokejepsen, really !? Loader did get parented in Maya here in my end. 😲

Ok, I'm not on the latest core, so it could just be something with my fork. Nvm :)

Copy link
Collaborator

@BigRoy BigRoy left a comment

Choose a reason for hiding this comment

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

Tested it again, and now the tools open fine on Houdini without errors and they are nicely parented. The Avalon stylesheet still doesn't apply perfectly, it still seems to inherit quite a bit from the Houdini styling.

However it seems the styling of Houdini itself might change in the near future that might automatically resolve it. Maybe it's best to leave it as is since it isn't a big issue and then maybe with an upcoming Houdini version less of the Houdini styling pushes through in the Avalon tools.

Nice work @davidlatwe ! 🚀

@davidlatwe
Copy link
Collaborator Author

Will merge this tomorrow if no other objections :)

@mottosso
Copy link
Contributor

mottosso commented Nov 6, 2019

I've scanned through the code and couldn't find anything I didn't like. Nice work. :)

@davidlatwe davidlatwe merged commit 815aa55 into getavalon:master Nov 7, 2019
@davidlatwe davidlatwe deleted the parenting-tools branch November 7, 2019 10:15
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.

4 participants