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

PySide 6 Update #178

Closed
wants to merge 5 commits into from
Closed

PySide 6 Update #178

wants to merge 5 commits into from

Conversation

HeftyCoder
Copy link
Contributor

@HeftyCoder HeftyCoder commented Dec 15, 2023

  • PySide6 works without breaking PySide2. Probably works the same for PyQt5, PyQt6, but haven't tested. Not Supported
  • Scaled connection path animation can now start / stop and was slightly optimized
  • Fixed the bug when taking a full view screenshot
  • NodeInspectorDefaultWidget updates and fixes (splitter for content, load unload for child if it exists)

If you check it out and see no problems, I can also make pyside6 the default.

This can also help close #138

- PySide6 works, haven't tested with PyQt6
- Scaled connection path animation can now start / stop and was slightly optimized
- Fixed the bug when taking a full view screenshot
- Unload happens after Inspector has been removed, Load happens before the inspector is made visible
- Splitter for NodeInspectorDefaultWidget for its content. Text description can now be hidden.
- FIX: NodeInspectorDefaultWidget load and unload now calls load and unload of a child as well, if it exists.
@leon-thomm
Copy link
Owner

Probably works the same for PyQt5, PyQt6, but haven't tested.

PyQt is incompatible and won't be supported.

Are all these changes related to PySide6? If not, can we separate them? It might be useful for others to know what changes need to be done for PySide migration.

@leon-thomm leon-thomm changed the title PySide 6 / PyQt 6 Update PySide 6 Update Dec 16, 2023
@HeftyCoder
Copy link
Contributor Author

HeftyCoder commented Dec 16, 2023

PyQt is incompatible and won't be supported.

Had no idea.

The first three commits are fixes so that Ryven works with both PySide2 and PySide6. They also include a (one word) fix so the full screen capture works (was bugged previously) and an update to ConnectionAnimation so that it is compatible with both PySide versions. I also added the ability to start, stop instead of only toggling.

The last commit is a small change in the NodeInspector that has nothing to do with pyside. The default inspector didn't call the load / unload of the child, if it existed and I added a splitter between the content and the description for the default inspector.

Last commit is easy to remove and add to a new PR, for the first I'd have to remove start / stop from ConnectionAnimation for it to only include pyside fixes.

@HeftyCoder
Copy link
Contributor Author

The changes were minor, so I believe they should be noted here instead of having to re-arrange the whole PR. I can edit the PR description to include what needed to be done.

@HeftyCoder
Copy link
Contributor Author

HeftyCoder commented Dec 19, 2023

How should we proceed? As is and mention pyside changes in the description or remove all the other stuff and include them in a new PR?

@pfjarschel
Copy link

Looking forward to this. Will use your fork for the time being.

@pfjarschel
Copy link

Just a heads up: There are 2 issues that I found when using PySide6:

  1. Button node no longer works. The solution is simple though: Swap the QPushButton/NodeMainWidget inheritance order on ButtonNode_MainWidget definition
  2. The sizes are all weird. It seems PySide6 inserts lots of margins everywhere, so widgets feel smaller than they should. Also, titles of nodes that have the "small" style become hidden and unreadable. For now, I disabled the "small" style of such nodes. I believe this also has to do with enormous margins.

@leon-thomm
Copy link
Owner

back from winter break;

the migration is not trivial (clearly requires more than changing imports, will need some manual testing, probably changes to the stylesheet) so I'd like to not mess this up with other changes. I can try to cherry-pick relevant changes here to initiate another PR.

@HeftyCoder
Copy link
Contributor Author

HeftyCoder commented Jan 6, 2024

Happy new year!

I can try to cherry-pick relevant changes here to initiate another PR.

I'd suggest you keep all the changes, most of them were replacing deprecated qt stuff with their current working versions and only ONE import had to be changed. That's what it took to get pyside6 running. The other stuff are just a fix for the screen shot bug and a splitter for the inspector, I see no reason not keeping them. This PR doesn't change the default to pyside6, so it doesn't break anything.

# for compatibility between qt5 and qt6
try:
    from qtpy.QtGui import QUndoStack
