Bug 478426 - [Wayland] Screenshot quality issues with dual monitor setup and fractional scale factor on one of them
Summary: [Wayland] Screenshot quality issues with dual monitor setup and fractional sc...
Status: RESOLVED FIXED
Alias: None
Product: Spectacle
Classification: Applications
Component: General (show other bugs)
Version: 24.01.80
Platform: Fedora RPMs Linux
: NOR normal
Target Milestone: ---
Assignee: Noah Davis
URL:
Keywords: multiscreen, qt6, wayland
: 482750 483816 484003 (view as bug list)
Depends on:
Blocks:
 
Reported: 2023-12-12 08:26 UTC by John Iliopoulos
Modified: 2024-04-11 01:57 UTC (History)
10 users (show)

See Also:
Latest Commit:
Version Fixed In: 24.05


Attachments
100% scaling (60.38 KB, image/png)
2023-12-12 08:26 UTC, John Iliopoulos
Details
125% scaling (106.88 KB, image/png)
2023-12-12 08:28 UTC, John Iliopoulos
Details

Note You need to log in before you can comment on or make changes to this bug.
Description John Iliopoulos 2023-12-12 08:26:59 UTC
Created attachment 164102 [details]
100% scaling

SUMMARY
I have two monitors both 1920x1080 but one of them is at 125% scaling.
when taking a screenshot of something on my 100% scaling monitor picture is blurry and screwed.
when taking a screenshot on my 125% scaled monitor it looks fine.

STEPS TO REPRODUCE
1. Have two monitors
2. set one to some kind of fractional scaling
3. take a screenshot on the monitor with the normal scaling

OBSERVED RESULT

See the attached image, image quality is obviously dropped.

EXPECTED RESULT

image quality quality should be preserved as on the monitor.

SOFTWARE/OS VERSIONS
Operating System: Fedora Linux 40
KDE Plasma Version: 5.90.0
KDE Frameworks Version: 5.246.0
Qt Version: 6.6.1
Kernel Version: 6.7.0-0.rc4.20231208git5e3f5b81de80.38.fc40.x86_64 (64-bit)
Graphics Platform: Wayland
Processors: 16 × AMD Ryzen 7 5700U with Radeon Graphics
Memory: 14.9 GiB of RAM
Graphics Processor: AMD Radeon Graphics
Manufacturer: HP
Product Name: HP ENVY x360 Convertible 15-eu0xxx
Comment 1 John Iliopoulos 2023-12-12 08:28:48 UTC
Created attachment 164103 [details]
125% scaling

