Re: gdiplus: Reimplement metafile loading using gdi32 instead of IPicture.

[Nikolay Sivov]

On 17.08.2016 11:26, Sebastian Lackner wrote:
> +static GpStatus load_wmf(IStream *stream, GpMetafile **metafile)
> +{
> - WmfPlaceableFileHeader pfh;
> - GpStatus status = GenericError;
> - BOOL is_placeable = FALSE;
> - LARGE_INTEGER seek;
> - METAHEADER mh;
> - HMETAFILE hmf;
> - HRESULT hr;
> - UINT size;
> - void *buf;
> -
> - hr = IStream_Read(stream, &mh, sizeof(mh), &size);
> - if (hr != S_OK || size != sizeof(mh))
> -
>    return GenericError;
> -
> - if (mh.mtType == 0xcdd7 && mh.mtHeaderSize == 0x9ac6)

It'd be more readable to test this as UINT32 it's supposed to be,
and having this constant defined to GDIP_WMF_PLACEABLEKEY (locally in
gdiplus, it's only mentioned in comment in SDK) will help looking it up.
> - {
> -
>    is_placeable = TRUE;
> -
> -
>    seek.QuadPart = 0;
> -
>    hr = IStream_Seek(stream, seek, STREAM_SEEK_SET, NULL);
> -
>    if (FAILED(hr)) return hresult_to_status(hr);
> -
> -
>    hr = IStream_Read(stream, &pfh, sizeof(pfh), &size);
> -
>    if (hr != S_OK || size != sizeof(pfh))
> -
>        return GenericError;
> -
> -
>    hr = IStream_Read(stream, &mh, sizeof(mh), &size);
> -
>    if (hr != S_OK || size != sizeof(mh))
> -
>        return GenericError;
> - }
> -
> - seek.QuadPart = is_placeable ? sizeof(pfh) : 0;
> - hr = IStream_Seek(stream, seek, STREAM_SEEK_SET, NULL);
> - if (FAILED(hr)) return hresult_to_status(hr);
> -
> - buf = heap_alloc(mh.mtSize * 2);
> - if (!buf) return OutOfMemory;
> -
> - hr = IStream_Read(stream, buf, mh.mtSize * 2, &size);
> - if (hr == S_OK && size == mh.mtSize * 2)
> - {
> -
>    hmf = SetMetaFileBitsEx(mh.mtSize * 2, buf);
> -
>    if (hmf)
> -
>    {
> -
>        status = GdipCreateMetafileFromWmf(hmf, TRUE, is_placeable ? &pfh : NULL, metafile);
> -
>        if (status != Ok)
> -
>            DeleteMetaFile(hmf);
> -
>    }
> - }
> -
> - heap_free(buf);
> - return status;
>
> +}
> -
>
> +static GpStatus decode_image_wmf(IStream *stream, GpImage **image)
>  {
> - IPicture *pic;
> -
>
> GpMetafile *metafile;
> -
>
> GpStatus status;
>
> TRACE("%p %p\n", stream, image);
> - if(!stream || !image)
> - if (!stream || !image)
>     return InvalidParameter;
> - if(OleLoadPicture(stream, 0, FALSE, &IID_IPicture,
> -
>    (LPVOID*) &pic) != S_OK){
> -
>    TRACE("Could not load picture\n");
> - status = load_wmf(stream, &metafile);
> - if (status != Ok)
> - {
> -
>    TRACE("Could not load metafile\n");
> -
>    return status;
> - }
> -
> - *image = (GpImage *)metafile;
> - TRACE("<-- %p\n", *image);
> -
> - return Ok;
>
> +}
> -
>
> +static GpStatus load_emf(IStream *stream, GpMetafile **metafile)
> +{
> - GpStatus status = GenericError;
> - LARGE_INTEGER seek;
> - ENHMETAHEADER emh;
> - HENHMETAFILE hemf;
> - HRESULT hr;
> - UINT size;
> - void *buf;
> -
> - hr = IStream_Read(stream, &emh, sizeof(emh), &size);
> - if (hr != S_OK || size != sizeof(emh) || emh.dSignature != ENHMETA_SIGNATURE)
>     return GenericError;
> -
> - seek.QuadPart = 0;
> - hr = IStream_Seek(stream, seek, STREAM_SEEK_SET, NULL);
> - if (FAILED(hr)) return hresult_to_status(hr);
> -
> - buf = heap_alloc(emh.nBytes);
> - if (!buf) return OutOfMemory;
> -
> - hr = IStream_Read(stream, buf, emh.nBytes, &size);
> - if (hr == S_OK && size == emh.nBytes)
> - {
> -
>    hemf = SetEnhMetaFileBits(emh.nBytes, buf);
> -
>    if (hemf)
> -
>    {
> -
>        status = GdipCreateMetafileFromEmf(hemf, TRUE, metafile);
> -
>        if (status != Ok)
> -
>            DeleteEnhMetaFile(hemf);
> -
>    }
> }
> - /* FIXME: missing initialization code */
> - *image = heap_alloc_zero(sizeof(GpMetafile));
> - if(!*image) return OutOfMemory;
> - (*image)->type = ImageTypeMetafile;
> - (*image)->decoder = NULL;
> - (*image)->picture = pic;
> - (*image)->flags   = ImageFlagsNone;
> - (*image)->frame_count = 1;
> - (*image)->current_frame = 0;
> - (*image)->palette = NULL;
> - list_init(&(*(GpMetafile**)image)->containers);
> - heap_free(buf);
> - return status;
>
> +}
> -
>
> +static GpStatus decode_image_emf(IStream *stream, GpImage **image)
> +{
> -
>
> GpMetafile *metafile;
> -
>
> GpStatus status;
> -
> -
>
> TRACE("%p %p\n", stream, image);
> -
> -
>
> if (!stream || !image)
> -
>    return InvalidParameter;
> -
> -
>
> status = load_emf(stream, &metafile);
> -
>
> if (status != Ok)
> -
>
> {
> -
>    TRACE("Could not load metafile\n");
> -
>    return status;
> -
>
> }
> -
>
> *image = (GpImage *)metafile;
> TRACE("<-- %p\n", *image);
>
> return Ok;
>
> @@ -4721,7 +4750,7 @@ static const struct image_codec codecs[NUM_CODECS] = {
>              /* SigMask */            emf_sig_mask,
>          },
>          NULL,
> -
>    decode_image_olepicture_metafile,
> -
>    decode_image_emf,
>    NULL
> },
>    {
>
> @@ -4741,7 +4770,7 @@ static const struct image_codec codecs[NUM_CODECS] = {
>              /* SigMask */            wmf_sig_mask,
>          },
>          NULL,
> -
>    decode_image_olepicture_metafile,
> -
>    decode_image_wmf,
>    NULL
> },
>    {
>
> diff --git a/dlls/gdiplus/metafile.c b/dlls/gdiplus/metafile.c
> index a854e55..8b93e81 100644
> --- a/dlls/gdiplus/metafile.c
> +++ b/dlls/gdiplus/metafile.c
> @@ -317,7 +317,6 @@ GpStatus WINGDIPAPI GdipRecordMetafile(HDC hdc, EmfType type, GDIPCONST GpRectF
>      }
>  (*metafile)->image.type = ImageTypeMetafile;
> - (*metafile)->image.picture = NULL;
> (*metafile)->image.flags   = ImageFlagsNone;
> (*metafile)->image.palette = NULL;
> (*metafile)->image.xres = dpix;
>
> diff --git a/dlls/gdiplus/tests/image.c b/dlls/gdiplus/tests/image.c
> index ad84feb..cabd2fce 100644
> --- a/dlls/gdiplus/tests/image.c
> +++ b/dlls/gdiplus/tests/image.c
> @@ -1446,19 +1446,19 @@ static void test_loadwmf(void)
>  stat = GdipGetImageBounds(img, &bounds, &unit);
>  expect(Ok, stat);
> - todo_wine expect(UnitPixel, unit);
> - expect(UnitPixel, unit);
> expectf(0.0, bounds.X);
> expectf(0.0, bounds.Y);
> - todo_wine expectf(320.0, bounds.Width);
> - todo_wine expectf(320.0, bounds.Height);
> -
>
> expectf(320.0, bounds.Width);
> -
>
> expectf(320.0, bounds.Height);
>
> stat = GdipGetImageHorizontalResolution(img, &res);
> expect(Ok, stat);
> - todo_wine expectf(1440.0, res);
> -
>
> expectf(1440.0, res);
>
> stat = GdipGetImageVerticalResolution(img, &res);
> expect(Ok, stat);
> - todo_wine expectf(1440.0, res);
> -
>
> expectf(1440.0, res);
>
> memset(&header, 0, sizeof(header));
> stat = GdipGetMetafileHeaderFromMetafile((GpMetafile*)img, &header);