-
Notifications
You must be signed in to change notification settings - Fork 940
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
Allow custom cursor caching #3276
Conversation
ba4f5e2
to
5bb3dd4
Compare
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.
LGTM to me for X11 changes
Looks really nice and just like we talked about. Also implementing this shouldn't be too hard for x11 and wayland anymore because both of their cursor types already do cleanup with |
To summarize what I did for Web here because this was ridiculously complicated: Before
Unfortunately this isn't ideal, as we also didn't determine if the previous cursor has finished loading. It also complicated the code, storing and account for previous cursors. After
This simplified the code a lot, but we also had to add a lot more code to handle all this correctly. Specifically the fact that |
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.
API looks fine to me
1f03bbc
to
e38ad41
Compare
e38ad41
to
c07b701
Compare
This is a proposal to change the API as discussed before in IRC and #3218.
The API now works like this:
So now building a
CustomCursor
requires access toEventLoopWindowTarget
, the cursor itself remainsSend + Sync
(the builder itself as well).All backends work exactly the same as before, ergo they don't implement caching. I only implemented caching for Web.
I will make follow-up PRs for Windows and MacOS, where I know caching should be pretty straightforward and uncontroversial to implement.
I also made a big improvement in the Web implementation: we now wait for cursors to actually load before applying them. We had a really complex fallback cursor system in place beforehand.
Cc @kchibisov, @eero-lehtinen.