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

Performance improvements, and (fractional) DPI scaling #53

Merged
merged 7 commits into from
Aug 16, 2024

Conversation

ids1024
Copy link
Member

@ids1024 ids1024 commented Aug 8, 2024

I've noticed cosmic-bg is somewhat slow to start, particularly with a couple monitors.

I see it's noticeably better changing a couple things:

  • Cache the image, instead of opening the file when each monitor is draw. Given I have the same image shown on all monitors.
  • Use tiny_skia for scaling; image::imageops seems pretty inefficient in comparison.

RUST_LOG=debug cargo run --release shows much better wallpaper draw times.

It also is drawing 6 times, for just 3 outputs, also adding to run time. It seems first_configure is never set to anything other than false? Presumably some change should be made there.

tiny-skia only handles RGBA8. So I guess we may need to fallback to imageops for HDR source images, if anyone is using those. I also have only tried updating the zoom scaler, but the others can probably be done more efficiently with tiny-skia as well.

@ids1024
Copy link
Member Author

ids1024 commented Aug 8, 2024

Performance seems good with fast_image_resize. So if the unnecessary additional draws can also be fixed, I'm be happy with the performance of cosmic-bg.

@ids1024 ids1024 force-pushed the cosmic-bg-speed branch 4 times, most recently from 01f87b4 to 4a9e921 Compare August 8, 2024 21:18
@ids1024
Copy link
Member Author

ids1024 commented Aug 8, 2024

I think it should be possible to improve the performance of all three scaling modes, with fast_image_resize. Though some testing is needed to make sure behavior is right. The API is a bit different, though not that complicated.

I guess cosmic-bg still needs to properly handling DPI scaling (#51). I guess it should bind the fractional scale protocol...

@ids1024 ids1024 changed the title WIP Performance improvements WIP Performance improvements, and (fractional) DPI scaling Aug 8, 2024
@ids1024
Copy link
Member Author

ids1024 commented Aug 8, 2024

I think the fractional scaling implementation I've added is correct. Though there are still a few details to improve here.

@ids1024
Copy link
Member Author

ids1024 commented Aug 8, 2024

Oh, and I guess I should note that I'm testing with a 6000x4000 (24 megapixel) image from my DSLR. So if I wanted it to be faster I could probably scale that down a bit before cosmic-bg accesses the file. But if performance is good with that, it should handle most reasonable desktop background images. (And I guess scaling multiple times would results in somewhat worse visual quality, in principle.)

src/wallpaper.rs Outdated
img.width() != layer.width || img.height() != layer.height
}) {
let CosmicBgLayer { width, height, .. } = *layer;
let fractional_scale = layer.fractional_scale.unwrap();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we require the fractional-scale protocol now?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm assuming cosmic-bg isn't really targeting compositors other than cosmic-comp, so there's no harm in depending on protocols cosmic-comp provides. (Especially if it's a standard protocol other current compositors should support.)

We could definitely not require it, but it makes this a bit more complicated, particularly if we still want to support integer scaling without the protocol.

ids1024 added 7 commits August 9, 2024 09:59
Anchoring to all edges will do that automatically, and should adjust if
the resolution changes.
We don't draw until we get an initial configure, so there's no need to
have the size before then.

`first_configure` was never set to `true`, so the logic there wasn't
right. And I was seeing more draws than needed with multiple monitors,
slowing things down.

Instead have a `needs_redraw` field that is set when a surface should be
drawn, due to a new configure. This is at least better than how this was
behaving.
Now requires viewporter and fractional scale Wayland protocols.
`fast_image_resize` provides a range of scaling algorithms, but much
better optimized than those in `image::imageops`, including SIMD
optimization. This noticeably improves startup time.
@ids1024 ids1024 changed the title WIP Performance improvements, and (fractional) DPI scaling Performance improvements, and (fractional) DPI scaling Aug 9, 2024
@ids1024 ids1024 marked this pull request as ready for review August 9, 2024 19:46
@ids1024 ids1024 requested a review from a team August 9, 2024 19:47
@ids1024
Copy link
Member Author

ids1024 commented Aug 16, 2024

Apparently it was 69c7b5f that previously removed the use of fast_image_resize.

Not sure if fast_image_resize supports 10-bit color (is that why it was removed there?), but this PR still uses the resizing in image as a fallback if fast_image_resize errors due to not supporting a format.

Copy link
Member

@Drakulix Drakulix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@ids1024 ids1024 merged commit 1dbaf96 into master Aug 16, 2024
7 checks passed
@ids1024 ids1024 deleted the cosmic-bg-speed branch August 16, 2024 15:57
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 this pull request may close these issues.

2 participants