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

new install script with various tweaks #61

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

SynthetikzZ
Copy link

@SynthetikzZ SynthetikzZ commented Sep 22, 2024

hello,

since KDE 6 i had graphical issues and because of PySide 2 relying on outdated dependencies it needs to run in a virtual env.

i made a new install script that manages the installation of the venv automatically, but for me that didnt resolved the graphical issues. the script also allows to rewrite the TinyPedal files to use PySide6.This allows running TinyPedal with latest Python. using PySide6 you need to use QT_QPA_PLATFORM=xcb flag to be able to move the widgets using Wayland wich i added to TinyPedal.sh. on KDE 6 desktops the dependencies for tinypedal usually already installed and graphical issues went away.the scaling of the widgets changed slightly for me (they are bigger, but they where too small before). installation destination and sudo rights all managed by the script. do not run the script itself with sudo.

installing dependencies when using PySide2 with venv is automatically done trough the script.

using a custom installation path is still possible, script doesnt know that its installed after installation so script wont allow to setup venv/ switch to PySide6. solution would be to write the path into a config file stored somewhere

@SynthetikzZ
Copy link
Author

quick notes: using the QT_QPA_PLATFORM=xcb flag also stops every widget to appear as own window when alt+tabbing trough windows in linux

@SynthetikzZ
Copy link
Author

possible additions for the future :
installing the installscript to the /bin path as tpsetup
adding detection for updates
installing the updates directly trough the script.

@s-victor
Copy link
Owner

s-victor commented Sep 23, 2024

Hi, thanks for the notice and work.

The issue related to pyside2 and pyside6 was discussed here earlier:
#42 (comment)

The main reasons the APP wasn't ported to pyside6 (only a few lines change) are largely due to significantly increased CPU & Memory usage (almost doubled CPU percent and 20mb+ memory, which is not good). I had actually spent quite some time testing and debugging with pyside6, but there is no sign as why it consumed more system resource than pyside2. So for this reason, using pyside6 may still be a remained question.

I probably won't help or test with the issue since I don't have a real linux environment to test (only have a simple virtual machine setup for testing basic functionalities).

You may consider discuss the details with @berarma as he has been helping maintaining linux platform stuff.

I'll try review pyside6 viability later once I finished currently scheduled work.

Cheers

@SynthetikzZ
Copy link
Author

On my machine it's the only way to use TinyPedal without graphical bugs. The installer gives you the choice to try it out, but also to revert the changes if pyside2 runs without bugs :)

@SynthetikzZ
Copy link
Author

SynthetikzZ commented Sep 23, 2024

I searched the web and apparently QT_QPA_PLATFORM=xcb also should fix CPU usage on Linux. Also didn't noticed any performance issues (have a capable Mashine tho) , I'll do some tests later

Note: scaling of widgets changes because pyside6 uses high dpi scaling by default.

@berarma
Copy link
Contributor

berarma commented Sep 23, 2024

Hi.

Sorry, I don't have a lot of time now but I've taken a look and tested it very basically. My concerns below. I'm not the main target user so it should be weighted with opinions from others.

I see the advantage of using a virtual env but I don't understand why the script tells me I need a virtual env to run TinyPedal. I guess that's why I don't see an option to install in the menu, but I know it can run without it.

I don't have any of the issues mentioned above using PySide2. Maybe it's just me but I think no one else had complained before. I think the messages make it too big of a deal.

I'm wary of scripts that want to do too much, and also changing the app code. I would prefer the old minimalist approach. And with so many tasks at hand, making sure this script works fully on several systems can be a bit cumbersome.

Anyway, if you're fully on it, could we make it less tailored to your system (the messages and venv requirement) and make an option for the minimalist install?

@SynthetikzZ
Copy link
Author

SynthetikzZ commented Sep 23, 2024

Hi.

Sorry, I don't have a lot of time now but I've taken a look and tested it very basically. My concerns below. I'm not the main target user so it should be weighted with opinions from others.

I see the advantage of using a virtual env but I don't understand why the script tells me I need a virtual env to run TinyPedal. I guess that's why I don't see an option to install in the menu, but I know it can run without it.

I don't have any of the issues mentioned above using PySide2. Maybe it's just me but I think no one else had complained before. I think the messages make it too big of a deal.

I'm wary of scripts that want to do too much, and also changing the app code. I would prefer the old minimalist approach. And with so many tasks at hand, making sure this script works fully on several systems can be a bit cumbersome.

Anyway, if you're fully on it, could we make it less tailored to your system (the messages and venv requirement) and make an option for the minimalist install?

you cant install pyside2 on linux globally because it depends on outdated packages (clang) since i think KDE6 came out so it has to run in a venv otherwise you cant install it. might be a rolling release problem for now, but will appear on other distros sooner or later aswell.

i run endeavour os and with pyside2 running, if i move the widgets, they leave flickering lines everywhere on my monitor. this occurs only since kde 6 to me.

the script checks for tinypedal installs under /usr/local/ and §HOME/.local and only presents the options to you that are relevant. installation options only available if no install found under these two locations.

i surely can change some texts tho.

if really no one has issues i could remove pyside6 conversion entirely and leave it to my personal installation.

the sudo handling stuff definitly worth keeping.

i started to modify the script because it had problems parsing white spaces in source path variable and somehow ended up inserting much more. rightnow i am compiling clang 17 so maybe i still can install pyside2, but the venv option definitly eases up things for people who maybe new to linux. compiling clang takes some time so venv definitly worth keeping as an option.

@s-victor
Copy link
Owner

Thank you both @berarma & @SynthetikzZ for help and work.

Please take your time, I think there is no rush needed for this feature/issue until there is a good solution for everyone (hopefully).

Moving to pyside6(Qt6) will probably be inevitable eventually, though currently pyside2 is more efficient/faster and lightweight compare to the latter for the time being (as from side by side testing results earlier). I hope to get some time to re-evaluate pyside6 viability after finishing up current work on new stuff and next coming update.

@SynthetikzZ
Copy link
Author

Thank you both @berarma & @SynthetikzZ for help and work.

Please take your time, I think there is no rush needed for this feature/issue until there is a good solution for everyone (hopefully).

Moving to pyside6(Qt6) will probably be inevitable eventually, though currently pyside2 is more efficient/faster and lightweight compare to the latter for the time being (as from side by side testing results earlier). I hope to get some time to re-evaluate pyside6 viability after finishing up current work on new stuff and next coming update.

Yes there is no haste, I keep the pull request open and do some changes and you can look over the code when y'all have time for it.

@BabaR-f1 BabaR-f1 mentioned this pull request Jan 3, 2025
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.

3 participants