Re: [PATCH v2 3/5] ole32: Add support for loading wmf from contents streams to data cache.

[Huw Davies]

On Fri, Mar 30, 2018 at 12:11:35PM -0500, Sergio Gómez Del Real wrote:
> Signed-off-by: Sergio Gómez Del Real
>
> dlls/ole32/datacache.c | 71 +++++++++++++++++++++++++++++---------------------
>  1 file changed, 42 insertions(+), 29 deletions(-)
>
> diff --git a/dlls/ole32/datacache.c b/dlls/ole32/datacache.c
> index e6cda331dd..0f23faf08c 100644
> --- a/dlls/ole32/datacache.c
> +++ b/dlls/ole32/datacache.c
> @@ -609,6 +609,17 @@ static HRESULT synthesize_emf( HMETAFILEPICT data, STGMEDIUM *med )
>      GlobalUnlock( data );
>      return hr;
>  }
> +#include <pshpack2.h>
> +struct meta_placeable
> +{
> - DWORD key;
> - WORD hwmf;
> - WORD bounding_box[4];
> - WORD inch;
> - DWORD reserved;
> - WORD checksum;
>
> +};
> +#include <poppack.h>
>
> static HRESULT read_pres_header( IStream *stm, DWORD *clip_format,
>                                   PresentationDataHeader *pres_hdr )
> @@ -629,28 +640,34 @@ static HRESULT load_mf_pict( DataCacheEntry *cache_entry, IStream *stm )
>      HRESULT hr;
>      STATSTG stat;
>      ULARGE_INTEGER current_pos;
> - void *bits;
> - void *bits, *hdr;
> METAFILEPICT *mfpict;
> HGLOBAL hmfpict;
> - PresentationDataHeader header;
> - CLIPFORMAT clipformat;
> static const LARGE_INTEGER offset_zero;
> - ULONG read;
> -
> - if (cache_entry->load_stream_num == STREAM_NUMBER_CONTENTS)
> - {
> -
>    FIXME( "Unimplemented for CONTENTS stream\n" );
> -
>    return E_FAIL;
> - }
> -
>
> ULONG read, hdr_size;
> -
>
> /* only standard clipformats */
> -
>
> struct {
> -
>    DWORD cf[2];
> -
>    PresentationDataHeader pres;
> -
>
> } pres_hdr;
> -
>
> struct meta_placeable meta_place_rec;
>
> hr = IStream_Stat( stm, &stat, STATFLAG_NONAME );
> if (FAILED( hr )) return hr;
> - hr = read_clipformat( stm, &clipformat );
> - if (FAILED( hr )) return hr;
> - if (cache_entry->load_stream_num != STREAM_NUMBER_CONTENTS)
> - {
> -
>    hdr_size = sizeof(pres_hdr);
> -
>    hdr = &pres_hdr;
> - }
> - else
> - {
> -
>    hdr_size = sizeof(struct meta_placeable);
> -
>    hdr = &meta_place_rec;
> - }

I don't think setting up hdr and hdr_size in order to have a common
IStream_Read makes much sense.  Keep the original behaviour for the
presentation streams in one if block and implement the reading of
meta_placable in the else block.
> - hr = IStream_Read( stm, &header, sizeof(header), &read );
> - if (hr != S_OK || read != sizeof(header)) return E_FAIL;
> -
>
> hr = IStream_Read( stm, hdr, hdr_size, &read );
> -
>
> if (hr != S_OK || read != hdr_size) return E_FAIL;
>
> hr = IStream_Seek( stm, offset_zero, STREAM_SEEK_CUR, &current_pos );
> if (FAILED( hr )) return hr;
>
> @@ -673,10 +690,18 @@ static HRESULT load_mf_pict( DataCacheEntry *cache_entry, IStream *stm )
>  if (SUCCEEDED( hr ))
>  {
> -
>    /* FIXME: get this from the stream */
>    mfpict->mm = MM_ANISOTROPIC;
> -
>    mfpict->xExt = header.dwObjectExtentX;
> -
>    mfpict->yExt = header.dwObjectExtentY;
> -
>    /* FIXME: get this from the stream */
> -
>    if (cache_entry->load_stream_num != STREAM_NUMBER_CONTENTS)
> -
>    {
> -
>        mfpict->xExt = pres_hdr.pres.dwObjectExtentX;
> -
>        mfpict->yExt = pres_hdr.pres.dwObjectExtentY;
> -
>    }
> -
>    else
> -
>    {
> -
>        mfpict->xExt = (meta_place_rec.bounding_box[2] * 2540) / meta_place_rec.inch;

This should probably be (bounding_box[2] - bounding_box[0]) * ...
unless you have evidence otherwise.

You may want to shorten "meta_place_rec" to something like "mf_place"...
> -
>        mfpict->yExt = (meta_place_rec.bounding_box[3] * 2540) / meta_place_rec.inch;
> -
>    }
>    mfpict->hMF = SetMetaFileBitsEx( stat.cbSize.u.LowPart, bits );
>    if (!mfpict->hMF)
>        hr = E_FAIL;
>
> @@ -964,18 +989,6 @@ end:
>      return hr;
>  }
>
> -#include <pshpack2.h>
> -struct meta_placeable
> -{
> - DWORD key;
> - WORD hwmf;
> - WORD bounding_box[4];
> - WORD inch;
> - DWORD reserved;
> - WORD checksum;
>
> -};
> -#include <poppack.h>
> -
>
> static HRESULT save_mfpict(DataCacheEntry *entry, BOOL contents, IStream *stream)
>  {
>      HRESULT hr = S_OK;
> -- 
> 2.14.1