[PATCH v2 0/10] MR9520: VMR7 fixes
For Resident Evil 2 Classic. These are relatively small and simple patches, but I can certainly cut at 5 or so for a first MR. -- v2: ddraw/tests: Test FourCC surface creation with DDSCAPS_OFFSCREENPLAIN. quartz/tests: Test VMR7 AllocateSurface with a BITMAPV4HEADER. quartz/tests: Test allocating BI_BITFIELDS pixel format. quartz/tests: Test allocating a surface with different bit depth from the primary. quartz/vmr7: Validate BITMAPINFOHEADER size. quartz/vmr7: Use the ddraw clipper to compute the presentation rect. quartz/vmr7: Reject unsupported FOURCC formats. ddraw: Advertise NV12 FOURCC as supported. quartz/vmr7: Reject BI_RGB and BI_BITFIELDS formats with different bit depth. quartz/vmr7: Handle BI_BITFIELDS formats. https://gitlab.winehq.org/wine/wine/-/merge_requests/9520
From: Henri Verbeet <hverbeet(a)locutus.nl> --- dlls/quartz/vmr7.c | 2 +- dlls/quartz/vmr7_presenter.c | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/dlls/quartz/vmr7.c b/dlls/quartz/vmr7.c index 83f8b1d62d3..4263e79d54f 100644 --- a/dlls/quartz/vmr7.c +++ b/dlls/quartz/vmr7.c @@ -255,7 +255,7 @@ static HRESULT vmr_render(struct strmbase_renderer *iface, IMediaSample *sample) copy_plane(&dst, surface_desc.lPitch / 2, surface_desc.dwHeight / 2, &src, src_pitch / 2, height / 2); copy_plane(&dst, surface_desc.lPitch / 2, surface_desc.dwHeight / 2, &src, src_pitch / 2, height / 2); } - else if (height > 0 && bitmap_header->biCompression == BI_RGB) + else if (height > 0 && (bitmap_header->biCompression == BI_RGB || bitmap_header->biCompression == BI_BITFIELDS)) { BYTE *dst = surface_desc.lpSurface; const BYTE *src = data; diff --git a/dlls/quartz/vmr7_presenter.c b/dlls/quartz/vmr7_presenter.c index c244c86236a..90a0b906610 100644 --- a/dlls/quartz/vmr7_presenter.c +++ b/dlls/quartz/vmr7_presenter.c @@ -201,6 +201,16 @@ static HRESULT WINAPI surface_allocator_AllocateSurface(IVMRSurfaceAllocator *if surface_desc.ddpfPixelFormat.dwGBitMask = 0x0000ff00; surface_desc.ddpfPixelFormat.dwBBitMask = 0x000000ff; } + else if (info->lpHdr->biCompression == BI_BITFIELDS) + { + const DWORD *mask = (DWORD *)((BITMAPINFO *)info->lpHdr)->bmiColors; + + surface_desc.ddpfPixelFormat.dwFlags = DDPF_RGB; + surface_desc.ddpfPixelFormat.dwRGBBitCount = info->lpHdr->biBitCount; + surface_desc.ddpfPixelFormat.dwRBitMask = mask[0]; + surface_desc.ddpfPixelFormat.dwGBitMask = mask[1]; + surface_desc.ddpfPixelFormat.dwBBitMask = mask[2]; + } else { surface_desc.ddpfPixelFormat.dwFlags = DDPF_FOURCC; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9520
From: Henri Verbeet <hverbeet(a)locutus.nl> --- dlls/quartz/vmr7_presenter.c | 51 ++++++++++++++++++++++-------------- 1 file changed, 31 insertions(+), 20 deletions(-) diff --git a/dlls/quartz/vmr7_presenter.c b/dlls/quartz/vmr7_presenter.c index 90a0b906610..1abee7b957d 100644 --- a/dlls/quartz/vmr7_presenter.c +++ b/dlls/quartz/vmr7_presenter.c @@ -187,29 +187,40 @@ static HRESULT WINAPI surface_allocator_AllocateSurface(IVMRSurfaceAllocator *if surface_desc.ddsCaps.dwCaps = DDSCAPS_FLIP | DDSCAPS_COMPLEX | DDSCAPS_OFFSCREENPLAIN; surface_desc.dwBackBufferCount = *count; - if (info->lpHdr->biCompression == BI_RGB) + if (info->lpHdr->biCompression == BI_RGB || info->lpHdr->biCompression == BI_BITFIELDS) { - if (info->lpHdr->biBitCount != 32) - { - FIXME("Unhandled bit depth %u.\n", info->lpHdr->biBitCount); - return E_NOTIMPL; - } + DDSURFACEDESC2 primary_desc; + DWORD *mask; - surface_desc.ddpfPixelFormat.dwFlags = DDPF_RGB; - surface_desc.ddpfPixelFormat.dwRGBBitCount = 32; - surface_desc.ddpfPixelFormat.dwRBitMask = 0x00ff0000; - surface_desc.ddpfPixelFormat.dwGBitMask = 0x0000ff00; - surface_desc.ddpfPixelFormat.dwBBitMask = 0x000000ff; - } - else if (info->lpHdr->biCompression == BI_BITFIELDS) - { - const DWORD *mask = (DWORD *)((BITMAPINFO *)info->lpHdr)->bmiColors; + primary_desc.dwSize = sizeof(primary_desc); + if (FAILED(hr = IDirectDrawSurface7_GetSurfaceDesc(presenter->primary, &primary_desc))) + return hr; + if (info->lpHdr->biBitCount != primary_desc.ddpfPixelFormat.dwRGBBitCount) + return E_FAIL; - surface_desc.ddpfPixelFormat.dwFlags = DDPF_RGB; - surface_desc.ddpfPixelFormat.dwRGBBitCount = info->lpHdr->biBitCount; - surface_desc.ddpfPixelFormat.dwRBitMask = mask[0]; - surface_desc.ddpfPixelFormat.dwGBitMask = mask[1]; - surface_desc.ddpfPixelFormat.dwBBitMask = mask[2]; + if (info->lpHdr->biCompression == BI_RGB) + { + if (info->lpHdr->biBitCount != 32) + { + FIXME("Unhandled bit depth %u.\n", info->lpHdr->biBitCount); + return E_NOTIMPL; + } + + surface_desc.ddpfPixelFormat.dwFlags = DDPF_RGB; + surface_desc.ddpfPixelFormat.dwRGBBitCount = 32; + surface_desc.ddpfPixelFormat.dwRBitMask = 0x00ff0000; + surface_desc.ddpfPixelFormat.dwGBitMask = 0x0000ff00; + surface_desc.ddpfPixelFormat.dwBBitMask = 0x000000ff; + } + else + { + mask = (DWORD *)((BITMAPINFO *)info->lpHdr)->bmiColors; + surface_desc.ddpfPixelFormat.dwFlags = DDPF_RGB; + surface_desc.ddpfPixelFormat.dwRGBBitCount = info->lpHdr->biBitCount; + surface_desc.ddpfPixelFormat.dwRBitMask = mask[0]; + surface_desc.ddpfPixelFormat.dwGBitMask = mask[1]; + surface_desc.ddpfPixelFormat.dwBBitMask = mask[2]; + } } else { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9520
From: Matteo Bruni <mbruni(a)codeweavers.com> --- dlls/ddraw/ddraw.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/ddraw/ddraw.c b/dlls/ddraw/ddraw.c index 3e66b134a75..fb8895f01b2 100644 --- a/dlls/ddraw/ddraw.c +++ b/dlls/ddraw/ddraw.c @@ -1794,7 +1794,7 @@ static HRESULT WINAPI ddraw7_GetFourCCCodes(IDirectDraw7 *iface, DWORD *NumCodes { WINED3DFMT_YUY2, WINED3DFMT_UYVY, WINED3DFMT_YV12, WINED3DFMT_DXT1, WINED3DFMT_DXT2, WINED3DFMT_DXT3, WINED3DFMT_DXT4, WINED3DFMT_DXT5, - WINED3DFMT_ATI2N, WINED3DFMT_NVHU, WINED3DFMT_NVHS + WINED3DFMT_ATI2N, WINED3DFMT_NVHU, WINED3DFMT_NVHS, WINED3DFMT_NV12, }; struct wined3d_display_mode mode; DWORD count = 0, i, outsize; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9520
From: Henri Verbeet <hverbeet(a)locutus.nl> --- dlls/quartz/vmr7_presenter.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/dlls/quartz/vmr7_presenter.c b/dlls/quartz/vmr7_presenter.c index 1abee7b957d..29e9f31c49a 100644 --- a/dlls/quartz/vmr7_presenter.c +++ b/dlls/quartz/vmr7_presenter.c @@ -171,6 +171,37 @@ static ULONG WINAPI surface_allocator_Release(IVMRSurfaceAllocator *iface) return IVMRImagePresenter_Release(&presenter->IVMRImagePresenter_iface); } +static BOOL fourcc_is_supported(IDirectDraw7 *ddraw, DWORD fourcc) +{ + DWORD *codes, count, i; + HRESULT hr; + + if (FAILED(hr = IDirectDraw7_GetFourCCCodes(ddraw, &count, NULL))) + { + ERR("Failed to get FOURCC code count, hr %#lx.\n", hr); + return FALSE; + } + + if (!count || !(codes = calloc(count, sizeof(*codes)))) + return FALSE; + + if (FAILED(hr = IDirectDraw7_GetFourCCCodes(ddraw, &count, codes))) + { + ERR("Failed to get FOURCC codes, hr %#lx.\n", hr); + free(codes); + return FALSE; + } + + for (i = 0; i < count; ++i) + { + if (codes[i] == fourcc) + break; + } + free(codes); + + return i < count; +} + static HRESULT WINAPI surface_allocator_AllocateSurface(IVMRSurfaceAllocator *iface, DWORD_PTR id, VMRALLOCATIONINFO *info, DWORD *count, IDirectDrawSurface7 **surface) { @@ -224,6 +255,9 @@ static HRESULT WINAPI surface_allocator_AllocateSurface(IVMRSurfaceAllocator *if } else { + if (!fourcc_is_supported(presenter->ddraw, info->lpHdr->biCompression)) + return VFW_E_DDRAW_CAPS_NOT_SUITABLE; + surface_desc.ddpfPixelFormat.dwFlags = DDPF_FOURCC; surface_desc.ddpfPixelFormat.dwFourCC = info->lpHdr->biCompression; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9520
From: Henri Verbeet <hverbeet(a)locutus.nl> --- dlls/quartz/vmr7_presenter.c | 48 ++++++++++++++++++++++++++++++++---- 1 file changed, 43 insertions(+), 5 deletions(-) diff --git a/dlls/quartz/vmr7_presenter.c b/dlls/quartz/vmr7_presenter.c index 29e9f31c49a..41f8d0d12a6 100644 --- a/dlls/quartz/vmr7_presenter.c +++ b/dlls/quartz/vmr7_presenter.c @@ -105,11 +105,47 @@ static HRESULT WINAPI image_presenter_StopPresenting(IVMRImagePresenter *iface, return E_NOTIMPL; } +static BOOL get_clip_rect(struct vmr7_presenter *presenter, RECT *r) +{ + IDirectDrawClipper *clipper; + RGNDATA *data; + HRESULT hr; + DWORD size; + + if (FAILED(hr = IDirectDrawSurface7_GetClipper(presenter->primary, &clipper))) + return FALSE; + + if (FAILED(hr = IDirectDrawClipper_GetClipList(clipper, NULL, NULL, &size))) + { + IDirectDrawClipper_Release(clipper); + return FALSE; + } + + if (!(data = malloc(size))) + { + IDirectDrawClipper_Release(clipper); + return FALSE; + } + + if (FAILED(hr = IDirectDrawClipper_GetClipList(clipper, NULL, data, &size))) + { + free(data); + IDirectDrawClipper_Release(clipper); + return FALSE; + } + + *r = data->rdh.rcBound; + + free(data); + IDirectDrawClipper_Release(clipper); + + return TRUE; +} + static HRESULT WINAPI image_presenter_PresentImage(IVMRImagePresenter *iface, DWORD_PTR cookie, VMRPRESENTATIONINFO *info) { struct vmr7_presenter *presenter = impl_from_IVMRImagePresenter(iface); - POINT point; HRESULT hr; RECT rect; @@ -125,10 +161,12 @@ static HRESULT WINAPI image_presenter_PresentImage(IVMRImagePresenter *iface, if (info->dwFlags & VMRSample_SrcDstRectsValid) FIXME("Ignoring src/dst rects.\n"); - GetClientRect(presenter->window, &rect); - point.x = point.y = 0; - ClientToScreen(presenter->window, &point); - OffsetRect(&rect, point.x, point.y); + if (!get_clip_rect(presenter, &rect)) + { + ERR("Failed to get clip rect.\n"); + return E_FAIL; + } + if (FAILED(hr = IDirectDrawSurface7_Blt(presenter->primary, &rect, info->lpSurf, NULL, DDBLT_WAIT, NULL))) ERR("Failed to blit, hr %#lx.\n", hr); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9520
From: Matteo Bruni <mbruni(a)codeweavers.com> --- dlls/quartz/vmr7_presenter.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/dlls/quartz/vmr7_presenter.c b/dlls/quartz/vmr7_presenter.c index 41f8d0d12a6..67414415592 100644 --- a/dlls/quartz/vmr7_presenter.c +++ b/dlls/quartz/vmr7_presenter.c @@ -249,6 +249,12 @@ static HRESULT WINAPI surface_allocator_AllocateSurface(IVMRSurfaceAllocator *if TRACE("presenter %p, id %#Ix, info %p, count %p, surface %p.\n", presenter, id, info, count, surface); + if (info->lpHdr->biSize != sizeof(*info->lpHdr)) + { + WARN("Invalid BITMAPINFOHEADER size %lu.\n", info->lpHdr->biSize); + return DDERR_INVALIDPIXELFORMAT; + } + surface_desc.dwFlags = DDSD_WIDTH | DDSD_HEIGHT | DDSD_PIXELFORMAT | DDSD_CAPS | DDSD_BACKBUFFERCOUNT; surface_desc.dwWidth = info->lpHdr->biWidth; surface_desc.dwHeight = info->lpHdr->biHeight; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9520
From: Matteo Bruni <mbruni(a)codeweavers.com> --- dlls/quartz/tests/vmr7.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/dlls/quartz/tests/vmr7.c b/dlls/quartz/tests/vmr7.c index 9502a994576..76c30bb716c 100644 --- a/dlls/quartz/tests/vmr7.c +++ b/dlls/quartz/tests/vmr7.c @@ -3689,11 +3689,13 @@ static void test_default_presenter_allocate(void) { WORD depth; DWORD compression; - DDPIXELFORMAT format; + BOOL expect_failure; } tests[] = { {32, BI_RGB}, + {16, BI_RGB, .expect_failure = TRUE}, + {24, BI_RGB, .expect_failure = TRUE}, {12, mmioFOURCC('N','V','1','2')}, {12, mmioFOURCC('Y','V','1','2')}, {16, mmioFOURCC('U','Y','V','Y')}, @@ -3769,11 +3771,14 @@ static void test_default_presenter_allocate(void) IDirectDraw7_Release(ddraw); + if (tests[i].expect_failure) + expect_hr = E_FAIL; hr = IVMRSurfaceAllocator_AllocateSurface(allocator, 0, &info, &count, &frontbuffer); ok(hr == expect_hr, "Got hr %#lx.\n", hr); - if (hr == VFW_E_DDRAW_CAPS_NOT_SUITABLE) + if (FAILED(hr)) { - skip("Format is not supported.\n"); + if (hr == VFW_E_DDRAW_CAPS_NOT_SUITABLE) + skip("Format is not supported.\n"); winetest_pop_context(); continue; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9520
From: Matteo Bruni <mbruni(a)codeweavers.com> --- dlls/quartz/tests/vmr7.c | 41 ++++++++++++++++++++++++++++++++++------ 1 file changed, 35 insertions(+), 6 deletions(-) diff --git a/dlls/quartz/tests/vmr7.c b/dlls/quartz/tests/vmr7.c index 76c30bb716c..c650587d4bf 100644 --- a/dlls/quartz/tests/vmr7.c +++ b/dlls/quartz/tests/vmr7.c @@ -3685,6 +3685,21 @@ static void test_default_presenter_allocate(void) .biPlanes = 1, }; + /* This works as BITMAPINFOHEADER + the three color masks for BI_BITFIELDS. */ + BITMAPV4HEADER bitmap_v4_header = + { + .bV4Size = sizeof(BITMAPV4HEADER), + .bV4Width = 32, + .bV4Height = 16, + .bV4Planes = 1, + .bV4BitCount = 16, + .bV4V4Compression = BI_BITFIELDS, + .bV4RedMask = 0x00ff0000, + .bV4GreenMask = 0x0000ff00, + .bV4BlueMask = 0x000000ff, + .bV4AlphaMask = 0, + }; + static const struct { WORD depth; @@ -3700,6 +3715,9 @@ static void test_default_presenter_allocate(void) {12, mmioFOURCC('Y','V','1','2')}, {16, mmioFOURCC('U','Y','V','Y')}, {16, mmioFOURCC('Y','U','Y','2')}, + {32, BI_BITFIELDS}, + {16, BI_BITFIELDS, .expect_failure = TRUE}, + {24, BI_BITFIELDS, .expect_failure = TRUE}, }; window = CreateWindowA("static", "quartz_test", WS_OVERLAPPEDWINDOW, 0, 0, @@ -3718,7 +3736,6 @@ static void test_default_presenter_allocate(void) info.dwInterlaceFlags = 0; info.szNativeSize.cx = info.szAspectRatio.cx = 640; info.szNativeSize.cy = info.szAspectRatio.cy = 480; - info.lpHdr = &bitmap_header; info.lpPixFmt = NULL; for (unsigned int i = 0; i < ARRAY_SIZE(tests); ++i) @@ -3727,10 +3744,22 @@ static void test_default_presenter_allocate(void) HRESULT expect_hr; DWORD count = 2; - winetest_push_context("Compression %#lx, depth %u", tests[i].compression, tests[i].depth); + winetest_push_context("Test %u: Compression %#lx, depth %u", i, tests[i].compression, tests[i].depth); - bitmap_header.biBitCount = tests[i].depth; - bitmap_header.biCompression = tests[i].compression; + if (tests[i].compression == BI_BITFIELDS) + { + info.lpHdr = (BITMAPINFOHEADER *)&bitmap_v4_header; + bitmap_v4_header.bV4BitCount = tests[i].depth; + bitmap_v4_header.bV4V4Compression = tests[i].compression; + bitmap_v4_header.bV4Size = sizeof(BITMAPINFOHEADER); + } + else + { + bitmap_header.biBitCount = tests[i].depth; + bitmap_header.biCompression = tests[i].compression; + info.lpHdr = &bitmap_header; + bitmap_header.biSize = sizeof(BITMAPINFOHEADER); + } ddraw = create_ddraw(window); @@ -3743,7 +3772,7 @@ static void test_default_presenter_allocate(void) desc.dwWidth = desc.dwHeight = 32; desc.dwBackBufferCount = 2; desc.ddpfPixelFormat.dwSize = sizeof(desc.ddpfPixelFormat); - if (tests[i].compression) + if (tests[i].compression != BI_RGB && tests[i].compression != BI_BITFIELDS) { desc.ddpfPixelFormat.dwFlags = DDPF_FOURCC; desc.ddpfPixelFormat.dwFourCC = tests[i].compression; @@ -3799,7 +3828,7 @@ static void test_default_presenter_allocate(void) ok(desc.dwHeight == 16, "Got height %lu.\n", desc.dwHeight); ok(desc.ddpfPixelFormat.dwSize == sizeof(desc.ddpfPixelFormat), "Got size %lu.\n", desc.ddpfPixelFormat.dwSize); - if (tests[i].compression) + if (tests[i].compression != BI_RGB && tests[i].compression != BI_BITFIELDS) { ok(desc.ddpfPixelFormat.dwFlags == DDPF_FOURCC, "Got flags %#lx.\n", desc.ddpfPixelFormat.dwFlags); ok(desc.ddpfPixelFormat.dwFourCC == bitmap_header.biCompression, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9520
From: Matteo Bruni <mbruni(a)codeweavers.com> --- dlls/quartz/tests/vmr7.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/dlls/quartz/tests/vmr7.c b/dlls/quartz/tests/vmr7.c index c650587d4bf..465a94da8bb 100644 --- a/dlls/quartz/tests/vmr7.c +++ b/dlls/quartz/tests/vmr7.c @@ -3704,6 +3704,7 @@ static void test_default_presenter_allocate(void) { WORD depth; DWORD compression; + BOOL v4; BOOL expect_failure; } tests[] = @@ -3718,6 +3719,7 @@ static void test_default_presenter_allocate(void) {32, BI_BITFIELDS}, {16, BI_BITFIELDS, .expect_failure = TRUE}, {24, BI_BITFIELDS, .expect_failure = TRUE}, + {32, BI_BITFIELDS, .v4 = TRUE, .expect_failure = TRUE}, }; window = CreateWindowA("static", "quartz_test", WS_OVERLAPPEDWINDOW, 0, 0, @@ -3746,12 +3748,15 @@ static void test_default_presenter_allocate(void) winetest_push_context("Test %u: Compression %#lx, depth %u", i, tests[i].compression, tests[i].depth); - if (tests[i].compression == BI_BITFIELDS) + if (tests[i].v4 || tests[i].compression == BI_BITFIELDS) { info.lpHdr = (BITMAPINFOHEADER *)&bitmap_v4_header; bitmap_v4_header.bV4BitCount = tests[i].depth; bitmap_v4_header.bV4V4Compression = tests[i].compression; - bitmap_v4_header.bV4Size = sizeof(BITMAPINFOHEADER); + if (tests[i].v4) + bitmap_v4_header.bV4Size = sizeof(bitmap_v4_header); + else + bitmap_v4_header.bV4Size = sizeof(BITMAPINFOHEADER); } else { @@ -3801,7 +3806,7 @@ static void test_default_presenter_allocate(void) IDirectDraw7_Release(ddraw); if (tests[i].expect_failure) - expect_hr = E_FAIL; + expect_hr = tests[i].v4 ? DDERR_INVALIDPIXELFORMAT : E_FAIL; hr = IVMRSurfaceAllocator_AllocateSurface(allocator, 0, &info, &count, &frontbuffer); ok(hr == expect_hr, "Got hr %#lx.\n", hr); if (FAILED(hr)) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9520
From: Matteo Bruni <mbruni(a)codeweavers.com> --- dlls/ddraw/tests/ddraw4.c | 46 +++++++++++++++++++++++++++++---------- dlls/ddraw/tests/ddraw7.c | 46 +++++++++++++++++++++++++++++---------- 2 files changed, 70 insertions(+), 22 deletions(-) diff --git a/dlls/ddraw/tests/ddraw4.c b/dlls/ddraw/tests/ddraw4.c index 60eaa3ec8d0..aedee1ed526 100644 --- a/dlls/ddraw/tests/ddraw4.c +++ b/dlls/ddraw/tests/ddraw4.c @@ -6375,21 +6375,19 @@ static void test_block_formats_creation(void) {MAKEFOURCC('D','X','T','5'), "D3DFMT_DXT5", SUPPORT_DXT5, 4, 4, 16, TRUE, FALSE}, {MAKEFOURCC('Y','U','Y','2'), "D3DFMT_YUY2", SUPPORT_YUY2, 2, 1, 4, FALSE, TRUE }, {MAKEFOURCC('U','Y','V','Y'), "D3DFMT_UYVY", SUPPORT_UYVY, 2, 1, 4, FALSE, TRUE }, + {MAKEFOURCC('I','V','5','0'), "D3DFMT_UYVY", SUPPORT_UYVY, 2, 1, 4, FALSE, TRUE }, + {MAKEFOURCC('N','O','P','E'), "D3DFMT_UYVY", SUPPORT_UYVY, 2, 1, 4, FALSE, TRUE }, }; static const struct { DWORD caps, caps2; const char *name; BOOL overlay; + BOOL allow_nondxt; + BOOL allow_all; } types[] = { - /* DDSCAPS_OFFSCREENPLAIN | DDSCAPS_SYSTEMMEMORY fails to create any fourcc - * surface with DDERR_INVALIDPIXELFORMAT. Don't care about it for now. - * - * Nvidia returns E_FAIL on DXTN DDSCAPS_OFFSCREENPLAIN | DDSCAPS_VIDEOMEMORY. - * Other hw / drivers successfully create those surfaces. Ignore them, this - * suggests that no game uses this, otherwise Nvidia would support it. */ { DDSCAPS_VIDEOMEMORY | DDSCAPS_TEXTURE, 0, "videomemory texture", FALSE @@ -6405,7 +6403,22 @@ static void test_block_formats_creation(void) { DDSCAPS_TEXTURE, DDSCAPS2_TEXTUREMANAGE, "managed texture", FALSE - } + }, + { + DDSCAPS_OFFSCREENPLAIN, 0, + "offscreen plain surface", FALSE, + .allow_nondxt = TRUE, .allow_all = TRUE, + }, + { + DDSCAPS_VIDEOMEMORY | DDSCAPS_OFFSCREENPLAIN, 0, + "videomemory offscreen plain surface", FALSE, + .allow_nondxt = TRUE, .allow_all = TRUE, + }, + { + DDSCAPS_SYSTEMMEMORY | DDSCAPS_OFFSCREENPLAIN, 0, + "systemmemory offscreen plain surface", FALSE, + .allow_nondxt = TRUE, + }, }; enum size_type { @@ -6520,18 +6533,29 @@ static void test_block_formats_creation(void) * behavior on windows because I have no hardware that doesn't at * least support np2_conditional. There's probably no HW that * supports DXTN textures but no conditional np2 textures. */ - if (!support && !(types[j].caps & DDSCAPS_SYSTEMMEMORY)) - expect_hr = DDERR_INVALIDPARAMS; - else if (formats[i].create_size_checked && !block_aligned) + + if (formats[i].create_size_checked && !block_aligned) { expect_hr = DDERR_INVALIDPARAMS; - if (!(types[j].caps & DDSCAPS_TEXTURE)) + } + else if (types[j].caps & DDSCAPS_OFFSCREENPLAIN) + { + expect_hr = types[j].caps & DDSCAPS_VIDEOMEMORY ? E_NOTIMPL : DDERR_INVALIDPIXELFORMAT; + if (types[j].caps & DDSCAPS_SYSTEMMEMORY) todo = TRUE; } + else if (!support && !(types[j].caps & DDSCAPS_SYSTEMMEMORY)) + expect_hr = DDERR_INVALIDPARAMS; else expect_hr = D3D_OK; hr = IDirectDraw4_CreateSurface(ddraw, &ddsd, &surface, NULL); + if (hr == DD_OK && types[j].caps & DDSCAPS_OFFSCREENPLAIN) + { + BOOL is_dxt = (formats[i].fourcc & MAKEFOURCC(0xff,0xff,0xff,0)) == MAKEFOURCC('D','X','T',0); + if (types[j].allow_all || (types[j].allow_nondxt && !is_dxt)) + expect_hr = DD_OK; + } todo_wine_if (todo) ok(hr == expect_hr, "Got unexpected hr %#lx for format %s, resource type %s, size %ux%u, expected %#lx.\n", diff --git a/dlls/ddraw/tests/ddraw7.c b/dlls/ddraw/tests/ddraw7.c index a3df2fa659f..7cbc6d9945b 100644 --- a/dlls/ddraw/tests/ddraw7.c +++ b/dlls/ddraw/tests/ddraw7.c @@ -6219,21 +6219,19 @@ static void test_block_formats_creation(void) {MAKEFOURCC('D','X','T','5'), "D3DFMT_DXT5", SUPPORT_DXT5, 4, 4, 16, TRUE, FALSE}, {MAKEFOURCC('Y','U','Y','2'), "D3DFMT_YUY2", SUPPORT_YUY2, 2, 1, 4, FALSE, TRUE }, {MAKEFOURCC('U','Y','V','Y'), "D3DFMT_UYVY", SUPPORT_UYVY, 2, 1, 4, FALSE, TRUE }, + {MAKEFOURCC('I','V','5','0'), "D3DFMT_UYVY", SUPPORT_UYVY, 2, 1, 4, FALSE, TRUE }, + {MAKEFOURCC('N','O','P','E'), "D3DFMT_UYVY", SUPPORT_UYVY, 2, 1, 4, FALSE, TRUE }, }; static const struct { DWORD caps, caps2; const char *name; BOOL overlay; + BOOL allow_nondxt; + BOOL allow_all; } types[] = { - /* DDSCAPS_OFFSCREENPLAIN | DDSCAPS_SYSTEMMEMORY fails to create any fourcc - * surface with DDERR_INVALIDPIXELFORMAT. Don't care about it for now. - * - * Nvidia returns E_FAIL on DXTN DDSCAPS_OFFSCREENPLAIN | DDSCAPS_VIDEOMEMORY. - * Other hw / drivers successfully create those surfaces. Ignore them, this - * suggests that no game uses this, otherwise Nvidia would support it. */ { DDSCAPS_VIDEOMEMORY | DDSCAPS_TEXTURE, 0, "videomemory texture", FALSE @@ -6249,7 +6247,22 @@ static void test_block_formats_creation(void) { DDSCAPS_TEXTURE, DDSCAPS2_TEXTUREMANAGE, "managed texture", FALSE - } + }, + { + DDSCAPS_OFFSCREENPLAIN, 0, + "offscreen plain surface", FALSE, + .allow_nondxt = TRUE, .allow_all = TRUE, + }, + { + DDSCAPS_VIDEOMEMORY | DDSCAPS_OFFSCREENPLAIN, 0, + "videomemory offscreen plain surface", FALSE, + .allow_nondxt = TRUE, .allow_all = TRUE, + }, + { + DDSCAPS_SYSTEMMEMORY | DDSCAPS_OFFSCREENPLAIN, 0, + "systemmemory offscreen plain surface", FALSE, + .allow_nondxt = TRUE, + }, }; enum size_type { @@ -6364,18 +6377,29 @@ static void test_block_formats_creation(void) * behavior on windows because I have no hardware that doesn't at * least support np2_conditional. There's probably no HW that * supports DXTN textures but no conditional np2 textures. */ - if (!support && !(types[j].caps & DDSCAPS_SYSTEMMEMORY)) - expect_hr = DDERR_INVALIDPARAMS; - else if (formats[i].create_size_checked && !block_aligned) + + if (formats[i].create_size_checked && !block_aligned) { expect_hr = DDERR_INVALIDPARAMS; - if (!(types[j].caps & DDSCAPS_TEXTURE)) + } + else if (types[j].caps & DDSCAPS_OFFSCREENPLAIN) + { + expect_hr = types[j].caps & DDSCAPS_VIDEOMEMORY ? E_NOTIMPL : DDERR_INVALIDPIXELFORMAT; + if (types[j].caps & DDSCAPS_SYSTEMMEMORY) todo = TRUE; } + else if (!support && !(types[j].caps & DDSCAPS_SYSTEMMEMORY)) + expect_hr = DDERR_INVALIDPARAMS; else expect_hr = D3D_OK; hr = IDirectDraw7_CreateSurface(ddraw, &ddsd, &surface, NULL); + if (hr == DD_OK && types[j].caps & DDSCAPS_OFFSCREENPLAIN) + { + BOOL is_dxt = (formats[i].fourcc & MAKEFOURCC(0xff,0xff,0xff,0)) == MAKEFOURCC('D','X','T',0); + if (types[j].allow_all || (types[j].allow_nondxt && !is_dxt)) + expect_hr = DD_OK; + } todo_wine_if (todo) ok(hr == expect_hr, "Got unexpected hr %#lx for format %s, resource type %s, size %ux%u, expected %#lx.\n", -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9520
On Fri Nov 21 22:30:44 2025 +0000, Elizabeth Figura wrote:
Yeah, these aren't too complicated so I don't mind the large series. However: * 1/9 could use tests; should be easy enough to plug into test_default_presenter_allocate(). * 2/9... wow, that's surprising; it allows BI_BITFIELDS but not 16- or 24-bit BI_RGB. Okay then. * 3/9 is fine, just want to write that for reference I did check with Helen and the NV11 card does report NV12. * Why do we need 4/9? Shouldn't it fail CreateSurface()? * 5/9 is probably fine, although as someone less familiar with how this area works I'd appreciate an explanation of why the current code is wrong. It's not exactly easy to look up, anyway. * 6/9 should presumably only be applied if the application has called SetAspectRatioMode(VMR_ARMODE_LETTER_BOX). Has it? And if it has, we presumably need to also clear the rest of the surface to black. * 8/9 would probably make more sense if you just fit it into the table, with depth=16 and depth=24 entries. Thumb up for everything I'm not mentioning.
**4/9** Resident Evil 2 Classic (the one from GOG at least) comes with its own ddraw.dll wrapper on top of d3d9 which doesn't reject invalid FourCC formats for `DDSCAPS_OFFSCREENPLAIN` surfaces. It looks like the same is true for ddraw on Windows: nVidia rejects DXT* formats, allowing all the rest. AMD only rejects DXT* formats on `DDSCAPS_OFFSCREENPLAIN | DDSCAPS_SYSTEMMEMORY`, everything else is alright. FWIW with RE2 we end up trying to create an IV50 surface and things go quite wrong once that succeeds. I added a ddraw test for this. **5/9** I think it has to do with current window / client coordinates not necessarily matching with reality with fullscreen d3d and especially ddraw, i.e. resizing the window while in fullscreen mode doesn't necessarily reflect on what appears on the screen. The ddraw clipper is "the ultimate authority" on the geometry that's supposed to actually end up on the screen; as I understand it, in practice the clipper is not affected by e.g. moving the window position while in fullscreen. In the case of RE2, apparently the window rect doesn't exactly match the fullscreen rect, but also both `GetClientRect` and `ClientToScreen` are hooked by the included ddraw.dll, such that their results are mangled e.g. `GetClientRect` returns (-54,0)-(17,35), while the clip rect is the correct (0,0)-(320,240). I'm not sure if this is a good enough justification for the change though. **6/9** It doesn't look like RE2 calls that function, not sure if there are other ways to get there? I don't see `IVMRAspectRatioControl` being queried either. Hacking the vmr7 tests it seems to confirm that the default is to stretch (I guess you already knew this), but the game on Windows has the letterboxing. I'd be curious about the window dimensions on Windows, but I suspect that peeking into that is a step too far. I dropped the patch for the time being. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9520#note_123335
**4/9**
Resident Evil 2 Classic (the one from GOG at least) comes with its own ddraw.dll wrapper on top of d3d9 which doesn't reject invalid FourCC formats for `DDSCAPS_OFFSCREENPLAIN` surfaces. It looks like the same is true for ddraw on Windows: nVidia rejects DXT\* formats, allowing all the rest. AMD only rejects DXT\* formats on `DDSCAPS_OFFSCREENPLAIN | DDSCAPS_SYSTEMMEMORY`, everything else is alright. FWIW with RE2 we end up trying to create an IV50 surface and things go quite wrong once that succeeds. I added a ddraw test for this.
Unfortunately I think this is not the right way to fix this, or at least, not the way native deals with this. If I use native quartz on builtin ddraw, and remove all formats from GetFourCCCodes(), it doesn't change which ones are accepted. YV12, UYVY, YUY2 are still accepted, as is NV12 (which we don't even return from GetFourCCCodes() in the first place). DXT1 is also accepted. IV50 isn't, but presumably would be if wined3d allowed it. What *does* depend on GetFourCCCodes() is apparently ReceiveConnection(). So we should probably be filtering there instead. I'm guessing that'll solve your problem.
**5/9**
I think it has to do with current window / client coordinates not necessarily matching with reality with fullscreen d3d and especially ddraw, i.e. resizing the window while in fullscreen mode doesn't necessarily reflect on what appears on the screen. The ddraw clipper is "the ultimate authority" on the geometry that's supposed to actually end up on the screen; as I understand it, in practice the clipper is not affected by e.g. moving the window position while in fullscreen.
In the case of RE2, apparently the window rect doesn't exactly match the fullscreen rect, but also both `GetClientRect` and `ClientToScreen` are hooked by the included ddraw.dll, such that their results are mangled e.g. `GetClientRect` returns (-54,0)-(17,35), while the clip rect is the correct (0,0)-(320,240). I'm not sure if this is a good enough justification for the change though.
Oh, right, this is the bug where the game hooks a bunch of stuff. So do we expect that this change is a no-op in the absence of hooking? If so, I'm willing to accept it—it may not be correct, but I have no idea how we'd figure out what is correct. We should add a comment though to specify why we're using a more verbose code path.
**6/9**
It doesn't look like RE2 calls that function, not sure if there are other ways to get there? I don't see `IVMRAspectRatioControl` being queried either. Hacking the vmr7 tests it seems to confirm that the default is to stretch (I guess you already knew this), but the game on Windows has the letterboxing. I'd be curious about the window dimensions on Windows, but I suspect that peeking into that is a step too far.
This is more of a problem though. I don't think we want to be changing the default AR mode. Is there a different path that we could be using to get the image onto the screen, one that might also be hooked by the native ddraw? [and that might also sidestep the GetClientRect() issue...] -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9520#note_123340
**5/9**
Oh, right, this is the bug where the game hooks a bunch of stuff. So do we expect that this change is a no-op in the absence of hooking?
Effectively yes. After looking into it some more, I'm not so convinced that we want to do this, though. It looks like we can't depend on `GetClientRect` giving a proper origin (which BTW I think is supposed to always be at (0,0)? At least MSDN is very explicit about it), while `ClientToScreen` just returns `FALSE` for the video child window. Maybe using `presenter->native_size` and / or `info->szAspectRatio` is more proper? I'll probably try something along those lines.
**6/9**
It doesn't look like RE2 calls that function, not sure if there are other ways to get there? I don't see `IVMRAspectRatioControl` being queried either. Hacking the vmr7 tests it seems to confirm that the default is to stretch (I guess you already knew this), but the game on Windows has the letterboxing. I'd be curious about the window dimensions on Windows, but I suspect that peeking into that is a step too far.
This is more of a problem though. I don't think we want to be changing the default AR mode. Is there a different path that we could be using to get the image onto the screen, one that might also be hooked by the native ddraw? \[and that might also sidestep the GetClientRect() issue...\]
I think I figured it out. The game window is 320x240 fullscreen while the video is 320x160. VMR is set up in windowed mode. The game calls `VideoWindow_SetWindowPosition(..., 0, 10, 320, 160)`, which would center the video vertically when accounting for the caption bar that the parent (ddraw, fullscreen) window is supposed to have. But we tweak the window styles when entering fullscreen and the game doesn't expect or check for that, so the child window ends up higher than it should be. I don't think there is much we can do upstream for this one until we figure out some way to stop messing with window styles in wined3d. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9520#note_124405
**4/9**
What _does_ depend on GetFourCCCodes() is apparently ReceiveConnection(). So we should probably be filtering there instead. I'm guessing that'll solve your problem.
Yes it does, thanks for the investigation! -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9520#note_124407
Maybe using `presenter->native_size` and / or `info->szAspectRatio` is more proper?
Actually I'm starting to think that the interesting info is supposed to be stored by the VMR filter during the `VideoWindow_SetWindowPosition()` call and then passed to the presenter via `info->rcDst` in `PresentImage()`. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9520#note_124541
**5/9**
Oh, right, this is the bug where the game hooks a bunch of stuff. So do we expect that this change is a no-op in the absence of hooking?
Effectively yes.
After looking into it some more, I'm not so convinced that we want to do this, though. It looks like we can't depend on `GetClientRect` giving a proper origin (which BTW I think is supposed to always be at (0,0)? At least MSDN is very explicit about it), while `ClientToScreen` just returns `FALSE` for the video child window. Maybe using `presenter->native_size` and / or `info->szAspectRatio` is more proper?
I think those are supposed to be the original size of the video. We want the size that's actually going to be displayed on screen, which may be stretched to match the window we're rendering into.
I'll probably try something along those lines.
Actually I'm starting to think that the interesting info is supposed to be stored by the VMR filter during the `VideoWindow_SetWindowPosition()` call and then passed to the presenter via `info->rcDst` in `PresentImage()`.
Hmm. rcSrc/rcDst look like they're supposed to come from rcSource/rcTarget in VIDEOINFOHEADER, but at least in renderless mode they don't. Maybe in windowed mode they should be used like that. I'm not sure. SetWindowPosition() IIRC takes the window rect, so we would need to account for that. But it's plausible. I suppose you could test this theory by calling SetWindowPosition(), then moving and resizing the window under quartz's nose [you can get the window handle with IOverlay::GetWindowHandle() as we often do in tests], then seeing whether it renders into the window as-is or where the window used to be. Of course, you might also get the same effect by just calling GetWindowRect() then AdjustWindowRect(). Trouble is you still need to get to screen coordinates *somehow*. If you can't use ClientToScreen() you're in a bit of a pickle. In theory you can render to an arbitrarily deep child window [granted, I haven't tested this], but getting on the screen in ddraw requires blitting to the primary using screen coordinates. Unless I'm mistaken about that and there's another way?
**6/9**
It doesn't look like RE2 calls that function, not sure if there are other ways to get there? I don't see `IVMRAspectRatioControl` being queried either. Hacking the vmr7 tests it seems to confirm that the default is to stretch (I guess you already knew this), but the game on Windows has the letterboxing. I'd be curious about the window dimensions on Windows, but I suspect that peeking into that is a step too far.
This is more of a problem though. I don't think we want to be changing the default AR mode. Is there a different path that we could be using to get the image onto the screen, one that might also be hooked by the native ddraw? \[and that might also sidestep the GetClientRect() issue...\]
I think I figured it out. The game window is 320x240 fullscreen while the video is 320x160. VMR is set up in windowed mode. The game calls `VideoWindow_SetWindowPosition(..., 0, 10, 320, 160)`, which would center the video vertically when accounting for the caption bar that the parent (ddraw, fullscreen) window is supposed to have. But we tweak the window styles when entering fullscreen and the game doesn't expect or check for that, so the child window ends up higher than it should be.
I don't think there is much we can do upstream for this one until we figure out some way to stop messing with window styles in wined3d.
Ah! That makes sense. Fortunately I think 9585 will get us closer to that. In any case it probably makes sense to resend this merge request without the presentation bits. The rest is fine (well, after the GetFourCCCodes() usage is fixed) and we are getting a bit close to freeze. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9520#note_124586
I suppose you could test this theory by calling SetWindowPosition(), then moving and resizing the window under quartz's nose \[you can get the window handle with IOverlay::GetWindowHandle() as we often do in tests\], then seeing whether it renders into the window as-is or where the window used to be.
Good idea. Unfortunately it looks like rendering moves with the window. I'll recheck that I'm testing what I think I'm testing, but this seems pretty definitive.
Trouble is you still need to get to screen coordinates _somehow_. If you can't use ClientToScreen() you're in a bit of a pickle. In theory you can render to an arbitrarily deep child window \[granted, I haven't tested this\], but getting on the screen in ddraw requires blitting to the primary using screen coordinates. Unless I'm mistaken about that and there's another way?
That sounds right :/ It does give me the idea that maybe VMR7 is really supposed to use the ddraw clipper for this, perhaps via `IDirectDrawClipper::SetHWnd` on the child window. I don't know the details here, so maybe it's not workable but I'll have a look.
Ah! That makes sense. Fortunately I think 9585 will get us closer to that.
I didn't notice, very glad to see it!
In any case it probably makes sense to resend this merge request without the presentation bits. The rest is fine (well, after the GetFourCCCodes() usage is fixed) and we are getting a bit close to freeze.
Fair enough :thumbsup: -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9520#note_124603
That sounds right :/ It does give me the idea that maybe VMR7 is really supposed to use the ddraw clipper for this, perhaps via `IDirectDrawClipper::SetHWnd` on the child window. I don't know the details here, so maybe it's not workable but I'll have a look.
Okay, no, the game's ddraw wrapper gets in the way of that as well :/ Pushing without the presentation part shortly. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9520#note_124688
participants (4)
-
Elizabeth Figura (＠zfigura) -
Henri Verbeet
-
Matteo Bruni
-
Matteo Bruni (＠Mystral)