[PATCH v3] windowscodecs: tiffformat: Fix TiffFrameDecode_GetResolution, add test

Joachim Priesner joachim.priesner at web.de
Mon Feb 5 13:34:59 CST 2018


The behavior for the invalid resolution case is different for Windows XP
and Server 2003. Implement the behavior of current Windows versions, but
allow the old behavior in the test.

v3: Account for a difference between libtiff versions in handling the
case where the resolution's denominator is zero. libtiff commit a39f6131
changed the output value from INFINITY (division by zero) to 0 in that
case.

Signed-off-by: Joachim Priesner <joachim.priesner at web.de>
---
 dlls/windowscodecs/tests/tiffformat.c | 137 +++++++++++++++++++++++++++++++++-
 dlls/windowscodecs/tiffformat.c       |  72 ++++++++++--------
 2 files changed, 173 insertions(+), 36 deletions(-)

diff --git a/dlls/windowscodecs/tests/tiffformat.c b/dlls/windowscodecs/tests/tiffformat.c
index e4a4b66ea8..a7b6f5b594 100644
--- a/dlls/windowscodecs/tests/tiffformat.c
+++ b/dlls/windowscodecs/tests/tiffformat.c
@@ -16,6 +16,7 @@
  * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
  */
 
+#include <math.h>
 #include <stdarg.h>
 #include <stdio.h>
 
@@ -84,8 +85,8 @@ static const struct tiff_1bpp_data
         { 0x115, IFD_SHORT, 1, 1 }, /* SAMPLESPERPIXEL */
         { 0x116, IFD_LONG, 1, 1 }, /* ROWSPERSTRIP */
         { 0x117, IFD_LONG, 1, 1 }, /* STRIPBYTECOUNT */
-        { 0x11a, IFD_RATIONAL, 1, FIELD_OFFSET(struct tiff_1bpp_data, res) },
-        { 0x11b, IFD_RATIONAL, 1, FIELD_OFFSET(struct tiff_1bpp_data, res) },
+        { 0x11a, IFD_RATIONAL, 1, FIELD_OFFSET(struct tiff_1bpp_data, res) }, /* XRESOLUTION */
+        { 0x11b, IFD_RATIONAL, 1, FIELD_OFFSET(struct tiff_1bpp_data, res) }, /* YRESOLUTION */
         { 0x128, IFD_SHORT, 1, 2 }, /* RESOLUTIONUNIT */
     },
     0,
@@ -124,8 +125,8 @@ static const struct tiff_8bpp_alpha
         { 0x115, IFD_SHORT, 1, 2 }, /* SAMPLESPERPIXEL */
         { 0x116, IFD_LONG, 1, 2 }, /* ROWSPERSTRIP */
         { 0x117, IFD_LONG, 1, 8 }, /* STRIPBYTECOUNT */
-        { 0x11a, IFD_RATIONAL, 1, FIELD_OFFSET(struct tiff_8bpp_alpha, res) },
-        { 0x11b, IFD_RATIONAL, 1, FIELD_OFFSET(struct tiff_8bpp_alpha, res) },
+        { 0x11a, IFD_RATIONAL, 1, FIELD_OFFSET(struct tiff_8bpp_alpha, res) }, /* XRESOLUTION */
+        { 0x11b, IFD_RATIONAL, 1, FIELD_OFFSET(struct tiff_8bpp_alpha, res) }, /* YRESOLUTION */
         { 0x11c, IFD_SHORT, 1, 1 }, /* PLANARCONFIGURATION */
         { 0x128, IFD_SHORT, 1, 2 }, /* RESOLUTIONUNIT */
         { 0x152, IFD_SHORT, 1, 1 } /* EXTRASAMPLES: 1 - Associated alpha with pre-multiplied color */
@@ -134,6 +135,81 @@ static const struct tiff_8bpp_alpha
     { 96, 1 },
     { 0x11,0x22,0x33,0x44,0x55,0x66,0x77,0x88 }
 };
