Fix: multiple displays
The fix works for:
- Windows
- Linux+XOrg, with a good preparation for Wayland
- Most likely for MacOS, wasn't tested.
Qt6 has an issue with the primary display detection. Depending on the location of the screens, the primary screen doesn't remain unchanged for Qt. I also tried using first/last, but it didn't work either. It means that I cannot detect the offset for the composite (all screens) screenshot. The screenshot generates properly, but I didn't find a way to locate it correctly. So I reworked the logic of the application.
New logic:
- Take a screenshot from only the active display (mouse position).
- If you move the mouse cursor to another display, it will make a screenshot for the new active display.
- If you started to edit a screenshot, a new screenshot won't be taken.
This approach simplifies work for Mac and Wayland (once implemented). The issue with "no icon in the status bar" is handled.
Also, I partially removed some features, like "take an area" from the command line, as it didn't work anyway. I reworked a screenshot-grabbing mechanism from scratch.
Note: This MR is not well-tested. Please help me with it.
@borgmanJeremy @mmahmoudian
Testing this on Arch with i3 and it seems to be working, although it only shows on 1 display at a time (which is fine).
@keysmashes, try this one, it was completely reworked.
This seems to work perfectly in terms of DPI/multimonitor :)
I'm not the biggest fan of the new one-display-at-a-time interface, and it's a bit weird that after making a selection and then deselecting (clicking outside so the instructions show up again) the "active" display no longer changes, but it's not the end of the world I guess...
This seems to work perfectly in terms of DPI/multimonitor :)
I'm not the biggest fan of the new one-display-at-a-time interface, and it's a bit weird that after making a selection and then deselecting (clicking outside so the instructions show up again) the "active" display no longer changes, but it's not the end of the world I guess...
@keysmashes, thank you very much for the feedback.
Unfortunately, I couldn't resolve the Qt6 issues and didn't find a way to calculate the offset for different screen positions. Also, we have issues with a composite screenshot for macOS (where a similar approach is used from the beginning) and Wayland (where we cannot draw across multiple displays). This approach is one stone, many birds.
About the issue with deselection - I'll fix it a bit later, it's not a big deal.
@panpuchkov Thanks for taking the bullet and working on this. I know this is not a convenient thing to handle.
Take a screenshot from only the active display (mouse position).
This would potentially also help with different monitor scaling. The only drawback is that the user cannot take a screenshot that is on two monitors. I do acknowledge that this is not the most common use-case for a screenshot tool and it would be a "good enough" work-around.
I just wonder how Spectacle from KDE is working. It is also on Qt6 and is handling the multi-monitor pretty well. Can we get any inspiration from them?
This MR is not well-tested. Please help me with it.
I'll definitely test it and give you feedback.
@mmahmoudian Spectacle has the advantage of only need to work on the most recent KDE release. Flameshot needs to work across 3 operating systems and the growing number of linux DE's. I agree with @panpuchkov that the best path forward is the single desktop approach.
I think if flameshot is invoked with no screen argument it should use the position of the cursor to determine the screen. However, i would like to adjust the tray menu so "Take Screenshot" has a sub menu with the screen numbers to choose from. Then on DE's that don't have a tray per desktop users can still select which screen should be captured.
I have not tested yet as my main machine is wayland only.
Edit: seems to work on ubuntu gnome, but I think adjusting the menu would be useful (after adding wayland).
I'm not against the proposed solution, and it would solve some of our problems. I was just curious about Spectacle.
The only issue I can think of is placing the Flameshot window exactly on the monitor that the screenshot is taken from.
@mmahmoudian Yes I agree with you on that. I've seen wayland issues where its hard to have Qt reposition the full widget to another screen if the DE decides to draw it on the wrong screen.
@merianos @borgmanJeremy
We can discuss various options and add additional features in the future, too.
My main point is that I spent two weekends from 9 am to 9 pm without finding a way to obtain accurate information from Qt6 regarding the top left corner and the display on which it starts drawing. So, I asked how people are happy on Mac with a single-display approach, and they were mostly pleased. A bit worse approach actually worked for a few years for macOS.
Also, it significantly simplifies code, future support, and testing. I eliminated a huge platform-dependent code, and now it has become more or less readable.
All ideas about extra menu options, menu adjustments, etc., as far as I'm concerned, are part of the idea's development.
If we're good with the idea in general, I'd like to clean up the code, make some refactoring, as I didn't have enough time to polish it, and merge it. Next, add all the extra features.
What do you think about it?
I'm happy to merge before the extra features are added, but I think we need to fix wayland. BTW I 100% agree with this approach and happily use flameshot on Macos even with this limitation.
Great! I'll try to find a time next weekend for the Wayland. Or, if anyone has time for it, it would be great. In theory, we only need to grab it; the rest should work fine.
I looked into the details a bit on how to accomplish this on Wayland and while possible it may be complicated. The actual xdg portal spec does not define a way to capture just 1 screen: https://flatpak.github.io/xdg-desktop-portal/docs/doc-org.freedesktop.portal.Screenshot.html
We may need to need to capture all the screens, figure out which portion of the pixmap the cursor was in, trim the image, and display that trimmed image from the composite.
Furthermore we will be unable to move the fully screen widget to the captured screen if it displays on a different one. Not quite sure how to resolve this issue yet.
@panpuchkov did you hit the same issue I did with wayland?
@borgmanJeremy Actually, I didn't have a chance to work on it yet. I have to work on my main job, and Flameshot is only on weekends. After 4 weeks without rest on weekends, I felt exhausted. Need time to recover.
I have an idea how to handle it, but I haven't checked it yet. I want to calculate all offsets from the current screen, instead of the primary one, which appears to be unpredictable. And draw on the current, instead of the primary. Or, probably, to calculate from the first one.
@mmahmoudian Yes, it works on Windows and has been actively used by many Customer Support employees at Namecheap for a few weeks now. I added a minor functional fix a few days ago (allowing the change of an Active Screen after removing a selection). I am not sure about other platforms, but Windows is tested.
@mmahmoudian
Yes, it works on Windows and has been actively used by many Customer Support employees at Namecheap for a few weeks now.
I added a minor functional fix a few days ago (allowing the change of an Active Screen after removing a selection).
I am not sure about other platforms, but Windows is tested.
It does not work on Wayland as of yet.
Have been using this on Windows for a month now as well without issues. This is the only way to make Flameshot usable for me, and probably many others.
I still haven't found a viable way to make it work on Wayland.
I'm considering merging this and then keening the old method around for Wayland until we can find a fix there.
Then users on windows an x11 can enjoy the fix.