Re: [PATCH] gdi32: Update enhanced metafile world transform immediately in winnt mode

[Huw Davies]

On Wed, Jul 19, 2017 at 02:06:29PM -0500, Andrew Eikum wrote:
> /*****************************************************************************
> -
>       PlayEnhMetaFileRecord  (GDI32.@)
>
> @@ -1443,14 +1458,20 @@ BOOL WINAPI PlayEnhMetaFileRecord(
>              info->state.world_transform.eM11 = info->state.world_transform.eM22 = 1;
>              info->state.world_transform.eM12 = info->state.world_transform.eM21 = 0;
>              info->state.world_transform.eDx  = info->state.world_transform.eDy  = 0;
> -
>        if (!IS_WIN9X())
> -
>            EMF_Update_MF_Xform(hdc, info);
>        break;
>    case MWT_LEFTMULTIPLY:
>        CombineTransform(&info->state.world_transform, &lpModifyWorldTrans->xform,
>                         &info->state.world_transform);
> -
>        if (!IS_WIN9X())
> -
>            ModifyWorldTransform(hdc, &lpModifyWorldTrans->xform, MWT_LEFTMULTIPLY);
>        break;
>    case MWT_RIGHTMULTIPLY:
>        CombineTransform(&info->state.world_transform, &info->state.world_transform,
>                         &lpModifyWorldTrans->xform);
> -
>        if (!IS_WIN9X())
> -
>            EMF_Update_MF_Xform(hdc, info);
>        break;
>    default:
>        FIXME("Unknown imode %d\n", lpModifyWorldTrans->iMode);
>
> @@ -2216,6 +2237,13 @@ BOOL WINAPI PlayEnhMetaFileRecord(
>    LPtoDP(hdc, (POINT*)&tmprc, 2);
>    TRACE("L:0,0 - 1000,1000 -> D:%s\n", wine_dbgstr_rect(&tmprc));
> - if (emr_modifies_transform( mr->iType ) && !IS_WIN9X())
> - {
> -
>  /* WinNT - update the transform (win9x updates when the next graphics
> -
>     output record is played). */
> -
>  EMF_Update_MF_Xform(hdc, info);
> - }
> - return TRUE;
>
> }

I think it would be better to call EMF_Update_MF_Xform() in each
'case' block that needs it, just like you are doing for
EMR_MODIFYWORLDTRANSFORM.  Also, what about EMR_SETMAPMODE?
> diff --git a/dlls/gdi32/tests/metafile.c b/dlls/gdi32/tests/metafile.c
> index eeec79c800..eaad666361 100644
> --- a/dlls/gdi32/tests/metafile.c
> +++ b/dlls/gdi32/tests/metafile.c

[snip]
> START_TEST(metafile)
>  {
>      init_function_pointers();
> @@ -3928,6 +4430,9 @@ START_TEST(metafile)
>      test_emf_paths();
>      test_emf_PolyPolyline();
>      test_emf_GradientFill();
> - test_emf_WorldTransform_simple();
> - test_emf_WorldTransform_modify();
> - test_emf_WorldTransform_modified_playback();

You have a lot of duplicated code here.  It should be possible to have
a single test_emf_WorldTransform() that uses a table to drive these
three tests.

Could you also make the matrices a little more interesting?  Adding a
translation would make them non-commuting which would be good to test.

Huw.