[PATCH v5 3/7] win32u: fix return value of StretchDIBits for EMF DC.

[Jin-oh Kang]

On Wed, Nov 3, 2021 at 1:09 AM Jacek Caban <jacek at codeweavers.com> wrote:
>
> On 11/2/21 4:46 AM, Jin-oh Kang wrote:
> > On Tue, Nov 2, 2021 at 6:21 AM Jacek Caban <jacek at codeweavers.com> wrote:
> >> Hi Jinoh,
> >>
> >> On 11/1/21 6:30 AM, Jinoh Kang wrote:
> >>> Signed-off-by: Jinoh Kang <jinoh.kang.kr at gmail.com>
> >>> ---
> >>>    dlls/win32u/emfdrv.c | 2 +-
> >>>    1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/dlls/win32u/emfdrv.c b/dlls/win32u/emfdrv.c
> >>> index e05f11d7b9a..395d51048d8 100644
> >>> --- a/dlls/win32u/emfdrv.c
> >>> +++ b/dlls/win32u/emfdrv.c
> >>> @@ -408,7 +408,7 @@ static INT CDECL EMFDRV_StretchDIBits( PHYSDEV dev, INT x_dst, INT y_dst, INT wi
> >>>                                           UINT wUsage, DWORD dwRop )
> >>>    {
> >>>        /* FIXME: Update bound rect */
> >>> -    return height_src;
> >>> +    return abs( info->bmiHeader.biHeight );
> >>>    }
> >>>
> >>>    static INT CDECL EMFDRV_SetDIBitsToDevice( PHYSDEV dev, INT x_dst, INT y_dst, DWORD width,
> >>
> >> For this and the next patch, you could simply remove
> >> EMFDRV_StretchDIBits and EMFDRV_SetDIBitsToDevice and let the null
> >> driver take care of it.
> > The nulldrv code is still broken w.r.t. BI_RLE[48] case, so I guess
> > that side needs fixing?
>
>
> If it's broken then ideally yes, but not necessarily in this series. A
> quick testing with tests from your patches works fine.

My gdi32 emf tests didn't cover the BI_RLE[48] cases mainly for
brevity reasons--metafile.c was approaching ~10kloc at this point.
Mind if I move all the fixtures to .rc files in the next patch series
where I would (possibly) fix the nulldrv code to match EMF and cover
more pathological cases?

>
>
> >> I originally left it because of bound rect
> >> update (that's supposed to happen in win32u instead of of gdi32), but
> >> that doesn't seem worth keeping it now.
> > I suppose the most ideal way to deal with it is to do normalization
> > early (in either gdi32 or NtGdi* functions) so that the calculation
> > code doesn't have to be duplicated between emfdc.c, EMFDRV and
> > nulldrv. Currently the args are passed as-is to both EMFDC and NtGdi*,
> > and the normalization work is done *twice*.
>
>
> NtGdi* functions generally need to validate input. They are syscalls
> which generally can't trust callers much and we mostly follow what
> Windows does here.

I agree.  A thing to note, though, is that in general validation !=
normalisation (although the latter sometimes helps sanitize the
input).  It is possible that most normalization is done on the GDI32
side, and win32k.sys accepts whatever it seems as valid.  Then again,
this is a pure speculation and I'm not an NT developer.

> It's possible that gdi32.dll should be more involved,
> but we will still need some duplication.
>
>
> EMFDRV is a different story. Most of those functions should probably
> just be removed. They are there because we should move bounding rect
> calculation for EMFs to win32u. However, I later found that there is an
> undocumented generic way to request that for any DC. It's currently not
> implemented, but ideally we'd use that. Once implemented, it should live
> in common DC code, so most of EMFDRV entries will probably not be needed.
>
>
> Thanks,
>
> Jacek
>