-
Notifications
You must be signed in to change notification settings - Fork 598
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
Rewrite the xcursor caching code #3745
base: master
Are you sure you want to change the base?
Conversation
Previously, the cache for xcursors consisted of a static array. We had a hardcoded list of supported cursor names and each one got assigned a position in this array. This hardcoded list means that one cannot simply use whatever cursor a cursor theme happens to provide, since the name needs to be in our list. In this commit, the code is rewritten. Instead of a hardcoded list, a sorted array is now used as the cache, as provided by the BARRAY code from common/util.h. The array is sorted by the name of the cursor. This change implies an API change for the xcursor code. Previously, one had to first translate a cursor name into a cache index (xcursor_font_fromstr()) and could then use this to actually get the cursor (xcursor_new()). With this commit, xcursor_new() is the only function provided by the cursor code and it directly gets a string as argument. Signed-off-by: Uli Schlachter <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #3745 +/- ##
=======================================
Coverage 90.99% 90.99%
=======================================
Files 900 900
Lines 57499 57498 -1
=======================================
Hits 52319 52319
+ Misses 5180 5179 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
I had an idea! diff --git a/awesomerc.lua b/awesomerc.lua
index c598a3e08..a5cad2eed 100644
--- a/awesomerc.lua
+++ b/awesomerc.lua
@@ -191,6 +191,7 @@ screen.connect_signal("request::desktop_decoration", function(s)
position = "top",
screen = s,
-- @DOC_SETUP_WIDGETS@
+ cursor = "foobar",
widget = {
layout = wibox.layout.align.horizontal,
{ -- Left widgets results in
Checking Will investigate the above X11 errors and report back. Edit: Excerpt from
Looks a bit more like a problem in xcb-util-cursor... Edit: Now this looks like a mis-compilation?! We are running this code: https://gitlab.freedesktop.org/xorg/lib/libxcb-cursor/-/blob/3d7e713e85af18d7e52cafdc9d20a2715048dee7/cursor/load_cursor.c#L209-211
You see that Edit: And if I ask
The check against |
Ahhh, debian is using an old (ancient?) version of libxcb-cursor. The bug I found above was fixed 7 years ago: https://gitlab.freedesktop.org/xorg/lib/libxcb-cursor/-/commit/cf26479ece9ab9e04616bc10ba674d88a284e5b0 Edit: Reported as https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1025409 |
Certainly. Compiled against LuaJIT 2.1 beta3, xcb-util-cursor 0.1.4. Everything looks good. Did not notice any issue so far. And after changing the startup cursor like this: 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 It does exactly what I want. Thank you! Please let me know if I have to test something else. |
Thanks a lot for testing this! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was a mildly challenging but interesting review since I haven't had read common/array.h before. Thank you, @psychon, for this PR.
Don't go there! It is dark magic that wasn't touched in ages! ;-) But yeah, that file is actually used a lot in the C code to manage lists, but I guess one seldomly has to actually interact with this file. (Latest commit is from 2016 by yours truly and it removes a feature. Before that, there were one commit in 2015, 2014, 2011, 2010. I guess this must mean that this is good code that doesn't need any touching...) |
this would require manual merging because of codecov |
Previously, the cache for xcursors consisted of a static array. We had a hardcoded list of supported cursor names and each one got assigned a position in this array. This hardcoded list means that one cannot simply use whatever cursor a cursor theme happens to provide, since the name needs to be in our list.
In this commit, the code is rewritten. Instead of a hardcoded list, a sorted array is now used as the cache, as provided by the BARRAY code from common/util.h. The array is sorted by the name of the cursor.
This change implies an API change for the xcursor code. Previously, one had to first translate a cursor name into a cache index (xcursor_font_fromstr()) and could then use this to actually get the cursor (xcursor_new()). With this commit, xcursor_new() is the only function provided by the cursor code and it directly gets a string as argument.
Signed-off-by: Uli Schlachter [email protected]
This PR is related to #3737. I am 80% sure that this PR implements (half of) that feature request by making it possible to use arbitrary cursors, as long as xcb-util-cursor can load a cursor by that name.
@akinokonomi Are you in a position to test this? (=Can you build awesome from a git branch and verify whether this does what you want?)
Personally, I only started awesome via
tests/run.sh /dev/null
and started xterm from the menu. The "waiting for startup" cursor briefly appeared. Nothing more was tested.