+
+static const struct tiff_resolution_test_data
+{
+    struct IFD_rational resx;
+    struct IFD_rational resy;
+    LONG resolution_unit;
+    double expected_dpi_x;
+    double expected_dpi_y;
+    /* if != 0: values for different behavior of some Windows versions */
+    double broken_dpi_x;
+    double broken_dpi_y;
+} tiff_resolution_test_data[] =
+{
+    { { 100, 1 }, {  50, 1 }, 0, 100.0,  50.0,     0,      0  }, /* invalid resolution unit */
+    { {  50, 1 }, { 100, 1 }, 0,  50.0, 100.0,     0,      0  },
+
+    { { 100, 1 }, {  50, 1 }, 1, 100.0,  50.0,     0,      0  }, /* RESUNIT_NONE */
+    { {  50, 1 }, { 100, 1 }, 1,  50.0, 100.0,     0,      0  },
+
+    { {  49, 1 }, {  49, 1 }, 2,  49.0,  49.0,     0,      0  }, /* same resolution for both X and Y */
+    { {  33, 1 }, {  55, 1 }, 2,  33.0,  55.0,     0,      0  }, /* different resolutions for X and Y */
+    { {  50, 2 }, {  66, 3 }, 2,  25.0,  22.0,     0,      0  }, /* denominator != 1 */
+
+    { { 100, 1 }, { 200, 1 }, 3, 254.0, 508.0,     0,      0  }, /* unit = centimeters */
+
+    /* XP and Server 2003 do not discard both resolution values if only one of them is invalid */
+    { {   0, 1 }, {  29, 1 }, 2,  96.0,  96.0,     0,   29.0  }, /* resolution 0 */
+    { {  58, 1 }, {  29, 0 }, 2,  96.0,  96.0,  58.0,      0  }, /* denominator 0 (division by zero) */
+
+    /* XP and Server 2003 return 96 dots per centimeter (= 243.84 dpi) as fallback value */
+    { {   0, 1 }, { 100, 1 }, 3,  96.0,  96.0, 243.84, 254.0  }, /* resolution 0 and unit = centimeters */
+    { {  50, 1 }, {  72, 0 }, 3,  96.0,  96.0, 127.0,  243.84 }  /* denominator 0 and unit = centimeters */
+};
+
+static struct tiff_resolution_image_data
+{
+    USHORT byte_order;
+    USHORT version;
+    ULONG dir_offset;
+    USHORT number_of_entries;
+    struct IFD_entry entry[13];
+    ULONG next_IFD;
+    struct IFD_rational resx;
+    struct IFD_rational resy;
+    BYTE pixel_data[4];
+} tiff_resolution_image_data =
+{
+#ifdef WORDS_BIGENDIAN
+    'M' | 'M' << 8,
+#else
+    'I' | 'I' << 8,
+#endif
+    42,
+    FIELD_OFFSET(struct tiff_resolution_image_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, 1 }, /* BITSPERSAMPLE */
+        { 0x103, IFD_SHORT, 1, 1 }, /* COMPRESSION: XP doesn't accept IFD_LONG here */
+        { 0x106, IFD_SHORT, 1, 1 }, /* PHOTOMETRIC */
+        { 0x111, IFD_LONG, 1, FIELD_OFFSET(struct tiff_resolution_image_data, pixel_data) }, /* STRIPOFFSETS */
+        { 0x115, IFD_SHORT, 1, 1 }, /* SAMPLESPERPIXEL */
+        { 0x116, IFD_LONG, 1, 1 }, /* ROWSPERSTRIP */
+        { 0x117, IFD_LONG, 1, 1 }, /* STRIPBYTECOUNT */
+        { 0x11a, IFD_RATIONAL, 1, FIELD_OFFSET(struct tiff_resolution_image_data, resx) }, /* XRESOLUTION */
+        { 0x11b, IFD_RATIONAL, 1, FIELD_OFFSET(struct tiff_resolution_image_data, resy) }, /* YRESOLUTION */
+        { 0x128, IFD_SHORT, 1, 1 }, /* RESOLUTIONUNIT -- value will be filled with test data */
+    },
+    0,
+    { 72, 1 }, /* value will be filled with test data */
+    { 72, 1 }, /* value will be filled with test data */
+    { 0x11, 0x22, 0x33, 0 }
+};
 #include "poppack.h"
 
 static IWICImagingFactory *factory;
@@ -370,6 +446,58 @@ static void test_tiff_8bpp_alpha(void)
     IWICBitmapDecoder_Release(decoder);
 }
 
