[PATCH 0/2] MR7924: windowscodecs/tiff: Remove unnecessary image data byte-swaping.
Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7924
From: Nikolay Sivov <nsivov(a)codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/windowscodecs/tests/tiffformat.c | 206 ++++++++++++++++++++++++++ 1 file changed, 206 insertions(+) diff --git a/dlls/windowscodecs/tests/tiffformat.c b/dlls/windowscodecs/tests/tiffformat.c index e8d2975dfb9..a42635ab44d 100644 --- a/dlls/windowscodecs/tests/tiffformat.c +++ b/dlls/windowscodecs/tests/tiffformat.c @@ -19,11 +19,13 @@ #include <math.h> #include <stdarg.h> #include <stdio.h> +#include <stdint.h> #define COBJMACROS #include "windef.h" #include "wincodec.h" +#include "winternl.h" #include "wine/test.h" #define EXPECT_REF(obj,ref) _expect_ref((IUnknown*)obj, ref, __LINE__) @@ -63,6 +65,81 @@ struct IFD_rational LONG denominator; }; +static unsigned int put_u16be(char **dst, const char **src) +{ + uint16_t r, w; + + memcpy(&r, *src, sizeof(r)); + *src += sizeof(r); + w = RtlUshortByteSwap(r); + memcpy(*dst, &w, sizeof(w)); + *dst += sizeof(w); + + return r; +} + +static unsigned int put_u32be(char **dst, const char **src) +{ + uint32_t r, w; + + memcpy(&r, *src, sizeof(r)); + *src += sizeof(r); + w = RtlUlongByteSwap(r); + memcpy(*dst, &w, sizeof(w)); + *dst += sizeof(w); + + return r; +} + +static void put_ifd_entry_be(char **dst, const char **src) +{ + const struct IFD_entry *entry = (const struct IFD_entry *)*src; + + put_u16be(dst, src); /* id */ + put_u16be(dst, src); /* type */ + put_u32be(dst, src); /* count */ + + switch (entry->type) + { + case IFD_SHORT: + put_u16be(dst, src); + put_u16be(dst, src); + break; + case IFD_LONG: + case IFD_RATIONAL: + put_u32be(dst, src); + break; + default: + ok(0, "Unexpected entry type %u.\n", entry->type); + } +} + +static char *get_tiff_be(const void *src, size_t size) +{ + const char *data = src; + unsigned int count; + char *dst, *p; + + if (!(dst = malloc(size))) return NULL; + p = dst; + + *p++ = 'M'; *p++ = 'M'; data += 2; + put_u16be(&p, &data); + put_u32be(&p, &data); + count = put_u16be(&p, &data); + for (int i = 0; i < count; ++i) + put_ifd_entry_be(&p, &data); + put_u32be(&p, &data); /* next IFD */ + put_u32be(&p, &data); + put_u32be(&p, &data); + + /* copy remaining image data as is */ + memcpy(p, data, size - (p - dst)); + + return dst; +} + + static const struct tiff_1bpp_data { USHORT byte_order; @@ -346,6 +423,42 @@ static const struct tiff_4bps_bgra { 0x12,0x34,0x70, 0xe8,0xf0,0xa0 } }, }; + +static const struct tiff_16bpp_data +{ + USHORT byte_order; + USHORT version; + ULONG dir_offset; + USHORT number_of_entries; + struct IFD_entry entry[13]; + ULONG next_IFD; + struct IFD_rational res; + BYTE pixel_data[2]; +} tiff_16bpp_data = +{ + 'I' | 'I' << 8, + 42, + FIELD_OFFSET(struct tiff_16bpp_data, number_of_entries), + 13, + { + { 0xff, IFD_SHORT, 1, 0 }, /* SUBFILETYPE */ + { 0x100, IFD_LONG, 1, 1 }, /* IMAGEWIDTH */ + { 0x101, IFD_LONG, 1, 1 }, /* IMAGELENGTH */ + { 0x102, IFD_SHORT, 1, 16 }, /* BITSPERSAMPLE */ + { 0x103, IFD_SHORT, 1, 1 }, /* COMPRESSION */ + { 0x106, IFD_SHORT, 1, 1 }, /* PHOTOMETRIC */ + { 0x111, IFD_LONG, 1, FIELD_OFFSET(struct tiff_16bpp_data, pixel_data) }, /* STRIPOFFSETS */ + { 0x115, IFD_SHORT, 1, 1 }, /* SAMPLESPERPIXEL */ + { 0x116, IFD_LONG, 1, 1 }, /* ROWSPERSTRIP */ + { 0x117, IFD_LONG, 1, 2 }, /* STRIPBYTECOUNT */ + { 0x11a, IFD_RATIONAL, 1, FIELD_OFFSET(struct tiff_16bpp_data, res) }, /* XRESOLUTION */ + { 0x11b, IFD_RATIONAL, 1, FIELD_OFFSET(struct tiff_16bpp_data, res) }, /* YRESOLUTION */ + { 0x128, IFD_SHORT, 1, 2 }, /* RESOLUTIONUNIT */ + }, + 0, + { 900, 3 }, + { 0x12, 0x34 } +}; #pragma pack(pop) static IWICImagingFactory *factory; @@ -1365,6 +1478,98 @@ static void test_tiff_4bps_bgra(void) IWICBitmapDecoder_Release(decoder); } +static void test_tiff_16bpp(void) +{ + IWICBitmapFrameDecode *frame; + IWICBitmapDecoder *decoder; + UINT count, width, height; + double dpi_x, dpi_y; + char *tiff_be; + GUID format; + HRESULT hr; + WICRect rc; + ULONG data; + + hr = create_decoder(&tiff_16bpp_data, sizeof(tiff_16bpp_data), &decoder); + ok(hr == S_OK, "Failed to load TIFF image data %#lx\n", hr); + ok(decoder != NULL, "Failed to load TIFF image data\n"); + + hr = IWICBitmapDecoder_GetFrameCount(decoder, &count); + ok(hr == S_OK, "GetFrameCount error %#lx\n", hr); + ok(count == 1, "got %u\n", count); + + hr = IWICBitmapDecoder_GetFrame(decoder, 0, &frame); + ok(hr == S_OK, "GetFrame error %#lx\n", hr); + + hr = IWICBitmapFrameDecode_GetSize(frame, &width, &height); + ok(hr == S_OK, "GetSize error %#lx\n", hr); + ok(width == 1, "got %u\n", width); + ok(height == 1, "got %u\n", height); + + hr = IWICBitmapFrameDecode_GetResolution(frame, &dpi_x, &dpi_y); + ok(hr == S_OK, "GetResolution error %#lx\n", hr); + ok(dpi_x == 300.0, "got %f\n", dpi_x); + ok(dpi_y == 300.0, "got %f\n", dpi_y); + + hr = IWICBitmapFrameDecode_GetPixelFormat(frame, &format); + ok(hr == S_OK, "GetPixelFormat error %#lx\n", hr); + ok(IsEqualGUID(&format, &GUID_WICPixelFormat16bppGray), "Unexpected format %s\n", wine_dbgstr_guid(&format)); + + rc.X = 0; + rc.Y = 0; + rc.Width = 1; + rc.Height = 1; + data = 0xfefefefe; + hr = IWICBitmapFrameDecode_CopyPixels(frame, &rc, 4, sizeof(data), (BYTE *)&data); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ok(data == 0xfefe3412, "Unexpected data %#lx.\n", data); + + IWICBitmapFrameDecode_Release(frame); + IWICBitmapDecoder_Release(decoder); + + /* Same data, BE format. */ + tiff_be = get_tiff_be(&tiff_16bpp_data, sizeof(tiff_16bpp_data)); + + hr = create_decoder(tiff_be, sizeof(tiff_16bpp_data), &decoder); + ok(hr == S_OK, "Failed to load TIFF image data %#lx\n", hr); + ok(decoder != NULL, "Failed to load TIFF image data\n"); + + hr = IWICBitmapDecoder_GetFrameCount(decoder, &count); + ok(hr == S_OK, "GetFrameCount error %#lx\n", hr); + ok(count == 1, "got %u\n", count); + + hr = IWICBitmapDecoder_GetFrame(decoder, 0, &frame); + ok(hr == S_OK, "GetFrame error %#lx\n", hr); + + hr = IWICBitmapFrameDecode_GetSize(frame, &width, &height); + ok(hr == S_OK, "GetSize error %#lx\n", hr); + ok(width == 1, "got %u\n", width); + ok(height == 1, "got %u\n", height); + + hr = IWICBitmapFrameDecode_GetResolution(frame, &dpi_x, &dpi_y); + ok(hr == S_OK, "GetResolution error %#lx\n", hr); + ok(dpi_x == 300.0, "got %f\n", dpi_x); + ok(dpi_y == 300.0, "got %f\n", dpi_y); + + hr = IWICBitmapFrameDecode_GetPixelFormat(frame, &format); + ok(hr == S_OK, "GetPixelFormat error %#lx\n", hr); + ok(IsEqualGUID(&format, &GUID_WICPixelFormat16bppGray), "Unexpected format %s\n", wine_dbgstr_guid(&format)); + + rc.X = 0; + rc.Y = 0; + rc.Width = 1; + rc.Height = 1; + data = 0xfefefefe; + hr = IWICBitmapFrameDecode_CopyPixels(frame, &rc, 4, sizeof(data), (BYTE *)&data); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + todo_wine + ok(data == 0xfefe1234, "Unexpected data %#lx.\n", data); + + IWICBitmapFrameDecode_Release(frame); + IWICBitmapDecoder_Release(decoder); + free(tiff_be); +} + START_TEST(tiffformat) { HRESULT hr; @@ -1384,6 +1589,7 @@ START_TEST(tiffformat) test_tiff_8bpp_alpha(); test_tiff_resolution(); test_tiff_24bpp(); + test_tiff_16bpp(); IWICImagingFactory_Release(factory); CoUninitialize(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7924
From: Nikolay Sivov <nsivov(a)codeweavers.com> This is handled automatically by libtiff when loading big-endian tiffs on little-endian build. It's implemented as a 'postdecode' hook and depends on TIFFTAG_BITSPERSAMPLE tag. Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/windowscodecs/libtiff.c | 31 --------------------------- dlls/windowscodecs/tests/tiffformat.c | 1 - 2 files changed, 32 deletions(-) diff --git a/dlls/windowscodecs/libtiff.c b/dlls/windowscodecs/libtiff.c index 98ad376f431..584dad116ed 100644 --- a/dlls/windowscodecs/libtiff.c +++ b/dlls/windowscodecs/libtiff.c @@ -659,11 +659,8 @@ static HRESULT CDECL tiff_decoder_get_decoder_palette(struct decoder *iface, UIN static HRESULT tiff_decoder_read_tile(struct tiff_decoder *This, UINT tile_x, UINT tile_y) { tsize_t ret; - int swap_bytes; tiff_decode_info *info = &This->cached_decode_info; - swap_bytes = TIFFIsByteSwapped(This->tiff); - if (info->tiled) ret = TIFFReadEncodedTile(This->tiff, tile_x + tile_y * info->tiles_across, This->cached_tile, info->tile_size); else @@ -874,34 +871,6 @@ static HRESULT tiff_decoder_read_tile(struct tiff_decoder *This, UINT tile_x, UI } } - if (swap_bytes && info->bps > 8) - { - UINT row, i, samples_per_row; - BYTE *sample, temp; - - samples_per_row = info->tile_width * info->samples; - - switch(info->bps) - { - case 16: - for (row=0; row<info->tile_height; row++) - { - sample = This->cached_tile + row * info->tile_stride; - for (i=0; i<samples_per_row; i++) - { - temp = sample[1]; - sample[1] = sample[0]; - sample[0] = temp; - sample += 2; - } - } - break; - default: - ERR("unhandled bps for byte swap %u\n", info->bps); - return E_FAIL; - } - } - if (info->invert_grayscale) { BYTE *byte, *end; diff --git a/dlls/windowscodecs/tests/tiffformat.c b/dlls/windowscodecs/tests/tiffformat.c index a42635ab44d..1bc94b03f35 100644 --- a/dlls/windowscodecs/tests/tiffformat.c +++ b/dlls/windowscodecs/tests/tiffformat.c @@ -1562,7 +1562,6 @@ static void test_tiff_16bpp(void) data = 0xfefefefe; hr = IWICBitmapFrameDecode_CopyPixels(frame, &rc, 4, sizeof(data), (BYTE *)&data); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); - todo_wine ok(data == 0xfefe1234, "Unexpected data %#lx.\n", data); IWICBitmapFrameDecode_Release(frame); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7924
This merge request was approved by Esme Povirk. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7924
Could we please avoid using stdint.h types in windowscodecs? We already have USHORT, ULONG and similar types. Also tests/metadata.c,byte_swap_ifd_data() already allows creating a byte swapped TIFF, please use it instead of inventing a new one. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7924#note_102009
participants (4)
-
Dmitry Timoshkov (＠dmitry) -
Esme Povirk (＠madewokherd)
-
Nikolay Sivov
-
Nikolay Sivov (＠nsivov)