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

[Feature request] Use left_ptr_watch for a startup notification #3737

Open
akinokonomi opened this issue Nov 14, 2022 · 12 comments
Open

[Feature request] Use left_ptr_watch for a startup notification #3737

akinokonomi opened this issue Nov 14, 2022 · 12 comments

Comments

@akinokonomi
Copy link

akinokonomi commented Nov 14, 2022

Output of awesome --version:
awesome v4.3-1551-g5077c8381-dirty (Too long)

Hello!

The reason for this is that I use Fuchsia cursor theme, and here is what watch vs. left_ptr_watch look like:

image

The latter one looks much more seamless to me.

I tried to patch it myself to submit a PR:

Click here
diff --git a/common/xcursor.c b/common/xcursor.c
index c4c691df5..356d3c68e 100644
--- a/common/xcursor.c
+++ b/common/xcursor.c
@@ -101,6 +101,7 @@ static char const * const xcursor_font[] =
     [XC_ul_angle] = "ul_angle",
     [XC_umbrella] = "umbrella",
     [XC_ur_angle] = "ur_angle",
+    [XC_left_ptr] = "left_ptr_watch",
     [XC_watch] = "watch",
     [XC_xterm] = "xterm",
 };
diff --git a/lib/awful/startup_notification.lua b/lib/awful/startup_notification.lua
index 9bc95236c..45ad9c639 100644
--- a/lib/awful/startup_notification.lua
+++ b/lib/awful/startup_notification.lua
@@ -18,7 +18,7 @@ local beautiful = require("beautiful")
 
 local app_starting = {}
 
-local cursor_waiting = "watch"
+local cursor_waiting = "left_ptr_watch"
 
 --- Show busy mouse cursor during spawn.
 -- @beautiful beautiful.enable_spawn_cursor

But nothing came out of it, because XC_left_ptr is a duplicate entry, and my knowledge is not enough to go any deeper.

Just for reference, Openbox uses left_ptr_watch as well.

Thanks!

@Elv13
Copy link
Member

Elv13 commented Nov 14, 2022

Use "left_ptr" without the C patch? XC_left_ptr is already handled by xcursor.c. There is no such thing as a left watch in the X11 enum.

@akinokonomi
Copy link
Author

Use "left_ptr" without the C patch?

That's what I did at first, but it does not work. It would still show just the watch cursor (and would never remove it even after app was started).

It actually shows left_ptr_watch after I patch the C file, but messes up everything else.

@Elv13
Copy link
Member

Elv13 commented Nov 14, 2022

But "left_ptr_watch" in your patch is XC_left_ptr. It has nothing to do with the watch. There is no such thing as a left watch as far as I know.

@akinokonomi
Copy link
Author

akinokonomi commented Nov 14, 2022

But "left_ptr_watch" in your patch is XC_left_ptr. It has nothing to do with the watch.

I used Openbox code as a reference, here both of them are XC_left_ptr:

$ rg -p "left_ptr" -uuu *
openbox.c
203:    cursors[OB_CURSOR_POINTER] = load_cursor("left_ptr", XC_left_ptr);
204:    cursors[OB_CURSOR_BUSYPOINTER] = load_cursor("left_ptr_watch",XC_left_ptr);

There is no such thing as a left watch as far as I know.

No, there is not.

@Elv13
Copy link
Member

Elv13 commented Nov 14, 2022

So "left_ptr" == "left_ptr_watch". You don't need a C patch, just use "left_ptr" and be done with it?

@akinokonomi
Copy link
Author

akinokonomi commented Nov 14, 2022

just use "left_ptr" and be done with it?

I'm not sure I understand.
If I place left_ptr_watch in startup_notification.lua, it does not wok.

EDIT: Oh, if you mean replace "watch" with just "left_ptr", well, it's not that "watch" cursor annoys me that much, so I rather just leave it alone.

@Elv13
Copy link
Member

Elv13 commented Nov 14, 2022

If I place left_ptr_watch in startup_notification.lua, it does not wok.

left_ptr_watch doesn't exist, use left_ptr. left_ptr_watch seems like an OpenBox invention and/or a legacy compatibility hack for a typo. We wont add an alias for a cursor name to awesome because OpenBox decided to do so. The cursor is called "left_ptr", nor "left_ptr_watch".

@akinokonomi
Copy link
Author

akinokonomi commented Nov 14, 2022

left_ptr_watch seems like an OpenBox invention

We wont add an alias for a cursor name to awesome because OpenBox decided to do so.

Then why does so many (all?) cursor themes have the file named left_ptr_watch?

If it's just an alias, why does Openbox show the cursor on right, not the one on the left, as a startup notification (see screenshot)?
EDIT: Neither does it show plain left_ptr, which looks like this:

image

