leon-thomm / Ryven

Flow-based visual scripting for Python
https://ryven.org
MIT License
3.76k stars 439 forks source link

PySide 6 Update #178

Closed HeftyCoder closed 3 months ago

HeftyCoder commented 9 months ago

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

This can also help close #138

leon-thomm commented 9 months ago

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.

HeftyCoder commented 9 months ago

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 commented 9 months ago

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 commented 8 months ago

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 commented 8 months ago

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

pfjarschel commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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).

HeftyCoder commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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!

leon-thomm commented 3 months ago

closing in favor of #189