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