Hi Peter On Tue, May 9, 2023 at 5:12 PM Peter Maydell <peter.mayd...@linaro.org> wrote:
> 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) ? > > feel free to take it, thanks