+static void test_tiff_resolution(void)
+{
+    HRESULT hr;
+    IWICBitmapDecoder *decoder;
+    IWICBitmapFrameDecode *frame;
+    double dpi_x, dpi_y;
+    int i;
+
+    for (i = 0; i < sizeof(tiff_resolution_test_data)/sizeof(tiff_resolution_test_data[0]); i++)
+    {
+        const struct tiff_resolution_test_data *test_data = &tiff_resolution_test_data[i];
+        tiff_resolution_image_data.resx = test_data->resx;
+        tiff_resolution_image_data.resy = test_data->resy;
+        tiff_resolution_image_data.entry[12].value = test_data->resolution_unit;
+
+        decoder = create_decoder(&tiff_resolution_image_data, sizeof(tiff_resolution_image_data));
+        ok(decoder != 0, "%d: Failed to load TIFF image data\n", i);
+        if (!decoder) continue;
+
+        hr = IWICBitmapDecoder_GetFrame(decoder, 0, &frame);
+        ok(hr == S_OK, "%d: GetFrame error %#x\n", i, hr);
+
+        hr = IWICBitmapFrameDecode_GetResolution(frame, &dpi_x, &dpi_y);
+        ok(hr == S_OK, "%d: GetResolution error %#x\n", i, hr);
+
+        if (test_data->broken_dpi_x != 0)
+        {
+            ok(fabs(dpi_x - test_data->expected_dpi_x) < 0.01 || broken(fabs(dpi_x - test_data->broken_dpi_x) < 0.01),
+                "%d: x: expected %f or %f, got %f\n", i, test_data->expected_dpi_x, test_data->broken_dpi_x, dpi_x);
+        }
+        else
+        {
+            ok(fabs(dpi_x - test_data->expected_dpi_x) < 0.01,
+                "%d: x: expected %f, got %f\n", i, test_data->expected_dpi_x, dpi_x);
+        }
+
+        if (test_data->broken_dpi_y != 0)
+        {
+            ok(fabs(dpi_y - test_data->expected_dpi_y) < 0.01 || broken(fabs(dpi_y - test_data->broken_dpi_y) < 0.01),
+                "%d: y: expected %f or %f, got %f\n", i, test_data->expected_dpi_y, test_data->broken_dpi_y, dpi_y);
+        }
+        else
+        {
+            ok(fabs(dpi_y - test_data->expected_dpi_y) < 0.01,
+                "%d: y: expected %f, got %f\n", i, test_data->expected_dpi_y, dpi_y);
+        }
+
+        IWICBitmapFrameDecode_Release(frame);
+        IWICBitmapDecoder_Release(decoder);
+    }
+}
+
 START_TEST(tiffformat)
 {
     HRESULT hr;
@@ -384,6 +512,7 @@ START_TEST(tiffformat)
     test_tiff_palette();
     test_QueryCapability();
     test_tiff_8bpp_alpha();
+    test_tiff_resolution();
 
     IWICImagingFactory_Release(factory);
     CoUninitialize();
diff --git a/dlls/windowscodecs/tiffformat.c b/dlls/windowscodecs/tiffformat.c
index a0eec53f5f..759a621e1a 100644
--- a/dlls/windowscodecs/tiffformat.c
+++ b/dlls/windowscodecs/tiffformat.c
@@ -529,21 +529,27 @@ static HRESULT tiff_get_decode_info(TIFF *tiff, tiff_decode_info *decode_info)
 
     decode_info->resolution_unit = 0;
     pTIFFGetField(tiff, TIFFTAG_RESOLUTIONUNIT, &decode_info->resolution_unit);
-    if (decode_info->resolution_unit != 0)
-    {
-        ret = pTIFFGetField(tiff, TIFFTAG_XRESOLUTION, &decode_info->xres);
-        if (!ret)
-        {
-            WARN("missing X resolution\n");
-            decode_info->resolution_unit = 0;
-        }
 
-        ret = pTIFFGetField(tiff, TIFFTAG_YRESOLUTION, &decode_info->yres);
-        if (!ret)
-        {
-            WARN("missing Y resolution\n");
-            decode_info->resolution_unit = 0;
-        }
+    ret = pTIFFGetField(tiff, TIFFTAG_XRESOLUTION, &decode_info->xres);
+    if (!ret)
+    {
+        WARN("missing X resolution\n");
+    }
+    /* Emulate the behavior of current libtiff versions (libtiff commit a39f6131)
+     * yielding 0 instead of INFINITY for IFD_RATIONAL fields with denominator 0. */
+    if (!isfinite(decode_info->xres))
+    {
+        decode_info->xres = 0.0;
+    }
+
+    ret = pTIFFGetField(tiff, TIFFTAG_YRESOLUTION, &decode_info->yres);
+    if (!ret)
+    {
+        WARN("missing Y resolution\n");
+    }
+    if (!isfinite(decode_info->yres))
+    {
+        decode_info->yres = 0.0;
     }
 
     return S_OK;
@@ -893,26 +899,28 @@ static HRESULT WINAPI TiffFrameDecode_GetResolution(IWICBitmapFrameDecode *iface
 {
     TiffFrameDecode *This = impl_from_IWICBitmapFrameDecode(iface);
 
-    switch (This->decode_info.resolution_unit)
+    if (This->decode_info.xres == 0 || This->decode_info.yres == 0)
     {
-    default:
-        FIXME("unknown resolution unit %i\n", This->decode_info.resolution_unit);
-        /* fall through */
-    case 0: /* Not set */
         *pDpiX = *pDpiY = 96.0;
-        break;
-    case 1: /* Relative measurements */
-        *pDpiX = 96.0;
-        *pDpiY = 96.0 * This->decode_info.yres / This->decode_info.xres;
-        break;
-    case 2: /* Inch */
-        *pDpiX = This->decode_info.xres;
-        *pDpiY = This->decode_info.yres;
-        break;
-    case 3: /* Centimeter */
-        *pDpiX = This->decode_info.xres / 2.54;
-        *pDpiY = This->decode_info.yres / 2.54;
-        break;
+    }
+    else
+    {
+        switch (This->decode_info.resolution_unit)
+        {
+        default:
+            FIXME("unknown resolution unit %i\n", This->decode_info.resolution_unit);
+            /* fall through */
+        case 0: /* Not set */
+        case 1: /* Relative measurements */
+        case 2: /* Inch */
+            *pDpiX = This->decode_info.xres;
+            *pDpiY = This->decode_info.yres;
+            break;
+        case 3: /* Centimeter */
+            *pDpiX = This->decode_info.xres * 2.54;
+            *pDpiY = This->decode_info.yres * 2.54;
+            break;
+        }
     }
 
     TRACE("(%p) <-- %f,%f unit=%i\n", iface, *pDpiX, *pDpiY, This->decode_info.resolution_unit);
-- 
2.16.1




More information about the wine-devel mailing list