added secondary attachment of the screenshot as taken on the 125% scaled monitor. As you can see there is no loss of image quality.
Comment 2 Noah Davis 2023-12-12 13:58:26 UTC
Moved to KWin since the screenshots come from KWin's screenshot plugin. Spectacle does rectangle screenshots by getting a workspace screenshot via the screenshot plugin and then cropping it.
Comment 3 Vlad Zahorodnii 2024-01-15 21:37:34 UTC
(In reply to Noah Davis from comment #2)
> Moved to KWin since the screenshots come from KWin's screenshot plugin.
> Spectacle does rectangle screenshots by getting a workspace screenshot via
> the screenshot plugin and then cropping it.

If it takes a workspace screenshot, kwin will use the highest scale factor (125%) for the screenshot because two monitors will be captured in one image. The contents on output with 100% may look distorted. Ideally, spectacle has to capture every screen individually.
Comment 4 Vlad Zahorodnii 2024-01-15 21:38:37 UTC
Moving back to spectacle
Comment 5 Noah Davis 2024-01-16 02:56:18 UTC
(In reply to Vlad Zahorodnii from comment #3)
> If it takes a workspace screenshot, kwin will use the highest scale factor
> (125%) for the screenshot because two monitors will be captured in one
> image. The contents on output with 100% may look distorted. Ideally,
> spectacle has to capture every screen individually.

In order to make a rectangle screenshot, we have to combine different images into one. We originally captured each screen separately, but the result was the same as KWin's workspace screenshots, making it pointless to do it ourselves. Perhaps the scaling done by KWin to the image from the 1x scale display could be done more smoothly?
Comment 6 Bug Janitor Service 2024-01-17 23:31:00 UTC
A possibly relevant merge request was started @ https://invent.kde.org/plasma/kwin/-/merge_requests/4973
Comment 7 Vlad Zahorodnii 2024-01-17 23:33:57 UTC
This should make upscaled x1 content smoother
Comment 8 Vlad Zahorodnii 2024-01-18 14:56:36 UTC
Git commit f6d9b8a0b492e9519a4ecb9dc200e3de53a3b919 by Vlad Zahorodnii.
Committed on 18/01/2024 at 15:56.
Pushed by vladz into branch 'master'.

plugins/screenshot: Use SmoothPixmapTransform when stitching area screenshots

M  +1    -0    src/plugins/screenshot/screenshot.cpp

https://invent.kde.org/plasma/kwin/-/commit/f6d9b8a0b492e9519a4ecb9dc200e3de53a3b919
Comment 9 Bug Janitor Service 2024-01-18 15:00:13 UTC
A possibly relevant merge request was started @ https://invent.kde.org/plasma/kwin/-/merge_requests/4980
Comment 10 Vlad Zahorodnii 2024-01-18 17:33:40 UTC
Git commit 558c87c3594f058d940b50d7cdf7308695d631f7 by Vlad Zahorodnii.
Committed on 18/01/2024 at 18:25.
Pushed by vladz into branch 'Plasma/6.0'.

plugins/screenshot: Use SmoothPixmapTransform when stitching area screenshots


(cherry picked from commit f6d9b8a0b492e9519a4ecb9dc200e3de53a3b919)

M  +1    -0    src/plugins/screenshot/screenshot.cpp

https://invent.kde.org/plasma/kwin/-/commit/558c87c3594f058d940b50d7cdf7308695d631f7
Comment 11 Bug Janitor Service 2024-01-19 19:12:43 UTC
A possibly relevant merge request was started @ https://invent.kde.org/plasma/kwin/-/merge_requests/5004
Comment 12 Nate Graham 2024-03-08 00:03:35 UTC
*** Bug 482750 has been marked as a duplicate of this bug. ***
Comment 13 Nate Graham 2024-04-01 16:17:27 UTC
*** Bug 483816 has been marked as a duplicate of this bug. ***
Comment 14 Noah Davis 2024-04-07 04:37:52 UTC
Git commit 00c90e574ae93b146e703b8f5a7cb6db42fda465 by Noah Davis.
Committed on 07/04/2024 at 04:36.
Pushed by ndavis into branch 'master'.

ImagePlatformKWin: Go back to manually combining images

The point of this is to improve image quality with fractional scaling and mixed DPI.
The reason why we aren't just patching the KWin screenshot plugin is because CaptureWorkspace and CaptureArea aren't actually intended for high quality captures.

We are now using OpenCV to scale the images. We do this because OpenCV has higher quality filters and because OpenCV should be fast enough.

This patch should make it so integer scale screenshots are crisper.
Fractional screenshots are already a bit blurry, so this combined with a high quality image filter should be OK.

There's not much more that can be done to fix bug 478426 because of competing requirements.
A combined screens image should have the screens layout, but images need to be scaled to fit the layout and images also need to look as crisp as possible.

M  +119  -23   src/Platforms/ImagePlatformKWin.cpp
M  +43   -1    src/Platforms/ImagePlatformKWin.h

https://invent.kde.org/graphics/spectacle/-/commit/00c90e574ae93b146e703b8f5a7cb6db42fda465
Comment 15 Niklāvs Koļesņikovs 2024-04-07 13:14:17 UTC
Crisper screenshots are neat but why pick OpenCV? At least back when I worked with it about a decade ago, it was mostly a prototyping tool and a teaching tool for the classroom.

OpenCV even has a weird BGR internal format which, I note, is still the case in 2024, since the new code is using RGBA8888 format instead of ARGB32 in an attempt to work around that limitation (see: https://invent.kde.org/graphics/spectacle/-/commit/00c90e574ae93b146e703b8f5a7cb6db42fda465#52e7fd6a687cf1a70a640ed6accd701837a20b5c_385_435 ). It's also worth pointing out that Linux desktop is supposed to be working towards supporting higher bit depth for HDR purposes, so, yes, Spectacle will be encountering more than 8 bit color channels soon enough.

This choice is particularly weird since OpenCV is not known for its scalers, which is the domain of the libplacebo ( https://libplacebo.org/ ). Please reconsider?
Comment 16 Noah Davis 2024-04-08 20:28:21 UTC
(In reply to Niklāvs Koļesņikovs from comment #15)
> Crisper screenshots are neat but why pick OpenCV? At least back when I
> worked with it about a decade ago, it was mostly a prototyping tool and a
> teaching tool for the classroom.
> 
> OpenCV even has a weird BGR internal format which, I note, is still the case
> in 2024, since the new code is using RGBA8888 format instead of ARGB32 in an
> attempt to work around that limitation (see:
> https://invent.kde.org/graphics/spectacle/-/commit/
> 00c90e574ae93b146e703b8f5a7cb6db42fda465#52e7fd6a687cf1a70a640ed6accd701837a2
> 0b5c_385_435 ). It's also worth pointing out that Linux desktop is supposed
> to be working towards supporting higher bit depth for HDR purposes, so, yes,
> Spectacle will be encountering more than 8 bit color channels soon enough.
> 
> This choice is particularly weird since OpenCV is not known for its scalers,
> which is the domain of the libplacebo ( https://libplacebo.org/ ). Please
> reconsider?

I basically searched "c++ image processing libraries". OpenCV was among the top of the results.
I went with OpenCV because:
- It seemed to have healthy development (major).
- Documentation (API, examples and Q&A website answers) was fairly plentiful (major).
- It seemed to have lots of optimizations including hardware acceleration (major).
- I wanted a way to apply Gaussian blur to a QImage for another patch (major).
- Some other users asked for optical character recognition, which OpenCV has a module for (minor).
- Some other KDE projects already used it, so I wouldn't have to ask sysadmin to update the CI (minor).

LibVips was almost chosen and I even got it working, but:
- The documentation was not great and examples/answers for how to use it were not plentiful.
- The source code was hard to follow (GLib stuff with macros is harder than Qt/KDE code for me), so that makes documentation deficiencies harder to deal with.
- No OCR.
- No other KDE projects using it.

I didn't want to spend ages trying different libraries, so I went with OpenCV.

The BGR issue doesn't really apply as long as I use RGBA instead of ARGB. It's not as optimal as QImage::Format_ARGB32_Premultiplied for QPainter, but Qt still says it's among the most optimal formats for QPainter. Qt even uses QImage::Format_RGBA8888_Premultiplied for the QImage that QQuickPaintedItem uses internally. RGBA is more compatible with OpenGL, so that's probably why.

I'm aware of the growing relevance of HDR on Linux, but I don't have the expertise necessary to deal with that right now. There are probably alternative formats to Format_RGBA8888_Premultiplied that can be used with OpenCV without conversion, but that's a problem for someone™ in the future.

I'm not that sure about my choice, so I'm open to other options, but the documentation for libplacebo seems to be very lacking.
Comment 17 Niklāvs Koļesņikovs 2024-04-08 22:07:24 UTC
If the search query asked for C++, then, indeed, the list was probably short, since by far most such software is written in C, CUDA or maybe even Python with numpy, pytorch or similar framework for SIMD or GPU acceleration. However by asking for hardware acceleration, it's implied that the actual processing would be done by some kind of a shader, so all it would really take was attaching a GLSL or SPIR-V shader implementing the desired mathematical kernel, which is something Qt can already do. Although probably not directly with QImage, so it would likely need to be copied to QOpenGLTexture or a similar type and after processing copied back to QImage.

Regarding OCR, I'm quite certain that OpenCV uses Tesseract OCR ( https://github.com/tesseract-ocr/tesseract ) which is written in C++ and could be used directly. That being said, my gut feeling is that there's probably something better out there, just maybe not published yet, since the best I could quickly find was using one of the generative adversarial networks (GAN) for cleanup before feeding the processed image into a  convolution neural network (CNN) based OCR i.e. Tesseract. However GANs are quite amazing and I'd expect them to eventually replace CNNs for OCR purposes.

In short, there's nothing magical about OpenCV and the desired bits can either bit assembled from existing projects or directly implemented in Qt or KF, since there's probably more than just Spectacle that would be greatly improved with some graphical or compute shader based features (I'd certainly love either advanced scalers or ANN processing in Okular and Gwenview).
Comment 18 Noah Davis 2024-04-08 22:41:42 UTC
(In reply to Niklāvs Koļesņikovs from comment #17)
> If the search query asked for C++, then, indeed, the list was probably
> short, since by far most such software is written in C, CUDA or maybe even
> Python with numpy, pytorch or similar framework for SIMD or GPU
> acceleration. However by asking for hardware acceleration, it's implied that
> the actual processing would be done by some kind of a shader, so all it
> would really take was attaching a GLSL or SPIR-V shader implementing the
> desired mathematical kernel, which is something Qt can already do. Although
> probably not directly with QImage, so it would likely need to be copied to
> QOpenGLTexture or a similar type and after processing copied back to QImage.
> 
> Regarding OCR, I'm quite certain that OpenCV uses Tesseract OCR (
> https://github.com/tesseract-ocr/tesseract ) which is written in C++ and
> could be used directly. That being said, my gut feeling is that there's
> probably something better out there, just maybe not published yet, since the
> best I could quickly find was using one of the generative adversarial
> networks (GAN) for cleanup before feeding the processed image into a 
> convolution neural network (CNN) based OCR i.e. Tesseract. However GANs are
> quite amazing and I'd expect them to eventually replace CNNs for OCR
> purposes.
> 
> In short, there's nothing magical about OpenCV and the desired bits can
> either bit assembled from existing projects or directly implemented in Qt or
> KF, since there's probably more than just Spectacle that would be greatly
> improved with some graphical or compute shader based features (I'd certainly
> love either advanced scalers or ANN processing in Okular and Gwenview).

Do you know of any image processing libraries besides OpenCV that have good documentation? If I was determined, I could figure out how to use most well made libraries eventually, but I want to get this over with as quickly as possible so that I can move on to other things. I'm currently not interested in trying anything that isn't fairly mature and widely used.

Another library I looked at briefly was CImg, but their documentation website doesn't even seem to work.
Comment 19 Niklāvs Koļesņikovs 2024-04-09 04:03:28 UTC
I'm pretty sure that the nice documentation is the computer graphics book that CImg developers want their users to buy. However, being 2000' academic software, it does have the Java style API documentation that was all the rage back in those days. The main link is slightly broken, however by clicking around a working one can be obtained (this one should lead straight to the resize function that allows picking the kernel: https://cimg.eu/reference/structcimg__library_1_1CImg.html#a6a668c8b3f9d756264d1fb31b7a915fc ).

There's also examples ( https://github.com/GreycLab/CImg/tree/master/examples ) to see how the CImg API comes together. That being said, I'm not aware of CImg having any kind of GPU acceleration and any CPU acceleration or optimization is likely handled by the optional `-Dcimg_use_openmp -Dcimg_use_lapack` which will pull in OpenMP and LAPACK respectively. Neither should be as much of a hassle as OpenCV, so an improvement overall.

When I did play around with C++ graphics on my own time, I always used C libraries wrapped in custom RAII C++ code. I have not used it myself but there is Intel's oneAPI ( https://spec.oneapi.io/oneipl/0.6/transform/resize_lanczos.html ) and specifically its DPC++ language/compiler ( https://intel.github.io/llvm-docs/GetStartedGuide.html ). I would not expect it to actually use GPU acceleration on the average Linux system right now, however the same might actually be true with OpenCV as well, because all my searching indicates it only supports CUDA for GPU acceleration.

Overall, maybe best to just lower the expectations and pick something CPU based like CImg and open a feature request for KF6 or Qt6 to implement better scaling methods (and ideally ability to use libplacebo compatible shaders, because there's quite a large selection of awesome shaders for mpv).
Comment 20 Noah Davis 2024-04-09 04:13:50 UTC
Other than the order of color channels being sometimes inconvenient, what's actually wrong with OpenCV? It's not clear why I should switch.
Comment 21 Niklāvs Koļesņikovs 2024-04-09 15:17:47 UTC
Above all else, I just do not see the use case for computer vision here. The requirement is for a nice scaler function, so there's plenty of choices available: Lanczos (sharp), Gaussian (smooth), spline16 (comparatively fast and for a lack of better term neutral). The list goes on but traditional scalers consist of either a single or multiple convolutions and often can be optimized into a single function that might fit on a single line of code. There should be suck optimized functions in https://code.videolan.org/videolan/libplacebo/-/blob/master/src/filters.c?ref_type=heads that probably can be copy, pasted and modified with a copyright notice, since ultimately it's just math.

OpenCV is convenient but it is also weird in using the BGR format and depending on distro choices OpenCV packages may contain externally contributed code. On my system it also pulls in half a dozen new dependencies, including the same LAPACK that CImg optionally uses. Requiring something as large as OpenCV just for a scaler function strikes me as suboptimal. And the Tesseract OCR can be integrated directly on the account of it being C++.
Comment 22 Nate Graham 2024-04-09 16:34:41 UTC
I think this is probably a "patches welcome" situation. You seem quite knowledgeable here; if you think you've got a better way to do it, and you have both the technical knowledge and programming chops to pull it off, feel free to submit a patch that shows the benefits of your proposed approach.

Hopefully those benefits would include:
- Less code in Spectacle
- Better performance or lower resource usage in Spectacle
- Using fewer 3rd-party dependencies
- Switching to 3rd-party dependencies that are better supported, developed, documented, and audited, and have better longevity and long-term prospects

I would expect to see at least one of those, ideally 2 or more.
Comment 23 Noah Davis 2024-04-09 16:36:45 UTC
I don't really have time to try out a new library, so switching out OpenCV is a low priority for me right now. You can make a bug report about this if you want. If you have the time and ability to make a patch, I'll review it for you.
Comment 24 Niklāvs Koļesņikovs 2024-04-09 21:00:21 UTC
Fair enough. I expected as much. I might have a go but typically the KDE programming style is very different from how I write C++, so when it comes to contributing to KDE, I might as well be a complete novice that couldn't correctly write a hello world GUI. Most likely I'll just swap out Spectacle for some other Wayland compatible screenshot snapper or just use the already working OBS. Cheers.
Comment 25 Nate Graham 2024-04-10 17:56:41 UTC
*** Bug 483883 has been marked as a duplicate of this bug. ***
Comment 26 Nate Graham 2024-04-11 01:57:50 UTC
*** Bug 484003 has been marked as a duplicate of this bug. ***