On Tue, 2 May 2023 at 20:36, Marc-André Lureau <marcandre.lur...@redhat.com> wrote: > > > > On Tue, May 2, 2023 at 5:56 PM Peter Maydell <peter.mayd...@linaro.org> wrote: >> >> When we take a PNG screenshot the ordering of the colour channels in >> the data is not correct, resulting in the image having weird >> colouring compared to the actual display. (Specifically, on a >> little-endian host the blue and red channels are swapped; on >> big-endian everything is wrong.) >> >> This happens because the pixman idea of the pixel data and the libpng >> idea differ. PIXMAN_a9r8g8b8 defines that pixels are 32-bit values, >> with A in bits 24-31, R in bits 16-23, G in bits 8-15 and B in bits >> 0-7. This means that on little-endian systems the bytes in memory >> are >> B G R A >> and on big-endian systems they are >> A R G B >> >> libpng, on the other hand, thinks of pixels as being a series of >> values for each channel, so its format PNG_COLOR_TYPE_RGB_ALPHA >> always wants bytes in the order >> R G B A >> >> This isn't the same as the pixman order for either big or little >> endian hosts. >> >> The alpha channel is also unnecessary bulk in the output PNG file, >> because there is no alpha information in a screenshot. >> >> To handle the endianness issue, we already define in ui/qemu-pixman.h >> various PIXMAN_BE_* and PIXMAN_LE_* values that give consistent >> byte-order pixel channel formats. So we can use PIXMAN_BE_r8g8b8 and >> PNG_COLOR_TYPE_RGB, which both have an in-memory byte order of >> R G B >> and 3 bytes per pixel. >> >> (PPM format screenshots get this right; they already use the >> PIXMAN_BE_r8g8b8 format.) >> >> Cc: qemu-sta...@nongnu.org >> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1622 >> Fixes: 9a0a119a382867 ("Added parameter to take screenshot with screendump >> as PNG") >> Signed-off-by: Peter Maydell <peter.mayd...@linaro.org> > > > Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com>
Thanks; shall I take this via target-arm.next, or would you prefer to take it via the UI queue (with the commit message typo fixed) ? -- PMM
