search

yvbbrjdr / i3lock-fancy-rapid

A faster implementation of i3lock-fancy.
BSD 3-Clause "New" or "Revised" License
149 stars 20 forks source link

Don't encode PNG #4

Closed mortie closed 5 years ago

mortie commented 5 years ago

As of https://github.com/i3/i3lock/pull/226, i3lock supports reading raw image data instead of PNG. This patch makes i3lock-fancy-rapid write raw image data to i3lock's stdin instead of encoding the PNG and writing it to a file and passing the path to i3lock.

I don't know your thoughts on how quickly you'll want to break compatibility with old versions of i3lock, or if you'll maybe want a flag which switches between the old behavior and the new, or make it a compile-time option.

EDIT: just to add some numbers; on my desktop, running time ./i3lock-fancy-rapid 0 0 takes around 0.28-0.30 seconds with this patch, and around 0.60-0.63 seconds without this patch. That means that just encoding and immediately decoding the PNG image on this computer wastes 300ms; that's already pretty bad, and it's much worse on systems with weaker CPUs.

yvbbrjdr commented 5 years ago

Thank you for your contriution! This is the kind of speed up I always wanted! Will merge after code review!

yvbbrjdr commented 5 years ago

The code looks good to me. It seems that i3lock hasn't released a new version for your patch so it will break on any OS using the official i3lock package. I'll first merge this PR to a dev branch and when the time comes will merge it to master. Does that sound good to you? Thank you again for your contributions!

mortie commented 5 years ago

Yeah, I didn't mean that you should merge this to master and release it immediately, I should maybe have been more clear about that :p I sent this pull request more just to have it out there, and let you make the decision about whether you just use this directly or add it as a compile-time or runtime option, and about when to break compatibility with old versions of i3lock.

You may also be curious about other changes I've made on my personal branch (https://github.com/mortie/i3lock-fancy-rapid/commits/master), most notably removing the need to allocate and free in each box_blur_once by passing in a buffer, removing the need to memcpy from dest to src in box_blur by switching between which buffer is considered "source" an which is considered "destination", and adding an option to downscale screenshots before blurring (since 4k images take a long time to blur but 4k blurred screenshots and 1080p blurred screenshots look virtually the same on a laptop). I may eventually package some of those things up as pull requests, but feel free to incorporate anything which seems useful.

yvbbrjdr commented 5 years ago

Yes I was to eliminate the buffer overhead as well. Will look at your branch!

yvbbrjdr commented 5 years ago

Also, I'm thinking about supporting Wayland (and thus sway) but it looks complicated to take a screenshot in Wayland.