Skip to content

Conversation

@berkentekin
Copy link

@berkentekin berkentekin commented May 13, 2024

  • This PR adds an option to hide cursor for screenshots taken in DEs other than GNOME and KDE. The checkbox is therefore disabled in GNOME, KDE and non-UNIX systems.
  • In screenGrabber.cpp, the application decides to use either a DBus interface or the wlroots-based grim program depending on the desktop environment. Unfortunately, from what I understand, KDE and GNOME do not support wlr protocols, and the DBus interface does not provide an option to show or hide the cursor. Therefore, my changes (should) only apply to DEs that can utilize grim.
  • grim includes the cursor in screenshots with -c argument. This PR basically just allows passing this option.
  • The cursor is hidden by default.
  • For now, no CLI options to set this behaviour. If requested, I can mark this PR as WIP and work on that.

Partially resolves #3582, #604

GRIM provides a -c option to display cursor in screenshots. I added a new option to make us of this functionality. However, technically this option needs to be hidden/disabled under GNOME and KDE. I may address this in a later commit.
@berkentekin berkentekin changed the title New option to hide cursor for wlroots-based grim New option to hide cursor for DEs other than GNOME and KDE May 13, 2024
@mmahmoudian
Copy link
Member

mmahmoudian commented May 14, 2024

Thanks for the PR. I believe the correct course UX is to have only and only one checkbox that works universally on all DEs and OSs. What I mean is that the user should not care which feature they should toggle based on what software stack they are using. So if we cannot turn off cursor in Gnome Wayland or KDE or Windows, we should somehow show a message to the user and inform them why the checkbox is disabled.

In this implementation the Windows, Mac, and BSD users will just get a disabled checkbox without any explanation, and the Gnome and KDE users will get different tooltip.

I believe it is better if

  • we can show a red text above this checkbox to notify the user
  • include Mac and BSD in the mix by reversing the if statement condition to if linux -> if wayland -> if not gnome or not kde -> ...

This PR also addresses #604 in a way (giving control over showing cursor in Wayland)

@mmahmoudian
Copy link
Member

mmahmoudian commented May 14, 2024

Aesthetically I like it, but the point is that users rarely check the tooltip for more info. How about adding a ℹ️ or something similar at the end of such items that kind of invite/attract the user to click/hover to get the reason on why this is red.

Btw, thanks for your efforts and responsiveness 👍

Now red text will show above the cursor hiding option if the desktop environment does not support setting the behaviour
@berkentekin
Copy link
Author

berkentekin commented May 14, 2024

@mmahmoudian Thank you for your kind words.

On second thought I think adding a text is a better idea precisely because of the reason you've mentioned. I just deleted my comment because my design was such a minor difference that I didn't think it'd be worth mentioning... Besides, whether an unsupported environment actually grabs the cursor or not is outside our control, so informing the user about the issue on first sight resolves the ambiguity of a disabled-but-checked box.

In the end this is what it looks like:

Screenshot_20240514_190908

if (!m_info.waylandDetected()) {
initDisabledHideCursor();
} else {
switch (m_info.windowManager()) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might better to "opt in" desktops where we KNOW this feature will work, than just to opt out KDE and GNOME. We have people using basically every esoteric DE there is.

@borgmanJeremy borgmanJeremy self-assigned this May 14, 2025
@oficsu
Copy link

oficsu commented Nov 26, 2025

@berkentekin

Unfortunately, from what I understand, KDE and GNOME do not support wlr protocols, and the DBus interface does not provide an option to show or hide the cursor

Currently, Spectacle (default KDE screenshot tool) has the checkbox to include/exclude cursor. It uses the following D-Bus API:

method call ... -> destination=org.kde.KWin.ScreenShot2 path=/org/kde/KWin/ScreenShot2; interface=org.kde.KWin.ScreenShot2; member=CaptureScreen
   string "eDP-1"
   array [
      dict entry(
         string "include-cursor"
         variant             boolean true
      )
      dict entry(
         string "include-decoration"
         variant             boolean true
      )
      dict entry(
         string "include-shadow"
         variant             boolean true
      )
      dict entry(
         string "native-resolution"
         variant             boolean true
      )
   ]
   ...

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.

Add an option to hide cursor on screenshots

4 participants