$ pacman -Ql oxygen | grep left_ptr_watch      
oxygen /usr/share/icons/KDE_Classic/cursors/left_ptr_watch
oxygen /usr/share/icons/Oxygen_Black/cursors/left_ptr_watch
oxygen /usr/share/icons/Oxygen_Blue/cursors/left_ptr_watch
oxygen /usr/share/icons/Oxygen_White/cursors/left_ptr_watch
oxygen /usr/share/icons/Oxygen_Yellow/cursors/left_ptr_watch
oxygen /usr/share/icons/Oxygen_Zion/cursors/left_ptr_watch

$ pacman -Ql breeze | grep left_ptr_watch      
breeze /usr/share/icons/Breeze_Snow/cursors/left_ptr_watch
breeze /usr/share/icons/breeze_cursors/cursors/left_ptr_watch

$ pacman -Ql xcursor-themes | grep left_ptr_watch 
xcursor-themes /usr/share/icons/handhelds/cursors/left_ptr_watch
xcursor-themes /usr/share/icons/redglass/cursors/left_ptr_watch
xcursor-themes /usr/share/icons/whiteglass/cursors/left_ptr_watch

$ pacman -Ql capitaine-cursors | grep left_ptr_watch 
capitaine-cursors /usr/share/icons/capitaine-cursors-light/cursors/left_ptr_watch
capitaine-cursors /usr/share/icons/capitaine-cursors/cursors/left_ptr_watch

$ pacman -Ql fuchsia-cursor | grep left_ptr_watch
fuchsia-cursor-theme-bin /usr/share/icons/Fuchsia-Pop/cursors/left_ptr_watch
fuchsia-cursor-theme-bin /usr/share/icons/Fuchsia-Red/cursors/left_ptr_watch
fuchsia-cursor-theme-bin /usr/share/icons/Fuchsia/cursors/left_ptr_watch

left_ptr_watch doesn't exist, use left_ptr.

I'm clearly missing something. How do you suggest to use it?

@psychon
Copy link
Member

psychon commented Dec 3, 2022

But nothing came out of it, because XC_left_ptr is a duplicate entry, and my knowledge is not enough to go any deeper.

What exactly do you mean? What does it duplicate? I looked at /usr/include/X11/cursorfont.h and found #define XC_left_ptr 68. Nothing else in there has value 68, so I would actually expect your patch to work just fine...

Looking at the code in AwesomeWM, "everything" basically only ever does xcursor_new(bla, xcursor_font_fromstr(something)). xcursor_font_tostr is completely unused (except one internal call). So... this intermediate layer that assigns numbers should be unnecessary and dropable. I guess this is just a left-over from before the port to xcb-util-cursor.

The only complication would be that some other caching mechanism than the current array would be needed, but that seems doable. Some data structure that maps a string to xcb_cursor_t can surely be implemented with the existing DO_BARRAY mechanism.

@akinokonomi
Copy link
Author

akinokonomi commented Dec 3, 2022

What exactly do you mean? What does it duplicate? I looked at /usr/include/X11/cursorfont.h and found #define XC_left_ptr 68.

@psychon The XC_left_ptr in static char const * const xcursor_font[] = {} is a duplicate, so its value gets overwritten.

The

    [XC_left_ptr] = "left_ptr",

already exists, so it left_ptr gets overwritten with left_ptr_watch.

    [XC_left_ptr] = "left_ptr_watch",

So, in theory, instead of plain left_ptr, awesome should show left_ptr_watch, but in practice no, it still shows plain left_ptr sometimes too. And does not hide left_ptr_watch after app window appears.
Maybe because I have xsetroot -cursor_name left_ptr & on autostart. No idea.

EDIT: It also breaks some other things, like hovering panel gets you a diagonal cross - [XC_X_cursor] = "X_cursor".

I think to fix this the table should be rewritten (if it's a table?), so that some other custom names would be assigned to both
XC_left_ptr + left_ptr_watch and both XC_left_ptr + left_ptr. But how to do that is more than I can say.

You can see that it's exactly what they did in openbox #3737 (comment): assigned custom name OB_CURSOR_BUSYPOINTER and OB_CURSOR_POINTER to those values.


Here's the warning at a build time:

...
[ 15%] Building C object CMakeFiles/awesome.dir/common/version.c.o
[ 15%] Generating raw_images/AUTOGEN_wibox_widget_piechart_border_width.svg
[ 15%] Building C object CMakeFiles/awesome.dir/common/xcursor.c.o
/home/user/awesome/src/build/common/xcursor.c:104:21: warning: initialized field overwritten [-Woverride-init]
  104 |     [XC_left_ptr] = "left_ptr_watch",
      |                     ^~~~~~~~~~~~~~~~
/home/user/awesome/src/build/common/xcursor.c:104:21: note: (near initialization for ‘xcursor_font[68]’)
...

@psychon
Copy link
Member

psychon commented Dec 4, 2022

Ah, thanks & sorry. Somehow I was thinking in quite a wrong direction here...

@akinokonomi
Copy link
Author

Sure, no problem.

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

No branches or pull requests

3 participants