wayland-rs
wayland-rs copied to clipboard
feat: load cursor from data
I want to add a way to load cursor from raw data
Codecov Report
Attention: Patch coverage is 0% with 39 lines in your changes are missing coverage. Please review.
Project coverage is 72.68%. Comparing base (
ea03873) to head (610e93a). Report is 4 commits behind head on master.
:exclamation: Current head 610e93a differs from pull request most recent head 85585de. Consider uploading reports for the commit 85585de to get more accurate results
| Files | Patch % | Lines |
|---|---|---|
| wayland-cursor/src/lib.rs | 0.00% | 39 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## master #680 +/- ##
==========================================
- Coverage 76.16% 72.68% -3.49%
==========================================
Files 45 48 +3
Lines 8153 7879 -274
==========================================
- Hits 6210 5727 -483
- Misses 1943 2152 +209
| Flag | Coverage Δ | |
|---|---|---|
| main | 58.04% <0.00%> (-1.03%) |
:arrow_down: |
| test-- | 78.08% <ø> (-2.99%) |
:arrow_down: |
| test--server_system | 61.08% <ø> (-3.59%) |
:arrow_down: |
| test-client_system- | 68.92% <ø> (-3.27%) |
:arrow_down: |
| test-client_system-server_system | 51.32% <ø> (-3.64%) |
:arrow_down: |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Thanks, that is interesting, however I'm not sure this design is the whole way we want to go.
As I understand, the goal is to allow an app to provide fallback cursor images by itself in case the system theme does not provide all the ones it needs.
The method you introduce directly creates a Cursor instance, storing its contents using the SHM pool of the theme, but without any other checks, nor inserting it into the internal state. As a result the user will have to keep the Cursor instance around themselves (as the CursorTheme does not store it) and check to use it whenever they need this particular cursor, to avoid reloading a new copy in memory every time.
I think a better approach would be to introduce a proper mechanism by which the user of wayland-cursor can specify a general fallback mechanism. A possible implementation of that would be to let the user provide a fallback Fn(&str) -> Cow<'static, [u8]> closure that would be invoked by load_cursor if the requested cursor was not found in the system theme. (The Cow<'static, [u8]> return type being used so that this closure could either return a reference to some cursor data bundled in the executable itself, or load the contents from disk in the app files.)
With such a fallback mechanism, later use of the CursorTheme would be transparent, and everything would "just work".
What do you think of this, would that cover your needs?
Ok, I have made change. I also think that is bad.. so I am also thinking a better way..
Hi gentle ping: @elinorbgr Can we get a review on this? 😄
Sorry for the long time, this kinda slipped outside of my radar...
LGTM overall, but:
* the `Fallback` function should take a size as argument in addition to the cursor name, like the `load_cursor`, making it possible to have multiple sizes for the fallback cursor if desired * Could you expand the documentation of `set_fallback()` to make it clearer what that callback is supposed to do, and how it's meant to be used ?
Ok, is that ok? I have pushed another commit
done @elinorbgr