--screenshot flips colors

[MatthiasCoppens]

~ $ uname -a
Linux gentoo-laptop 5.4.72-gentoo #1 SMP Sun Oct 25 10:54:30 CET 2020 x86_64 Intel(R) Core(TM) i7-5500U CPU @ 2.40GHz GenuineIntel GNU/Linux
~ $ sway -v
sway version 1.5-32b93ef6 (Dec  1 2020, branch 'HEAD')
~ $ swaylock -v
swaylock version v1.6-3-4-g3aa6b4b (Nov 26 2020, branch 'HEAD')

wlroots is also built from master on 1 december.

[johannlejeune]

I might have some hints about this issue. I'm using an OBS plugin called wlrobs that has an option to flip red and blue when capturing the screen, to fix this very issue. You can see how it's implemented here. I took a quick look at the code of this repo and didn't manage to find something that looked similar, but I don't think it'd be too hard to implement such an option, and it would probably fix the color issue.

[MatthiasCoppens]

I'm also pretty sure it's a problem with how swaylock-effects detects the pixel format, I might go through the code next week when I've got the time

[mortie]

Alright, so this isn't actually super surprising. main.c passes the format argument it receives from the compositor to the load_background_from_buffer in background-image.c, but load_background_from_buffer then ignores the format and assumes the input is in cairo's CAIRO_FORMAT_RGB24 (aka RGBX in native endianness; calling it "RGB24" is a misnomer IMO, since it still uses 32 bits per pixel, it just ignores 8 of them). If the format is rgbx8888 or rgba8888, this works, but otherwise it'll look wrong.

Any individual conversion from some pixel format to cairo's RGB24 is easy, but there's a lot of formats; handling all of them would be a ton of code.

The two solutions I see is: A) figure out which pixel formats compositors actually use and add conversions from those, or B) add a dependency on a pixel format conversion library which handles all of them for us. I'm not sure which is better, or, in the case of B, which libraries would be suitable.

[mortie]

wlroots’s screen copy example code just supports RGB and BGR (https://github.com/swaywm/wlroots/blob/master/examples/screencopy.c#L62), so just supporting those two pixel formats might be a good start.

[mortie]

I added pixel format conversion in https://github.com/mortie/swaylock-effects/pull/49. It should work now, but I don't have a system to test it with (sway on my laptop uses RGB, not BGR). Do colors look better with that patch?

[johannlejeune]

I tried it on my laptop and I'm getting a segfault upon launch, can't tell if it's related to the changes you made in the PR or not :confused: It works on my desktop system (so it doesn't crash), but this computer didn't have the color shifting problem with the master branch.

[mortie]

Yeah, sorry about that. I can't test the BGR case on my laptop obviously, but I made sure the RGB case worked... Except I didn't consider the fact that I'm skipping the pixel format conversion entirely if the compositor gives you RGB and your system is little endian. Both the pixel format conversion routines were just broken; it just usually isn't necessary to convert RGB.

https://github.com/mortie/swaylock-effects/pull/49/commits/535146627d86d501dca96e2c3e7d5e25e1cb56f6 should work better.

[MatthiasCoppens]

@mortie The pixelformat is still not quite right right on my laptop. Here are screenshots from my laptop on the master branch and on the fix/pixel-format-conversion branch.

Not locked

master branch

fix/pixel-format-conversion branch

[johannlejeune]

I'm getting very similar results on my side, I disabled the blur effect, here are the screenshots, just in case :

Screenshots

 Not locked  master branch  fix/pixel-format-conversion branch

[mortie]

Right, I had a typo in the XBGR conversion. https://github.com/mortie/swaylock-effects/pull/49/commits/c30e0d5f7525b73a4443cd7196b2717510d2af6b should fix it.

[johannlejeune]

Indeed, colors are looking good this time ! :+1:

[mortie]

Alright, it's merged now. Feel free to reopen if there are more screenshot pixel format issues!

[MatthiasCoppens]

Can confirm it's fixed on my end as well! :+1: