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

grimblast: add environment var 'GRIMBLAST_HIDE_CURSOR' to avoid high CPU loads on some machines #136

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

Conversation

DHDcc
Copy link
Contributor

@DHDcc DHDcc commented Mar 3, 2025

Description of changes

fixes #134

The grim cursor is not visible by default, but if your machine has high CPU loads you can add env = GRIMBLAST_HIDE_CURSOR, 0 to your Hyprland config file (the grim cursor will be visible).

Things done

  • For new programs

    • Add the program to the README table, with yourself as the maintainer
    • Add a README for the program itself
    • Add Makefile (and Nix derivation optionally, otherwise @fufexan will do it)
    • If the program is a script, add it to the CI checks matrix
  • For changes

    • Add changes to the CHANGELOG
    • Reflect your changes in the man pages (if they exist)

@DHDcc
Copy link
Contributor Author

DHDcc commented Mar 3, 2025

@Atemo-C @Gigas002 can you two try this?

@Atemo-C
Copy link

Atemo-C commented Mar 4, 2025

image

The screenshot cursor is back.

@DHDcc
Copy link
Contributor Author

DHDcc commented Mar 4, 2025

image

The screenshot cursor is back.

@Atemo-C, you need to add env = GRIMBLAST_HIDE_CURSOR, 1 in your hyprland config file or export GRIMBLAST_HIDE_CURSOR=1 in your uwsm env file.

@Atemo-C
Copy link

Atemo-C commented Mar 4, 2025

Did you add "env GRIMBLAST_HIDE_CURSOR, 1" in your hyprland config file or your uwsm env file?

Oh my hyprgod, I am actually blind… No, I did not. Adding it fixes it, and it works now. However, I now cannot take an area screenshot of my second monitor, with the following error:

grimblast: line 171: kill: (148500) - No such process

And the following notification:
image

This only happens when taking a screenshot that is only or includes part of my second monitor. Taking screenshots on my main one works fine, and the cursor does not appear when it should not.

@DHDcc
Copy link
Contributor Author

DHDcc commented Mar 4, 2025

Did you add "env GRIMBLAST_HIDE_CURSOR, 1" in your hyprland config file or your uwsm env file?

Oh my hyprgod, I am actually blind… No, I did not. Adding it fixes it, and it works now. However, I now cannot take an area screenshot of my second monitor, with the following error:

grimblast: line 171: kill: (148500) - No such process

And the following notification: image

This only happens when taking a screenshot that is only or includes part of my second monitor. Taking screenshots on my main one works fine, and the cursor does not appear when it should not.

yeah, I have the same issue on my main monitor but I don't think it's caused by this patch (I can still take screenshots) :

+ killHyprpicker
+ '[' '!' 173171 -eq -1 ']'
+ kill 173171
./grimblast: line 171: kill: (173171) - No such process
+ rm -f /run/user/1000/grimblast.lock

@Atemo-C
Copy link

Atemo-C commented Mar 4, 2025

yeah, I have the same issue but I don't think it's caused by this patch (I can still take screenshots) :

+ killHyprpicker
+ '[' '!' 173171 -eq -1 ']'
+ kill 173171
./grimblast: line 171: kill: (173171) - No such process
+ rm -f /run/user/1000/grimblast.lock

Strange… When using Grimblast from the unstable pkgs in NixOS, or the previously patched version with the fake display workaround that was too CPU intensive, it worked just fine on my second monitor. I will have to do some more testing.

Edit: Actually, the problem does go back to the previous patches with the fake display workaround, I must have missed it in testing earlier.

@Atemo-C
Copy link

Atemo-C commented Mar 4, 2025

51f3cf5

This is when the issue started appearing, and it is still here, though, again, only on my second monitor.

@DHDcc
Copy link
Contributor Author

DHDcc commented Mar 4, 2025

51f3cf5

This is when the issue started appearing, and it is still here, though, again, only on my second monitor.

It started before 51f3cf5:
image

edit: the bad commit actually stated at f4bea56

@Atemo-C
Copy link

Atemo-C commented Mar 4, 2025

edit: the bad commit actually stated at f4bea56

Oh wow, goes bad quite a bit further than I thought.

@Gigas002
Copy link

Gigas002 commented Mar 4, 2025

The env variable works for me, but as @Atemo-C noted the cursor is visible when it's set

@DHDcc
Copy link
Contributor Author

DHDcc commented Mar 4, 2025

Is it better if the cursor is not visible by default @Atemo-C @Gigas002 ?

@Atemo-C
Copy link

Atemo-C commented Mar 4, 2025

Is it better if the cursor is not visible by default @Atemo-C @Gigas002 ?

I see no reason for the cursor to be visible by default unless --cursor is used.

@Atemo-C
Copy link

Atemo-C commented Mar 4, 2025

With the latest changes, the error still appears when taking screenshots of my second monitor; However, the screenshot is actually taken and saved/copied properly, so the error saying it cannot invoke grim seems erroneous.

Example here of a screenshot spanning both my monitors.

image

@DHDcc
Copy link
Contributor Author

DHDcc commented Mar 4, 2025

With the latest changes, the error still appears when taking screenshots of my second monitor; However, the screenshot is actually taken and saved/copied properly, so the error saying it cannot invoke grim seems erroneous.

Example here of a screenshot spanning both my monitors.

image

The bad commit wasn't me so I can't fix this issue for now. You can create a new issue for that.

@DHDcc
Copy link
Contributor Author

DHDcc commented Mar 4, 2025

@fufexan

@Atemo-C
Copy link

Atemo-C commented Mar 4, 2025

The bad commit wasn't me so I can't fix this issue for now. You can create a new issue for that.

I created it here, includes a possible fix, too. With all of this put together, every single function now works as expected, nothing to say.

Copy link
Member

@fufexan fufexan left a comment

Choose a reason for hiding this comment

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

Looks good, but we should also add an entry in the manual and in the help section, informing users that if they get double cursors when screenshotting area, they should set the env var.

@DHDcc
Copy link
Contributor Author

DHDcc commented Mar 6, 2025

if they get double cursors when screenshotting

I changed the logic so that by default, the cursor is not visible and if you have high CPU loads, you can add env = GRIMBLAST_HIDE_CURSOR, 0.

@DHDcc
Copy link
Contributor Author

DHDcc commented Mar 6, 2025

It should be good now.

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.

Regression in screenshooting tool causes high CPU loads
4 participants