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

[Jacek Caban]

On 11/3/21 5:12 AM, Jin-oh Kang wrote:
> 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?


Sure, a separated series is fine, it's not strictly related to this series.


>>>> 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 to test what Windows does by calling win32u functions 
directly. Those functions are not really documented nor stable, through, 
and we don't need to follow that very strictly, but it's generally good 
to follow when possible.


Thanks,

Jacek