except ImportError:
    from qtpy.QtWidgets import QUndoStack

This is the only import that was different.

Just a heads up: There are 2 issues that I found when using PySide6

I hadn't tested all the nodes of the std library, nice catch on the button. Apart from that, are you maybe using a Mac? I do not see the behavior you are describing on Windows and a colleague of mine also tested it on Fedora, which had no problems (at least I think). Only MacOS had problems.

the migration is not trivial

Please check with other OSes as well. Perhaps it is pyside related and there is little to nothing that can be done (I'd argue against changing the stylesheets, for me on Windows pyside2 and pyside6 are identical, with pyside6 being sharper and a bit more performant on widget animations).

Copy link
Contributor Author

@HeftyCoder HeftyCoder Jan 6, 2024

Choose a reason for hiding this comment

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

Unrelated change to pyside6 migration. Fixes the full view screenshot bug.

Copy link
Contributor Author

@HeftyCoder HeftyCoder Jan 6, 2024

Choose a reason for hiding this comment

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

Unrelated change to pyside6 migration. Splits the inspector custom content with the meta-data content with a splitter for better inspection. Also loads / unloads the child inspector as well.

@HeftyCoder
Copy link
Contributor Author

These are the only two changes that have nothing to do with pyside6. Feel free to remove them if you deem it necessary, I have all my changes saved in another branch either way.

@pfjarschel
Copy link

pfjarschel commented Jan 8, 2024

Apart from that, are you maybe using a Mac? I do not see the behavior you are describing on Windows and a colleague of mine also tested it on Fedora, which had no problems (at least I think). Only MacOS had problems.

Actually, I am on Windows too (11)! Maybe it has to do with high DPI scaling? I tried some different settings, but it didn't help much. Also changed the scaling of my both displays to 100%, and it's still the same. I attach an example of the difference when supressing the style = 'small' lines:

image
image

One other possibility I can think of, is that somehow a full Qt installation could affect the appearance in some way? I did some work using Qt with C++, so I have the full installation also present here. Other than that, maybe I got some other package that messes with PySide styling. Will have to try on a clean environment. Just to clarify, I am using Python 3.11.4.

Will keep you guys posted!

Edit: Same thing on a clean environment...

@HeftyCoder
Copy link
Contributor Author

Just to clarify, I am using Python 3.11.4

I've been testing on 3.10 so I could test pyside2 and pyside6 as well. I think what's important here is checking of any differences between pyside2 and pyside6 rather pyside6 alone. Do you notice any difference between pyside2 and pyside6 in 3.10? Also, do you notice any difference between pyside6 in 3.10 and 3.11 respectively?

The small style not having a title is not something new I believe, the val node is the same way (apart from the weird margins), irrespective of pyside versions.

It is important to clarify whether pyside2 and pyside6 have different default values in anything. The code changes I made were replacing deprecated pyside code and one import correction. Please check the above.

@leon-thomm
Copy link
Owner

I think what's important here is checking of any differences between pyside2 and pyside6 rather pyside6 alone.

I agree. Note that pyside2 runs on Python <3.11, so 3.11 is actually not compatible with the pyside2 one. Could you test with 3.10?

One other possibility I can think of, is that somehow a full Qt installation could affect the appearance in some way?

There can be temporary bugs with shared libraries (I ran into them on win 10) but generally this shouldn't be an issue.

The small style not having a title is not something new I believe

correct, I think small style needs some changes, I think I would move the title visually to the top of the node - as in normal - but only show it on mouse hover or something.

@pfjarschel
Copy link

Sorry for the delay!
So, I tested on python 3.10, and the size issue shown in the pictures also happens with either PySide2 or PySide6, so it's definitely something on my end! Probably some Qt configuration or environment variable that is affecting it. If I ever discover the exact cause of this issue, I'll let you guys know!

This was referenced May 18, 2024
@leon-thomm
Copy link
Owner

closing in favor of #189

@leon-thomm leon-thomm closed this May 19, 2024
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.

Is it possible to officially upgrade to Qt6 & PySide6?
3 participants