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> > --- > Disclaimer: I don't have a BE system that I have convenient > graphics output from that I can use to test the screenshot > format there. > --- > ui/console.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/ui/console.c b/ui/console.c > index 6e8a3cdc62d..e173731e205 100644 > --- a/ui/console.c > +++ b/ui/console.c > @@ -311,7 +311,7 @@ static bool png_save(int fd, pixman_image_t *image, > Error **errp) > png_struct *png_ptr; > png_info *info_ptr; > g_autoptr(pixman_image_t) linebuf = > - qemu_pixman_linebuf_create(PIXMAN_a8r8g8b8, > width); > + qemu_pixman_linebuf_create(PIXMAN_BE_r8g8b8, width); > uint8_t *buf = (uint8_t *)pixman_image_get_data(linebuf); > FILE *f = fdopen(fd, "wb"); > int y; > @@ -341,7 +341,7 @@ static bool png_save(int fd, pixman_image_t *image, > Error **errp) > png_init_io(png_ptr, f); > > png_set_IHDR(png_ptr, info_ptr, width, height, 8, > - PNG_COLOR_TYPE_RGB_ALPHA, PNG_INTERLACE_NONE, > + PNG_COLOR_TYPE_RGB, PNG_INTERLACE_NONE, > PNG_COMPRESSION_TYPE_BASE, PNG_FILTER_TYPE_BASE); > > png_write_info(png_ptr, info_ptr); > -- > 2.34.1 > >
