flameshot-org / flameshot

Powerful yet simple to use screenshot software :desktop_computer: :camera_flash:
https://flameshot.org
GNU General Public License v3.0
24.58k stars 1.58k forks source link

The screenshot region selection window appears with a very noticeable delay (Wayland) #3039

Open fililip opened 1 year ago

fililip commented 1 year ago

Flameshot Version

Flameshot v12.1.0 (-) Compiled with Qt 5.15.5

Installation Type

Linux, MacOS, or Windows Package manager (apt, pacman, eopkg, choco, brew, ...)

Operating System type and version

Arch Linux 6.1.1-arch1-1

Description

The region selector appears with a very noticeable delay, even though the screenshot seems to be taken as soon as possible. I don't see any delay options in the settings so I'm assuming this is not meant to happen.

Edit: it seems that if I disable the second monitor, the region selector's startup time is slightly reduced.

Steps to reproduce

  1. run flameshot gui or click on the tray icon to take a screenshot
  2. after over 1-2s, the screenshot region selection window appears showing the desired (earlier) state of the screen meaning the app runs as fast as it can but apparently can't show the window fast enough

Screenshots or screen recordings

No response

System Information

OS: Arch Linux 6.1.1-arch1-1 DE: KDE Plasma 5.26.4, but the same thing happens on Gnome 43 (Wayland) as well Session type: Wayland Screen setup: 1920x1080@144, 1920x1080@60

rstrube commented 1 year ago

I can also confirm this behavior when building from HEAD (commit 685d0ee84bfa53b414be4d72627eca5600c6f954). When trying to select a region, the selection rectangle moves very very slowly.

This appears to be a regression, as for me using the latest official stable release (v12.1.0) the problem does not exist. I don't know exactly which commit causes the issue, but I can report that it seems way more pronounced on higher resolution monitors. On my latpop with a 1080p screen I don't have the issue (when building from HEAD), when I attach my latpop to two high resolution displays, the problem exists.

If I had to guess, I would say the commit adding snap to grid (commit 685d0ee84bfa53b414be4d72627eca5600c6f954) introduced the regression. I've been using the flameshot-git AUR package for months now (which always builds from HEAD) and this regression only happened recently.

Is there a way to disable to snap to grid functionality to confirm that this is indeed causing the regression?

mmahmoudian commented 1 year ago

@rstrube I haven't noticed any issue on X11 with 3 monitor setup. If it didn't exist in 12.1.0 and it exists in the head, it can be either the snap to grid (wich you can try to increase the distance to test if the issue reduces) or it is the live dimension showing. Also something to note is that @fililip is reporting this on 12.1.0 and not AUR

What DE are you using? are you on Wayland?

rstrube commented 1 year ago

@mmahmoudian apologies for not responding sooner. I probably should have posted more details about my system specifications.

I'm using KDE Plasma on Wayland.

I'll try to disable the live dimensions and report back. Can you easily change the snap to grid distance in the settings?

rstrube commented 1 year ago

@mmahmoudian here is more information that might be helpful.

flameshot --version
Flameshot v12.1.0 (685d0ee8)
Compiled with Qt 5.15.8
neofetch --stdout
robert@devone 
------------- 
OS: Arch Linux x86_64 
Host: Dev One Notebook PC 
Kernel: 6.1.4-arch1-1 
Uptime: 2 mins 
Packages: 975 (pacman) 
Shell: fish 3.5.1 
Resolution: 2560x1080 
DE: Plasma 5.26.5 
WM: kwin 
Theme: [Plasma], Breeze [GTK2/3] 
Icons: [Plasma], breeze-dark [GTK2/3] 
Terminal: kitty 
CPU: AMD Ryzen 7 PRO 5850U with Radeon Graphics (16) @ 4.507GHz 
GPU: AMD ATI Radeon Vega Series / Radeon Vega Mobile Series 
Memory: 1796MiB / 15317MiB

When running flameshot gui from terminal:

flameshot gui
QLayout: Attempting to add QLayout "" to SidePanelWidget "", which already has a layout
qt.qpa.wayland: Wayland does not support QWindow::requestActivate()
qt.qpa.wayland: Wayland does not support QWindow::requestActivate()
flameshot: info: Screenshot aborted.

Also disabling the geometry display did improve performance. Dragging the selection rectangle was noticeably quicker, but still quite slow compared to version 12.1.0.

