i3lock doesn't display background image with cairo 1.17.2

Farioko commented 5 years ago

I'm submitting a…

[x] Bug
[ ] Feature Request
[ ] Other (Please describe in detail)

Current Behavior

i3lock doesn't display some PNG images when using cairo 1.17.2

Expected Behavior

i3lock should display the PNG image as it did in combination with cairo 1.16.x

Reproduction Instructions

Supply i3lock a PNG image using the -i flag that is using the pixel format 16bpc in combination with cairo 1.17.2. I have attached an example image that is problematic.

Environment

Output of i3lock --version:

i3lock: version 2.12 © 2010 Michael Stapelberg

lockscreen

ghost commented 5 years ago

I can confirm this, having the same issue. Exporting the image with the 8bpc pixel format instead of 16bpc fixes it.

stapelberg commented 4 years ago

This sounds like a cairo bug to me? Is there any indication that i3lock is using cairo wrong? If no, can you report this in the cairo issue tracker please?

stapelberg commented 4 years ago

@psychon Can you clarify whether i3lock is using cairo wrong? We’re loading the PNG using cairo in https://github.com/i3/i3lock/blob/f3b061233e28ad0cf920c9421adc2459b6920812/i3lock.c#L1210-L1217

…later, we’re painting it: https://github.com/i3/i3lock/blob/f3b061233e28ad0cf920c9421adc2459b6920812/unlock_indicator.c#L102-L111

I was under the impression that cairo should handle the format conversion where necessary, but perhaps that’s wrong?

psychon commented 4 years ago

Can't spot anything wrong a quick look. Yes, cairo should handle the conversions. Random suggestions would be to check cairo_status, cairo_surface_status etc before destroying the resource. If the status is non-zero, something somewhere went wrong.

Uhm...

$ git log src/cairo-png.c | head
commit d061570a70c12ebf43e9aa914a9cbb87b616a979
Author: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Date:   Mon Dec 3 15:13:42 2018 +0100

    png: Add support for 16 bpc png reading as floating point format

    Similar to writing png, don't squash 16 bpc to 8 bpc and create
    a float surface to contain the image.

    Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

This sounds related. I wonder which release that commit is in....

$ git describe --contains d061570a70c12ebf43e9aa914a9cbb87b616a979
1.17.2~5

Sigh. One of these "drive-by patches" that do enough to get the submitters use case covered, but do not bother adding support everywhere.

Random guess: Cairo 1.17.2 (why can a point release add features?) added RGBA128F and RGB96F image formats. Here, the image data is stores as floats instead of integers. See commit a34cb719cd9cb4f0c5b78be80b80ab0ae22464a6. The above commit then makes cairo-png produce a surface in this new format for 16bpp PNGs.

Feel free to submit a bug report to cairo. Random work-around would be to create an ARGB32 (or RGB24, depending on what cairo_surface_get_content(surface_returned_from_create_from_png) says) surface and copy your surface_returned_from_create_from_png to that. This is a bit work-around since it needlessly makes stuff slower and instead cairo should handle this internally (where necessary, thus no slowdown where not necessary). Sadly, that bug report will be assigned to me.... uhm... Could you somehow get Maarten Lankhorst involved and ask him if he could fix this? :-)

(From a quick look at the git history, no test cases for this new image format were added to the test suite.)

stapelberg commented 4 years ago

Thanks for taking a look!

@mlankhorst it looks like your commit https://gitlab.freedesktop.org/cairo/cairo/commit/d061570a70c12ebf43e9aa914a9cbb87b616a979 breaks displaying 16bpc images in i3lock. Could you take a look please?

Farioko commented 1 year ago

I noticed today that the image started appearing again. I suspect that this is due to the cairo update (1.17.6-2 -> 1.17.8-2) that was installed on my system a while back.

i3 / i3lock