[RFC PATCH] gdi32: allow passing BITMAPINFO with bmiHeader.biSizeImage == 0 for EMF DC.

[Jinoh Kang]

On 10/13/21 03:31, Jinoh Kang wrote:
> On 10/6/21 00:40, Jacek Caban wrote:
>> On 10/5/21 3:47 AM, Jinoh Kang wrote:
>>> On 10/5/21 02:17, Jacek Caban wrote:
>>>> Looking at bitmapinfoheader_from_user_bitmapinfo, we should probably ignore biSizeImage for BI_RGB and BI_BITFIELDS. There is also a question about what we should use for biSizeImage recorded as part of BITMAPINFOHEADER. Maybe we should just copy bitmapinfoheader_from_user_bitmapinfo and use it here. Ideally, we'd have a test for that in tests/metafile.c.
>>> Since EMF DC has to serialize the entire BITMAPINFO complete with the colour table, perhaps we should just use bitmapinfo_from_user_bitmapinfo instead. This will make EMFDC_StretchDIBits consistent with NtGdiStretchDIBitsInternal. Ditto for EMFDC_SetDIBitsToDevice (NtGdiSetDIBitsToDeviceInternal).
>>
>>
>> Did you check that it does that on Windows? Another possibility is that it could record an incomplete color table and let NtGdiStretchDIBitsInternal fill it during replay, so a test showing the exact behaviour would be nice.
> 
> I performed a test going roughly as follows on Windows 10, build 2108:
> 
> - rect = {0, 0, 128, 128};
> - hdc = CreateEnhMetaFileW(NULL, NULL, &rect, NULL);  // NOTE current session on truecolor desktop
> - StretchDIBits(hdc, 0, 0, 32, 32, 0, 0, 32, 32, data, bmi, DIB_RGB_COLORS, SRCCOPY);
> 
> The results are as follows:
> 
> - bmiHeader.biCompression:
>   - BI_RGB and BI_BITFIELD: calculates cbBitsSrc based on width/height/depth; ignores bmiHeader.biSizeImage.
>   - BI_RLE4 and BI_RLE8: cbBitsSrc := bmiHeader.biSizeImage. Fails if bmiHeader.biSizeImage is zero.
>   - BI_JPEG and BI_PNG: (unable to test -- fails on my system; perhaps incorrect DC settings?)
> - bmiHeader.biSizeImage:
>   - Preserved as-is in resulting EMF, however large or small.
> - bmiHeader.biClrUsed and bmiHeader.biClrImportant:
>   - Preserved as-is in resulting EMF, however large or small.
>   - Invalid values seem to be also accepted.
>   - Works also on BI_RGB (no checks are done?)

A few corrections:

- bmiHeader.biClrUsed:
  - biBitCount >= 16: Preserved as-is in resulting EMF, however large or small.
    Does not affect cbBmiSrc at all. Invalid values seem to be also accepted.
  - biBitCount < 16: Preserved as-is in resulting EMF, however large or small.
    Adds to cbBmiSrc. biClrUsed=0 means the default palette size; however, the value "0" itself is preserved.
    Unnecessarily large (i.e. size exceeds maximum pixel value) biClrUsed are also accepted and the palette size is increased accordingly.

- bmiHeader.biClrImportant:
  (no change)

> - Usage of BITMAPCOREINFO instead of BITMAPINFO (size == 12):
>   - Normalized to BITMAPINFOHEADER (size == 40).
> 
> These behaviours closely resemble bitmapinfoheader_from_user_bitmapinfo _sans_ in-line mutation of biSizeImage.
> Perhaps we can write the calculated size to a separate pass-by-ref variable, and change the original usage as follows:
> 
>     -bitmapinfoheader_from_user_bitmapinfo(&dst->bmiHeader, &info->bmiHeader);
>     +bitmapinfoheader_from_user_bitmapinfo(&dst->bmiHeader, &info->bmiHeader, &dst->bmiHeader.biSizeImage);
> 
> Alternatively, we should factor it out into a separate function.
> 
>>
>>
>>> That said, there seems to be many other places that directly receive BITMAPINFO from user as well (e.g. metadc.c).
>>>
>>> IMHO it would be a good idea to just make the bitmapinfo{,header}_from_user_bitmapinfo function (dib.c) extern, and use it appropriately in future patches. Are you open to this option?
>>
>>
>> Yes, if we need such helper it would be nice to share it with metadc.c, but it's not possible to do that from dib.c, which is in Unix lib now. We will need a copy of it on PE side.
> 
> Since it's just the header (not the entire BITMAPINFO), copying the code sounds like a reasonable choice.
> Also we have a slight semantic difference as above, so we can't use the original one as is anyway.
> (Perhaps rename to bitmapinfoheader_and_size_from_user_bitmapinfo in the copied version?)
> 
>>
>>
>> Thanks,
>>
>> Jacek
>>
> 

-- 
Sincerely,
Jinoh Kang