[hide-cursor] add disabled_for option by vdegenne · Pull Request #344 · WayfireWM/wayfire-plugins-extra

vdegenne · 2026-08-31T12:00:11Z

Changes in this PR:

An option to prevent the cursor hiding in certain contexts.
Added reevaluation on workspace switch.
small refactoring.

Note: It's worth mentioning I don't have a good knowledge of C++ and I used ChatGPT as an assistant. Though the changes seems solid.

soreau · 2026-08-31T12:56:47Z

Thanks for the patch. At first glance, I see some code duplication we can wrap up but it looks good overall. But, since the toggle option is broken, I'd like to fix that first. I will do a bit of testing on the toggle binding and report back hopefully soon.

soreau · 2026-08-31T13:03:16Z

I tested toggle hide cursor and it works ok here.

soreau

The basic idea sounds good, but it could be restructured a bit, see review comments.

soreau · 2026-08-31T13:04:18Z

+        wf::get_core().connect(&workspace_changed);
+    }
+
+    void reevaluate()


I feel like this should have a bool argument for the last conditional in the function, to avoid code duplication (see next comment).

soreau · 2026-08-31T13:07:51Z

        [=] (wf::input_event_signal<wlr_pointer_motion_event> *ev)
    {
-        setup_hide_timer();
+        auto view = wf::get_core().get_cursor_focus_view();


Lines 77-95 is code duplicated in the reevaluate function above. You should be able to call reevaluate here with the added bool argument instead of 77-95, to hide or unhide in the case the window doesn't match disabled_for. You will have to modify the reevaluate function to handle the bool argument accordingly.

soreau · 2026-08-31T13:09:54Z

+        setup_hide_timer();
+    };
+
+    wf::signal::connection_t<wf::workspace_changed_signal> workspace_changed =


I feel this addition should be in a separate patch since it's new behavior. Ideally, you would have a patch that adds the option to xml and the bulk of the code here, and another that adds calling reevaluate on workspace_changed. This way, we can merge instead of squash it so it will be easier to bisect in case there is a bug.

soreau · 2026-08-31T13:17:04Z

I think we should try to find out why the toggle binding doesn't work for you, but it would be easier if you joined chat to avoid flooding github with too many messages.

vdegenne · 2026-08-31T13:24:59Z

I tested toggle hide cursor and it works ok here.

Yeah sorry I thought the option was to toggle the plugin itself.

vdegenne · 2026-08-31T15:38:04Z

@soreau Ok, I removed the workspace switch event, that means the user always need to move the mouse to make the cursor reappear (which is fine.)

If later we want to get it back we could just use hide_or_show_cursor

I built and installed and tested, works nicely for now.

vdegenne · 2026-08-31T16:30:56Z

Sorry for all the changes, I learn a little bit of C++ along the way.

soreau · 2026-08-31T16:41:22Z

@soreau Ok, I removed the workspace switch event, that means the user always need to move the mouse to make the cursor reappear (which is fine.)

If later we want to get it back we could just use hide_or_show_cursor

Sorry if I wasn't clear before, but I didn't mean that we should remove the workspace switch event. I meant that this PR ideally should have only two commits/patches: One that adds the option in xml and cpp, then another patch that introduces the workspace switch event handling. However, the latter could go into another PR since it's a new idea, but either is fine with me.

soreau · 2026-08-31T16:43:34Z

Sorry for all the changes, I learn a little bit of C++ along the way.

No worries, it's always a learning process. However, you can always consolidate your changes into one or two commits/patches and force-push your branch.

soreau

You really shaped it up, I think it looks much better now. Just one last thing in the comment below.

Let me know if you want to add workspace switch back, then we can run the CI and merge this.