TLDR: Using v12.1.0 I experience no performance issues, using the AUR package I have serious performance problems.

lbatalha commented 1 year ago

Setup:

Simple testing

I have tested using the commit before the snap to grid feature and the problem persists (this feature is disabled by default btw, but wish there was a config file option for it instead of relying on the sidebar, and I don't even see the feature toggles anywhere with latest HEAD?) Testing also with #3018 which predates that feature, using GRIM instead of xdg-desktop-portal, same issue.

Disabling the geometry display did not seem to make a difference. Disabling the magnifier did not seem to make a difference, although with it enabled, even before making a selection, the cursor stutters some amount, during the selection I cannot tell much of a difference similar to the geometry display. The reason I might be able to tell is due to my large render area.

Tracking the bad commit

The bad commit is in fact e68183451e872558116295a3333dc879e06fc53a from #2766 , the parent commit does not suffer from this issue. Note that enabling the magnifier still causes the cursor/selection stuttering, but to a much lesser degree than the geometry indicator, so I assume this needs to be investigated in a separate issue, even if the causes might be similar.

It is unclear to me why disabling the geometry indicator still causes such a severe problem. (though in the original PR discussion it was noted that there were issues, but I assume with their setup it was only a slight issue, amplified as resolution increases)

EDIT: #3059 improves performance and actually allows disabling of the geometry indicator code

pintassilgo commented 1 year ago

I just built from git and can confirm this bug. I'm on Plasma Wayland. Latest release (v12.1.0) doesn't have this issue.

adamdicarlo0 commented 1 year ago

can also confirm; whether doing initial selection or resizing the selection area, there's major lag. I'm on swaywm 1.8.1. I tried building flameshot with -DUSE_WAYLAND_GRIM=1 (in addition to -DUSE_WAYLAND_CLIPBOARD=1, which is included in the flameshot-git PKGBUILD on AUR). No difference.

raidenovich commented 1 year ago

can also confirm; whether doing initial selection or resizing the selection area, there's major lag. I'm on swaywm 1.8.1. I tried building flameshot with -DUSE_WAYLAND_GRIM=1 (in addition to -DUSE_WAYLAND_CLIPBOARD=1, which is included in the flameshot-git PKGBUILD on AUR). No difference.

Can also vouch. I did the same with no varying results on swaywm.

xahon commented 1 year ago

This commit fixes the issue (https://github.com/flameshot-org/flameshot/compare/master...xahon:flameshot:fix-performance-degradation)

This change doesn't introduce issues with updating the GUI of both selection and xywh tools

xahon commented 1 year ago

This issue also affects Windows 10, I checked the profiler and it shows me this

image

I also noticed slight delay when running the app in profiler and stretching the selection

* Red rectangle shows execution at the moment when I stretched the selection box

lbatalha commented 1 year ago

The lag when changing selection box is due to the xywh code, #3059 adds the ability to disable it (as well as other fixes), but the real problem is that update or repaint both cause too much lag for some unknown reason.

In that PR, the repaint was changed to update which is faster, but removing that line does not improve the slow startup problem. On my machine (Sway/Wayland) it still takes >3 seconds from calling flameshot gui to it displaying the screenshot and allowing selection.

Example video showing the delay (and that the environment is configured correctly, overriding QT_SCALE_FACTOR to empty since flameshot will not scale properly unlike other QT apps)

https://github.com/flameshot-org/flameshot/assets/5883822/42d96ab3-1068-4ee6-8927-6367a7162748

Removing the update/repaint does improve performance, but also causes smearing of the xywh box in certain situations (although if you have showSelectionGeometryHideTime set, the repaint triggered by the timer does clean them up).

Example with and without the update/repaint in the showxywh() function (its using the PR branch, but with your fix the results are the same): https://github.com/flameshot-org/flameshot/assets/5883822/f9977640-57ef-495d-8373-9757d6d14cd4 https://github.com/flameshot-org/flameshot/assets/5883822/62415995-e914-4a07-aaa4-ba973ae9257a

It could be debated that living with slight corruption outside the selection area (when the selection sweeps over it, it cleans it up) is worth it for the massive performance boost (without the repaint/update even on my system the selection dragging is absolutely smooth)

Not sure how other people feel about that tradeoff, but if it is acceptable, I can patch it in #3059