[PATCH v5 0/2] MR7324: Use Microsoft provided coefficients for YUV to RGB conversion.
The current YUV to RGB conversion provides values in studio-range [16-235], but full-range is required [0-255]. As a result, we currently write the value of 16 to represent black, and so we get gray instead. This MR changes the coefficients used in the conversion so that the resultant output is in full-range. It actually uses two sets of coefficients as these differ between SD and HD. Also included is a number of Direct Draw 7 tests. The coefficients used are documented here: https://learn.microsoft.com/en-us/windows/win32/medfound/recommended-8-bit-y... -- v5: wined3d: Use Microsoft provided coefficients for YUV to RGB conversion. ddraw/tests: Test yuv to rgb blt in ddraw7. https://gitlab.winehq.org/wine/wine/-/merge_requests/7324
From: Brendan McGrath <bmcgrath(a)codeweavers.com> --- dlls/ddraw/tests/ddraw7.c | 436 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 436 insertions(+) diff --git a/dlls/ddraw/tests/ddraw7.c b/dlls/ddraw/tests/ddraw7.c index e7d66278ad7..fb6311a7f6f 100644 --- a/dlls/ddraw/tests/ddraw7.c +++ b/dlls/ddraw/tests/ddraw7.c @@ -13652,6 +13652,441 @@ static void test_blt(void) DestroyWindow(window); } +static void test_yuv_to_rgb_blt(void) +{ + IDirectDrawSurface7 *rgb_surface, *yuv_surface; + DWORD flags, chroma_pitch, chroma_height; + DDSURFACEDESC2 surface_desc, lock_desc; + unsigned int i, j, x, y; + IDirectDraw7 *ddraw; + ULONG refcount; + UINT32 *pixel; + HWND window; + HRESULT hr; + BYTE *ptr; + + static const struct + { + const char *name; + DWORD fourCC; + BYTE h_subsampling; + BYTE v_subsampling; + BYTE u_plane; + BYTE v_plane; + BYTE y_offset; + BYTE u_offset; + BYTE v_offset; + BYTE y_increment; + BYTE u_increment; + BYTE v_increment; + } + test_textures[] = + { + {"UYVY", MAKEFOURCC('U', 'Y', 'V', 'Y'), 2, 1, 0, 0, 1, 0, 2, 2, 4, 4}, + {"YUY2", MAKEFOURCC('Y', 'U', 'Y', '2'), 2, 1, 0, 0, 0, 1, 3, 2, 4, 4}, + {"NV12", MAKEFOURCC('N', 'V', '1', '2'), 2, 2, 1, 1, 0, 0, 1, 1, 2, 2}, + {"YV12", MAKEFOURCC('Y', 'V', '1', '2'), 2, 2, 2, 1, 0, 0, 0, 1, 1, 1}, + }; + + static const struct + { + const char *name; + DWORD height; + BYTE y; + BYTE u; + BYTE v; + UINT32 expected; + BOOL todo; + /* some systems are configured for studio range */ + UINT32 studio_value; + /* some systems use SD values for HD. */ + BOOL test_for_broken; + UINT32 broken_value; + } + test_colors[] = + { + { + .name = "black SD", + .height = 576, + .y = 0x10, + .u = 0x80, + .v = 0x80, + .expected = 0x000000, + .todo = TRUE, + .studio_value = 0x101010, + }, + + { + .name = "white SD", + .height = 576, + .y = 0xeb, + .u = 0x80, + .v = 0x80, + .expected = 0xffffff, + .todo = TRUE, + .studio_value = 0xebebeb, + }, + + { + .name = "red SD", + .height = 576, + .y = 0x51, + .u = 0x5a, + .v = 0xf0, + .expected = 0xfe0000, + .todo = TRUE, + .studio_value = 0xeb1010, + }, + + { + .name = "green SD", + .height = 576, + .y = 0x91, + .u = 0x36, + .v = 0x22, + .expected = 0x00ff01, + .todo = TRUE, + .studio_value = 0x10eb11, + }, + + { + .name = "blue SD", + .height = 576, + .y = 0x29, + .u = 0xf0, + .v = 0x6e, + .expected = 0x0000ff, + .todo = TRUE, + .studio_value = 0x1010eb, + }, + + { + .name = "gray SD", + .height = 576, + .y = 0x7e, + .u = 0x80, + .v = 0x80, + .expected = 0x808080, + .todo = TRUE, + .studio_value = 0x7e7e7e, + }, + + { + .name = "past black SD", + .height = 576, + .y = 0x00, + .u = 0x80, + .v = 0x80, + .expected = 0x000000, + .todo = FALSE, + .studio_value = 0x101010, + }, + + { + .name = "past white SD", + .height = 576, + .y = 0xff, + .u = 0x80, + .v = 0x80, + .expected = 0xffffff, + .todo = FALSE, + .studio_value = 0xebebeb, + }, + + { + .name = "zeros SD", + .height = 576, + .y = 0x00, + .u = 0x00, + .v = 0x00, + .expected = 0x008700, + .todo = FALSE, + .studio_value = 0x108410, + }, + /* Windows will treat anything with a height greater than 576 as HD and, + * as per ITU-R Recommendation BT.709 + . = change the color scheme */ + { + .name = "black HD", + .height = 580, + .y = 0x10, + .u = 0x80, + .v = 0x80, + .expected = 0x000000, + .todo = TRUE, + .studio_value = 0x101010, + }, + + { + .name = "white HD", + .height = 580, + .y = 0xeb, + .u = 0x80, + .v = 0x80, + .expected = 0xffffff, + .todo = TRUE, + .studio_value = 0xebebeb, + }, + + { + .name = "red HD", + .height = 580, + .y = 0x3f, + .u = 0x66, + .v = 0xf0, + .expected = 0xff0100, + .todo = TRUE, + .studio_value = 0xeb1110, + .test_for_broken = TRUE, + .broken_value = 0xe90002, + }, + + { + .name = "green HD", + .height = 580, + .y = 0xad, + .u = 0x2a, + .v = 0x1a, + .expected = 0x00ff01, + .todo = TRUE, + .studio_value = 0x10eb11, + .test_for_broken = TRUE, + .broken_value = 0x14ff09, + }, + + { + .name = "blue HD", + .height = 580, + .y = 0x20, + .u = 0xf0, + .v = 0x76, + .expected = 0x0100ff, + .todo = TRUE, + .studio_value = 0x1110eb, + .test_for_broken = TRUE, + .broken_value = 0x0300f5, + }, + + { + .name = "gray HD", + .height = 580, + .y = 0x7e, + .u = 0x80, + .v = 0x80, + .expected = 0x808080, + .todo = TRUE, + .studio_value = 0x7e7e7e, + }, + + { + .name = "past black HD", + .height = 580, + .y = 0x00, + .u = 0x80, + .v = 0x80, + .expected = 0x000000, + .todo = FALSE, + .studio_value = 0x101010, + }, + + { + .name = "past white HD", + .height = 580, + .y = 0xff, + .u = 0x80, + .v = 0x80, + .expected = 0xffffff, + .todo = FALSE, + .studio_value = 0xebebeb, + }, + + { + .name = "zeros HD", + .height = 580, + .y = 0x00, + .u = 0x00, + .v = 0x00, + .expected = 0x004d00, + .todo = TRUE, + .studio_value = 0x105210, + .test_for_broken = TRUE, + .broken_value = 0x008700, + }, + }; + + window = create_window(); + if (!(ddraw = create_ddraw())) + { + skip("Failed to create a 3D device, skipping test.\n"); + DestroyWindow(window); + return; + } + + hr = IDirectDraw7_SetCooperativeLevel(ddraw, window, DDSCL_NORMAL); + ok(hr == S_OK, "Failed to set cooperative level, hr %#lx.\n", hr); + + memset(&surface_desc, 0, sizeof(surface_desc)); + surface_desc.dwSize = sizeof(surface_desc); + surface_desc.dwFlags = DDSD_WIDTH | DDSD_HEIGHT | DDSD_CAPS | DDSD_PIXELFORMAT; + surface_desc.dwWidth = 720; + surface_desc.ddsCaps.dwCaps = DDSCAPS_OFFSCREENPLAIN; + surface_desc.ddpfPixelFormat.dwSize = sizeof(surface_desc.ddpfPixelFormat); + surface_desc.ddpfPixelFormat.dwRGBBitCount = 32; + surface_desc.ddpfPixelFormat.dwRBitMask = 0xff0000; + surface_desc.ddpfPixelFormat.dwGBitMask = 0xff00; + surface_desc.ddpfPixelFormat.dwBBitMask = 0xff; + surface_desc.ddpfPixelFormat.dwRGBAlphaBitMask = 0; + + flags = DDSD_HEIGHT | DDSD_PITCH | DDSD_PIXELFORMAT | DDSD_WIDTH; + + memset(&lock_desc, 0, sizeof(lock_desc)); + lock_desc.dwSize = sizeof(lock_desc); + + yuv_surface = NULL; + + for (i = 0; i < ARRAY_SIZE(test_textures); ++i) + { + winetest_push_context("%s format", test_textures[i].name); + + surface_desc.ddpfPixelFormat.dwFourCC = test_textures[i].fourCC; + + for (j = 0; j < ARRAY_SIZE(test_colors); j++) + { + winetest_push_context("color %s", test_colors[j].name); + + if (!yuv_surface) + { + surface_desc.dwHeight = test_colors[j].height; + surface_desc.ddpfPixelFormat.dwFlags = DDPF_FOURCC; + hr = IDirectDraw7_CreateSurface(ddraw, &surface_desc, &yuv_surface, NULL); + if (hr != S_OK) + { + skip("Failed to create yuv surface, hr %#lx.\n", hr); + winetest_pop_context(); + break; + } + + surface_desc.ddpfPixelFormat.dwFlags = DDPF_RGB; + hr = IDirectDraw7_CreateSurface(ddraw, &surface_desc, &rgb_surface, NULL); + ok(hr == S_OK, "Failed to create rgb surface, hr %#lx.\n", hr); + } + + hr = IDirectDrawSurface7_Lock(yuv_surface, NULL, &lock_desc, DDLOCK_WRITEONLY | DDLOCK_WAIT | DDLOCK_SURFACEMEMORYPTR, NULL); + ok(hr == S_OK, "Got unexpected hr %#lx, expected S_OK.\n", hr); + ok((lock_desc.dwFlags & flags) == flags, "Got unexpected flags %#lx, expected %#lx.\n", + lock_desc.dwFlags, flags); + ok(lock_desc.ddpfPixelFormat.dwFlags == DDPF_FOURCC, + "Got unexpected pixel format flags %#lx, expected DDPF_FOURCC.\n", + lock_desc.ddpfPixelFormat.dwFlags); + ok(lock_desc.ddpfPixelFormat.dwFourCC == test_textures[i].fourCC, + "Got unexpected pixel format %#lx, expected %#lx.\n", + lock_desc.ddpfPixelFormat.dwFourCC, test_textures[i].fourCC); + + ptr = (BYTE*)lock_desc.lpSurface + test_textures[i].y_offset; + + for (y = 0; y < lock_desc.dwHeight; y++) + { + for (x = 0; x < lock_desc.lPitch / test_textures[i].y_increment; x++) + { + *ptr = test_colors[j].y; + ptr += test_textures[i].y_increment; + } + } + + chroma_pitch = lock_desc.lPitch / (test_textures[i].h_subsampling * test_textures[i].y_increment); + chroma_height = lock_desc.dwHeight / test_textures[i].v_subsampling; + + ptr = (BYTE*)lock_desc.lpSurface + test_textures[i].u_offset; + + if (test_textures[i].u_plane) + { + ptr += lock_desc.lPitch * lock_desc.dwHeight; + ptr += chroma_pitch * chroma_height * (test_textures[i].u_plane - 1); + } + + for (y = 0; y < chroma_height; y++) + { + for (x = 0; x < chroma_pitch; x++) + { + *ptr = test_colors[j].u; + ptr += test_textures[i].u_increment; + } + } + + ptr = (BYTE*)lock_desc.lpSurface + test_textures[i].v_offset; + if (test_textures[i].v_plane) + { + ptr += lock_desc.lPitch * lock_desc.dwHeight; + ptr += chroma_pitch * chroma_height * (test_textures[i].v_plane - 1); + } + + for (y = 0; y < chroma_height; y++) + { + for (x = 0; x < chroma_pitch; x++) + { + *ptr = test_colors[j].v; + ptr += test_textures[i].v_increment; + } + } + + hr = IDirectDrawSurface7_Unlock(yuv_surface, NULL); + ok(hr == S_OK, "Got unexpected hr %#lx, expected S_OK.\n", hr); + + hr = IDirectDrawSurface7_Blt(rgb_surface, NULL, yuv_surface, NULL, DDBLT_WAIT, NULL); + ok(hr == S_OK || broken(TRUE), "Got unexpected hr %#lx, expected S_OK.\n", hr); + if (hr != S_OK) + { + skip("Failed to blt to RGB surface, skipping the rest of the tests for this texture."); + IDirectDrawSurface7_Release(yuv_surface); + IDirectDrawSurface7_Release(rgb_surface); + yuv_surface = NULL; + winetest_pop_context(); + break; + } + + hr = IDirectDrawSurface7_Lock(rgb_surface, NULL, &lock_desc, DDLOCK_READONLY | DDLOCK_WAIT | DDLOCK_SURFACEMEMORYPTR, NULL); + ok(hr == S_OK, "Got unexpected hr %#lx, expected S_OK.\n", hr); + ok((lock_desc.dwFlags & flags) == flags, "Got unexpected flags %#lx, expected %#lx.\n", lock_desc.dwFlags, flags); + ok(lock_desc.ddpfPixelFormat.dwFlags == DDPF_RGB, "Got unexpected pixel format flags %#lx, expected DDPF_RGB.\n", + lock_desc.ddpfPixelFormat.dwFlags); + + /* check the top left 4x4 pixels */ + for (y = 0; y < 4; y++) + { + pixel = (UINT32*)((BYTE*)lock_desc.lpSurface + lock_desc.lPitch * y); + for (x = 0; x < 4; x++) + { + UINT32 color = pixel[x] & 0xffffff; + todo_wine_if(test_colors[j].todo) + ok(compare_color(color, test_colors[j].expected, 1) || + broken(compare_color(color, test_colors[j].expected, 4)) || + broken(compare_color(color, test_colors[j].studio_value, 0x10)) || + broken(test_colors[j].test_for_broken && compare_color(color, test_colors[j].broken_value, 4)), + "Got unexpected value at (%d,%d). Got %#lx, expected %#lx.\n", + x, y, (long unsigned)color, (long unsigned)test_colors[j].expected); + } + } + + hr = IDirectDrawSurface7_Unlock(rgb_surface, NULL); + ok(hr == S_OK, "Got unexpected hr %#lx, expected S_OK.\n", hr); + + if (j + 1 == ARRAY_SIZE(test_colors) || test_colors[j].height != test_colors[j+1].height) + { + IDirectDrawSurface7_Release(yuv_surface); + IDirectDrawSurface7_Release(rgb_surface); + yuv_surface = NULL; + } + winetest_pop_context(); + } + + winetest_pop_context(); + } + + refcount = IDirectDraw7_Release(ddraw); + ok(!refcount, "DirectDraw7 has %lu references left.\n", refcount); + DestroyWindow(window); +} + static void test_blt_z_alpha(void) { DWORD blt_flags[] = @@ -20609,6 +21044,7 @@ START_TEST(ddraw7) test_offscreen_overlay(); test_overlay_rect(); test_blt(); + test_yuv_to_rgb_blt(); test_blt_z_alpha(); test_cross_device_blt(); test_color_clamping(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7324
From: Brendan McGrath <bmcgrath(a)codeweavers.com> --- dlls/ddraw/tests/ddraw7.c | 20 -------------------- dlls/wined3d/glsl_shader.c | 23 +++++++++++++++++------ 2 files changed, 17 insertions(+), 26 deletions(-) diff --git a/dlls/ddraw/tests/ddraw7.c b/dlls/ddraw/tests/ddraw7.c index fb6311a7f6f..bc2e1f24024 100644 --- a/dlls/ddraw/tests/ddraw7.c +++ b/dlls/ddraw/tests/ddraw7.c @@ -13696,7 +13696,6 @@ static void test_yuv_to_rgb_blt(void) BYTE u; BYTE v; UINT32 expected; - BOOL todo; /* some systems are configured for studio range */ UINT32 studio_value; /* some systems use SD values for HD. */ @@ -13712,7 +13711,6 @@ static void test_yuv_to_rgb_blt(void) .u = 0x80, .v = 0x80, .expected = 0x000000, - .todo = TRUE, .studio_value = 0x101010, }, @@ -13723,7 +13721,6 @@ static void test_yuv_to_rgb_blt(void) .u = 0x80, .v = 0x80, .expected = 0xffffff, - .todo = TRUE, .studio_value = 0xebebeb, }, @@ -13734,7 +13731,6 @@ static void test_yuv_to_rgb_blt(void) .u = 0x5a, .v = 0xf0, .expected = 0xfe0000, - .todo = TRUE, .studio_value = 0xeb1010, }, @@ -13745,7 +13741,6 @@ static void test_yuv_to_rgb_blt(void) .u = 0x36, .v = 0x22, .expected = 0x00ff01, - .todo = TRUE, .studio_value = 0x10eb11, }, @@ -13756,7 +13751,6 @@ static void test_yuv_to_rgb_blt(void) .u = 0xf0, .v = 0x6e, .expected = 0x0000ff, - .todo = TRUE, .studio_value = 0x1010eb, }, @@ -13767,7 +13761,6 @@ static void test_yuv_to_rgb_blt(void) .u = 0x80, .v = 0x80, .expected = 0x808080, - .todo = TRUE, .studio_value = 0x7e7e7e, }, @@ -13778,7 +13771,6 @@ static void test_yuv_to_rgb_blt(void) .u = 0x80, .v = 0x80, .expected = 0x000000, - .todo = FALSE, .studio_value = 0x101010, }, @@ -13789,7 +13781,6 @@ static void test_yuv_to_rgb_blt(void) .u = 0x80, .v = 0x80, .expected = 0xffffff, - .todo = FALSE, .studio_value = 0xebebeb, }, @@ -13800,7 +13791,6 @@ static void test_yuv_to_rgb_blt(void) .u = 0x00, .v = 0x00, .expected = 0x008700, - .todo = FALSE, .studio_value = 0x108410, }, /* Windows will treat anything with a height greater than 576 as HD and, @@ -13813,7 +13803,6 @@ static void test_yuv_to_rgb_blt(void) .u = 0x80, .v = 0x80, .expected = 0x000000, - .todo = TRUE, .studio_value = 0x101010, }, @@ -13824,7 +13813,6 @@ static void test_yuv_to_rgb_blt(void) .u = 0x80, .v = 0x80, .expected = 0xffffff, - .todo = TRUE, .studio_value = 0xebebeb, }, @@ -13835,7 +13823,6 @@ static void test_yuv_to_rgb_blt(void) .u = 0x66, .v = 0xf0, .expected = 0xff0100, - .todo = TRUE, .studio_value = 0xeb1110, .test_for_broken = TRUE, .broken_value = 0xe90002, @@ -13848,7 +13835,6 @@ static void test_yuv_to_rgb_blt(void) .u = 0x2a, .v = 0x1a, .expected = 0x00ff01, - .todo = TRUE, .studio_value = 0x10eb11, .test_for_broken = TRUE, .broken_value = 0x14ff09, @@ -13861,7 +13847,6 @@ static void test_yuv_to_rgb_blt(void) .u = 0xf0, .v = 0x76, .expected = 0x0100ff, - .todo = TRUE, .studio_value = 0x1110eb, .test_for_broken = TRUE, .broken_value = 0x0300f5, @@ -13874,7 +13859,6 @@ static void test_yuv_to_rgb_blt(void) .u = 0x80, .v = 0x80, .expected = 0x808080, - .todo = TRUE, .studio_value = 0x7e7e7e, }, @@ -13885,7 +13869,6 @@ static void test_yuv_to_rgb_blt(void) .u = 0x80, .v = 0x80, .expected = 0x000000, - .todo = FALSE, .studio_value = 0x101010, }, @@ -13896,7 +13879,6 @@ static void test_yuv_to_rgb_blt(void) .u = 0x80, .v = 0x80, .expected = 0xffffff, - .todo = FALSE, .studio_value = 0xebebeb, }, @@ -13907,7 +13889,6 @@ static void test_yuv_to_rgb_blt(void) .u = 0x00, .v = 0x00, .expected = 0x004d00, - .todo = TRUE, .studio_value = 0x105210, .test_for_broken = TRUE, .broken_value = 0x008700, @@ -14057,7 +14038,6 @@ static void test_yuv_to_rgb_blt(void) for (x = 0; x < 4; x++) { UINT32 color = pixel[x] & 0xffffff; - todo_wine_if(test_colors[j].todo) ok(compare_color(color, test_colors[j].expected, 1) || broken(compare_color(color, test_colors[j].expected, 4)) || broken(compare_color(color, test_colors[j].studio_value, 0x10)) || diff --git a/dlls/wined3d/glsl_shader.c b/dlls/wined3d/glsl_shader.c index df60415f062..7a1f351b2db 100644 --- a/dlls/wined3d/glsl_shader.c +++ b/dlls/wined3d/glsl_shader.c @@ -12677,10 +12677,12 @@ static void glsl_blitter_generate_yuv_shader(struct wined3d_string_buffer *buffe { enum complex_fixup complex_fixup = get_complex_fixup(args->fixup); - shader_addline(buffer, "const vec4 yuv_coef = vec4(1.403, -0.344, -0.714, 1.770);\n"); + shader_addline(buffer, "const vec4 yuv_coef_sd = vec4(1.596, -0.392, -0.813, 2.017);\n"); + shader_addline(buffer, "const vec4 yuv_coef_hd = vec4(1.793, -0.213, -0.533, 2.112);\n"); shader_addline(buffer, "float luminance;\n"); shader_addline(buffer, "vec2 texcoord;\n"); shader_addline(buffer, "vec2 chroma;\n"); + shader_addline(buffer, "vec4 yuv_coef;\n"); shader_addline(buffer, "uniform vec2 size;\n"); shader_addline(buffer, "\nvoid main()\n{\n"); @@ -12715,11 +12717,20 @@ static void glsl_blitter_generate_yuv_shader(struct wined3d_string_buffer *buffe return; } - /* Calculate the final result. Formula is taken from - * http://www.fourcc.org/fccyvrgb.php. Note that the chroma - * ranges from -0.5 to 0.5. */ - shader_addline(buffer, "\n chroma.xy -= 0.5;\n"); - + /* Calculate the final result. Formula is taken from: + * https://learn.microsoft.com/en-us/windows/win32/medfound/recommended-8-bit-y.... + * SD and HD textures use different coefficients. SD is anything with both width and height smaller or equal to 720x576. + * Note that input values are clamped for SD, but not HD. Luminance is clamped to [16,235] ([0.063, 0.922] when normalized). + * Chroma is clamped to [16,240] ([0.063, 0.941] when normalized). */ + shader_addline(buffer, "\n if (size.x <= 720 && size.y <= 576)\n {\n"); + shader_addline(buffer, " luminance = clamp(luminance, 0.063, 0.922);\n"); + shader_addline(buffer, " chroma.xy = clamp(chroma.xy, 0.063, 0.941);\n"); + shader_addline(buffer, " yuv_coef = yuv_coef_sd;\n"); + shader_addline(buffer, " }\n else\n {\n"); + shader_addline(buffer, " yuv_coef = yuv_coef_hd;\n"); + shader_addline(buffer, " }\n"); + shader_addline(buffer, " luminance = (luminance - 0.063) * 1.164;\n"); + shader_addline(buffer, " chroma.xy = chroma.xy - 0.5;\n"); shader_addline(buffer, " %s.x = luminance + chroma.x * yuv_coef.x;\n", output); shader_addline(buffer, " %s.y = luminance + chroma.y * yuv_coef.y + chroma.x * yuv_coef.z;\n", output); shader_addline(buffer, " %s.z = luminance + chroma.y * yuv_coef.w;\n", output); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7324
assuming that this test is indeed useful in the first place, which it indeed may not be.
I've taken some advice from @huw on this one. We decided the tests may be worth keeping to validate the Wine implementation, but I've added a new column to record the `studio_value` and I use this value to mark a test broken; even though technically it's not broken, it's just the graphics card has been set-up differently.
I had to raise the tolerance a bit (4 seems to work)
I've increased the tolerance for Windows to allow 4. In that the test will pass on Wine and Windows if the tolerance is within 1, but it will be marked broken on Windows if between 1 and 4 (and fail on Wine). The tests are a bit different to usual for this one, I guess because we're not testing against core Windows behavior, but against the settings and implementation of the graphics card driver. But the tests now ~~pass~~ don't fail on the different test bot systems.
It also succeeds YV12 surface creation but fails the subsequent Blt(), so we'll probably need to handle that
I now handle this scenario by marking the test as broken and skipping the rest of the tests for that texture.
Some tests actually pass with the current implementation, so I do need a `todo_wine_if`.
Oops, I misread, sorry.
I decided the tests were a bit difficult to read, so I've reformatted and used the member names of the struct during initialization. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7324#note_95278
assuming that this test is indeed useful in the first place, which it indeed may not be.
I've taken some advice from @huw on this one. We decided the tests may be worth keeping to validate the Wine implementation, but I've added a new column to record the `studio_value` and I use this value to mark a test broken; even though technically it's not broken, it's just the graphics card has been set-up differently.
I don't think that it should be marked broken, no, not when the actual behaviour of d3d is not documented anywhere. (The cited Microsoft documentation is for Media Foundation, which is orthogonal to Direct3D). I especially don't think it makes sense to mark behaviour broken which is universal across drivers, i.e. the lack of a difference between "SD" and "HD" values based on image size. If WARP has unique results, that's something we generally consider broken. If results contradict documented behaviour or are clearly the result of a bug, that's also broken. Otherwise, since there isn't really a Direct3D spec, we treat any reasonable behaviour as valid. I'm not opposed to modifying Wine anyway, although the only particularly salient reason I would see to do so is "it makes an application look better". The behaviours of other media decoding libraries aren't particularly relevant here. I am curious if the application in question displays the same colour range on an NVidia Windows machine in its default configuration. I was going to test this myself, but hadn't had a chance to do so yet.
I had to raise the tolerance a bit (4 seems to work)
I've increased the tolerance for Windows to allow 4. In that the test will pass on Wine and Windows if the tolerance is within 1, but it will be marked broken on Windows if between 1 and 4 (and fail on Wine). The tests are a bit different to usual for this one, I guess because we're not testing against core Windows behavior, but against the settings and implementation of the graphics card driver. But the tests now ~~pass~~ don't fail on the different test bot systems.
I'd just leave it as 4 universally; there's no reason to hold Wine to a stricter standard than Windows.
Some tests actually pass with the current implementation, so I do need a `todo_wine_if`.
Oops, I misread, sorry.
I decided the tests were a bit difficult to read, so I've reformatted and used the member names of the struct during initialization.
Ehh... I don't want to bikeshed, but the way in which I misread is not really helped by the reformatting, and while I'm fine with accepting the new version, I think the inline table was better. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7324#note_95285
The cited Microsoft documentation is for Media Foundation, which is orthogonal to Direct3D
That's a good point. I got the impression early that they were analogous as when I was testing a YUV Blt to an RGB surface using ddraw7 (and later d3d9) I was getting results exactly as described in that document (including the HD results). It was only when testing on other machines that that impression began to dissolve. It seems all Media Foundation components (like decoder MFTs and the VideoProcessorMFT) all produce full-range, but graphics card related functions (like blting YUV to RGB or using the ID3D11VideoProcessor) can produce full-range or studio-range depending on configuration.
I am curious if the application in question displays the same colour range on an NVidia Windows machine in its default configuration. I was going to test this myself, but hadn't had a chance to do so yet
The game itself uses `IGraphBuilder::RenderFile` calling `IVideoWindow::put_Owner` with a window that has ddraw attached using `DDSCL_FULLSCREEN`; and under that circumstance, it gets RGB32 directly from the decoder. And the decoders always seem to produce full range. It was only in testing I could reproduce this issue on Windows, as when I don't set `DDSCL_FULLSCREEN`, NV12 is fetched from the decoder, and if my card is configured for studio-range, then it uses 16 to represent black. But that's another solution for this game; to request RGB32 directly from the decoder. But we'll still have an issue with applications that don't go full-screen. We may also run in to an issue if a decoder doesn't directly output RGB32, although I couldn't find an example of one (under quartz anyway, the H.264 decoding MFT doesn't). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7324#note_95290
Maybe detect what color range are the host os using and then choose the right conversion coefficients is a better choice? But then we have to make sure it would work correctly when there are multi monitors with different color ranges. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7324#note_95647
On Sat Feb 22 23:23:22 2025 +0000, Shengyu Qu wrote:
Maybe detect what color range are the host os using and then choose the right conversion coefficients is a better choice? But then we have to make sure it would work correctly when there are multi monitors with different color ranges. Fortunately I don't think this is an application problem. My understanding is the application can write full-range and the OS/GPU driver will adjust the color range before sending to the monitor.
As you can see, the 'Color Range' is just one setting of many. And many of them might be specific to the GPU driver. It'd be a nightmare if an application had to adjust for them all:  -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7324#note_95670
participants (4)
-
Brendan McGrath -
Brendan McGrath (＠redmcg)
-
Elizabeth Figura (＠zfigura)
-
Shengyu Qu (＠Stat_headcrabbed)