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

[Jinoh Kang]

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?)
- 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