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

Cursor Icon Regression on GNOME #425

Closed
Friz64 opened this issue Oct 30, 2023 · 9 comments · Fixed by #427
Closed

Cursor Icon Regression on GNOME #425

Friz64 opened this issue Oct 30, 2023 · 9 comments · Fixed by #427

Comments

@Friz64
Copy link
Contributor

Friz64 commented Oct 30, 2023

Description

The cursor icon refuses to change when hovering over the window's borders. When actively resizing, it changes to the correct icon, but goes back to its previous state after. Inside the window, it retains its previous state, even from other windows.

Offending Pull Request: #380 (cc @kchibisov)

Screencast.from.2023-10-30.04-28-39.webm

This issue can also be observed in winit.

I am running GNOME 45.0 on Arch Linux. I also tested KDE Plasma in a VM and it worked fine there.

I'll be glad to do more testing regarding this issue.

Before (for comparison)

Click to expand
Screencast.from.2023-10-30.04-27-48.webm
@Friz64
Copy link
Contributor Author

Friz64 commented Oct 30, 2023

Looks like in the themed_window example, the result of smithay_client_toolkit::seat::pointer::ThemedPointer::set_cursor is ignored, yet every call to it returns PointerThemeError::CursorNotFound.

@Friz64
Copy link
Contributor Author

Friz64 commented Oct 30, 2023

Looks like get_cursor expects a cursor icon name like bottom_left_corner (which it got in the previous version, and which still works when temporarily testing on latest master) but now receives a name like sw-resize through icon.name().

let cursor = themes
.get_cursor(conn, icon.name(), scale as u32, &self.shm)
.map_err(PointerThemeError::InvalidId)?
.ok_or(PointerThemeError::CursorNotFound)?;

@kchibisov
Copy link
Member

@Friz64
Copy link
Contributor Author

Friz64 commented Oct 30, 2023

Huh, indeed, when using the default Adwaita cursor theme it works as expected.

Would it be beneficial to fall back to legacy cursor icon names to accommodate users with outdated cursor themes? I would be open to making a Pull Request for this.

@kchibisov
Copy link
Member

Or we can try to update legacy themes while at it. The future users won't be affected by that anyway due to cursor-shape-v1. It's not like adding a symlink inside the theme will hurt anyone.

@kchibisov
Copy link
Member

You can also try to add alternative cursor names to https://github.com/rust-windowing/cursor-icon/ , but I'd sort of prefer to get themes patched as well.

@Friz64
Copy link
Contributor Author

Friz64 commented Oct 30, 2023

Of course patching themes would be the best solution, but it is not realistic. In the best case users get their cursor themes from the package manager, but I don't believe there will be a big effort to update them all. In the worst case cursor themes are downloaded from random websites, good luck updating any of these.

Yes, I guess this can be worked around by symlinks, but the average user won't know this. They won't even know what is causing the problem.

You can also try to add alternative cursor names to https://github.com/rust-windowing/cursor-icon/

How should the API for this work? Older winit versions seem to have already had some kind of "fallback" system, where in some cases multiple cursors are tried until one works. See

https://github.com/rust-windowing/winit/blob/2ebbfab6a47f93f76841553dcb3cc53683e4955c/src/platform_impl/linux/wayland/seat/pointer/mod.rs#L78-L126

@kchibisov
Copy link
Member

/// Alternative names sometimes used to refer to this icon.
fn alt_names(&self) -> &[&'static str]

@Friz64
Copy link
Contributor Author

Friz64 commented Oct 30, 2023

Opened a PR: rust-windowing/cursor-icon#11

I will also make a PR for this repository utilizing the new method so it can be tested.

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 a pull request may close this issue.

2 participants