soreau · 2026-08-31T16:58:49Z

    wf::signal::connection_t<wf::input_event_signal<wlr_pointer_motion_event>> pointer_motion =
        [=] (wf::input_event_signal<wlr_pointer_motion_event> *ev)
    {
-        setup_hide_timer();


When mouse motion happens, we need to reset the timer, so the cursor hides after moving the mouse.

it's at the bottom of the function yes. Because show_cursor disconnects the timer now, so it can't be placed before.

soreau · 2026-08-31T17:00:55Z

+
+    void show_cursor()
+    {
+        hide_timer.disconnect();


It almost seems like we shouldn't call hide_timer.disconnect() here because when we show the cursor, we want it to timeout and ultimately hide it again. I think you want to disconnect the timer instead in toggle_cursor for the hidden case where show_cursor is called.

I think it's fine because "show cursor" should explicitly show the cursor and should prevent any previously set timer to hide it again. The timer is set again on further mouse move (or any other event we might add in the future). Also it works well when turning the plugin off or for the toggle logic.

vdegenne · 2026-08-31T18:57:55Z

You really shaped it up, I think it looks much better now. Just one last thing in the comment below.

Let me know if you want to add workspace switch back, then we can run the CI and merge this.

I am not sure why but this

wf::signal::connection_t<wf::workspace_changed_signal> workspace_changed = 
        [=] (wf::workspace_changed_signal *ev)
    {
      ...
    }

didn't work well last time I tested (when using "viewport switcher" keybinds). Is there a specific protocol to enable?

Also we should think of the best strategy here, do we use same logic as mouse motion,

if (hidden)
{
    show_cursor();
}

restart_hide_timer();

or do we call hide_or_show_cursor directly?

The latter seems preferable but we have to make sure the transition is completed before calling anything or else this new condition if (view && disabled_for.matches(view)) would apply to the view before the transition.

If it's ok for now maybe you can merge this, so we can work clean on a new PR for workspace event

vdegenne · 2026-08-31T20:17:55Z

I added workspace_changed event because it's needed to avoid calling wf::get_core().unhide_cursor(); for instance when switching to a fullscreen app using keyboard that needs to capture the pointer (e.g. a game), or else the pointer will show up on top of the game forever as soon as we move it again. That's why the cursor needs to show up again before the switch.

However the event doesn't seem to be called on my end 😢 when using viewport switcher plugin. Is it expected?

vdegenne · 2026-08-31T20:39:09Z

Ok fixed. Just had to move the event listener in the plugin class where output object is available. Again I did that half blind because I used AI to help but it seems to work fine.
What do you think?

Edit: After 2 days of usage everything works smoothly, not a single issue.

@soreau

soreau · 2026-09-02T12:52:09Z

Ok fixed. Just had to move the event listener in the plugin class where output object is available. Again I did that half blind because I used AI to help but it seems to work fine. What do you think?

To be honest, I liked the code better when I approved the changes. The patch seems kinda sloppy to me now, but I will have to give it another review later I suppose.

vdegenne · 2026-09-02T13:04:16Z

@soreau Why would adding workspace change event be a bad thing?
Afaik it helps locating the cursor back when you change the viewport but most importantly (and that's the primary reason why I added this change) it helps fullscreen apps claiming the cursor visibility back without relying on wayfire core.

soreau · 2026-09-02T13:08:52Z

@soreau Why would adding workspace change event be a bad thing?

I am not saying that it is a bad thing, in fact we want this to be functional. I am saying that in the code, the only thing that should have happened is the addition of workspace change handler with the hide_or_show_cursor call. It seems that there were a lot more changes though.

vdegenne · 2026-09-02T13:21:57Z

Here's the changes since your last review:

adding guards in show_cursor, hide_cursor (just small safety)
centralized shared code into on_event called by both the mouse event and workspace change event (public so it can be accessed by global_idle).
I moved the workspace change event into the plugin class where the output object is available.
I made sure the event is disconnected when the plugin turns off.

So really, I just added workspace change event listener since last review, the rest is just small tweaks, refactoring, making things coherent.

vdegenne added 3 commits August 31, 2026 13:50

add option in xml

1d5a040

add disabled_for option

7cb1227

preserve original formatting

8525fde

soreau requested changes Aug 31, 2026

View reviewed changes

vdegenne added 4 commits August 31, 2026 15:18

unify duplicate code

ca677b3

accept "none" value

050017a

none is the default

b18c52d

none is supported by default

b3144ec

vdegenne added 4 commits August 31, 2026 16:01

complies to xml

d17ca3b

fix xml

98ea1c4

fix debouncer

cfec9ee

simplify code + remove workspace switch event

16730d9

vdegenne added 4 commits August 31, 2026 17:54

mouse functions

1284c39

guard show_cursor on mouse motion

cf55cf3

bracket return

37064f2

move mouse functions in wayfire_hide_cursor class

9546d79

soreau approved these changes Aug 31, 2026

View reviewed changes

Comment thread src/hide-cursor.cpp

soreau reviewed Aug 31, 2026

View reviewed changes

disconnect timer when hiding cursor

6892f67

add hidden guards

2739a62

added workspace_changed event

b3cbada

move workspace change event in plugin class

dd6d4e2

remove unwanted comments

cbd89c4

remove unecessary fstream

c41c685

Conversation

vdegenne commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

soreau commented Aug 31, 2026

Uh oh!

soreau commented Aug 31, 2026

Uh oh!

soreau left a comment

Choose a reason for hiding this comment

Uh oh!

soreau Aug 31, 2026

Choose a reason for hiding this comment

Uh oh!

soreau Aug 31, 2026

Choose a reason for hiding this comment

Uh oh!

soreau Aug 31, 2026

Choose a reason for hiding this comment

Uh oh!

soreau commented Aug 31, 2026

Uh oh!

vdegenne commented Aug 31, 2026

Uh oh!

vdegenne commented Aug 31, 2026

Uh oh!

vdegenne commented Aug 31, 2026

Uh oh!

soreau commented Aug 31, 2026

Uh oh!

soreau commented Aug 31, 2026

Uh oh!

soreau left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

soreau Aug 31, 2026

Choose a reason for hiding this comment

Uh oh!

vdegenne Aug 31, 2026

Choose a reason for hiding this comment

Uh oh!

soreau Aug 31, 2026

Choose a reason for hiding this comment

Uh oh!

vdegenne Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vdegenne commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vdegenne commented Aug 31, 2026

Uh oh!

vdegenne commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

soreau commented Sep 2, 2026

Uh oh!

vdegenne commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

soreau commented Sep 2, 2026

Uh oh!

vdegenne commented Sep 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

vdegenne commented Aug 31, 2026 •

edited

Loading

vdegenne Aug 31, 2026 •

edited

Loading

vdegenne commented Aug 31, 2026 •

edited

Loading

vdegenne commented Aug 31, 2026 •

edited

Loading

vdegenne commented Sep 2, 2026 •

edited

Loading