July 2022 - Wine-Devel - list.winehq.org

Re: [PATCH 0/5] MR355: win32u: Move more display mode logic out of graphics drivers.

by Zhiyi Zhang (＠zhiyi)

On Fri Jul 1 07:48:31 2022 +0000, Zhiyi Zhang wrote: > Be very careful about dmDriverExtra. You are losing private driver data > here. "memcpy(full_mode, found_mode, sizeof(*found_mode) + > found_mode->dmDriverExtra)" is how full mode is populated. Oh, this full mode is used for storing in the registry. So we don't need private data. But I think it's still best to dmDriverExtra to 0 after copying. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/355#note_3100

July 1, 2022

Re: [PATCH 0/5] MR355: win32u: Move more display mode logic out of graphics drivers.

by Zhiyi Zhang (＠zhiyi)

Zhiyi Zhang (@zhiyi) commented about dlls/winex11.drv/display.c: > return DISP_CHANGE_BADMODE; > } > > - if (!write_registry_settings(devname, full_mode)) > - { > - ERR("Failed to write %s display settings to registry.\n", wine_dbgstr_w(devname)); > - free_full_mode(full_mode); > - free(displays); > - return DISP_CHANGE_NOTUPDATED; > - } > - > + *devmode = *full_mode; Be very careful about dmDriverExtra. You are losing private driver data here. "memcpy(full_mode, found_mode, sizeof(*found_mode) + found_mode->dmDriverExtra)" is how full mode is populated. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/355#note_3099

July 1, 2022

Re: [PATCH 0/5] MR355: win32u: Move more display mode logic out of graphics drivers.

by Zhiyi Zhang (＠zhiyi)

Zhiyi Zhang (@zhiyi) commented about dlls/win32u/sysparams.c: > if (!NtUserEnumDisplaySettings( devname, ENUM_CURRENT_SETTINGS, &current_mode, 0 )) current_mode.dmSize = 0; > if (!(devmode = validate_display_settings( &default_mode, &current_mode, devmode ))) return DISP_CHANGE_BADMODE; > > - ret = user_driver->pChangeDisplaySettingsEx( device_name, devmode, hwnd, flags, lparam ); > - if (ret != DISP_CHANGE_SUCCESSFUL) > - ERR( "Changing %s display settings returned %d.\n", debugstr_us(devname), ret ); > + ret = user_driver->pChangeDisplaySettingsEx( device_name, devmode, hwnd, flags | CDS_TEST, lparam ); > + if (!ret && (flags & CDS_UPDATEREGISTRY) && !write_registry_settings( adapter_path, devmode )) ret = DISP_CHANGE_NOTUPDATED; > + else if (flags & (CDS_TEST | CDS_NORESET)) ret = DISP_CHANGE_SUCCESSFUL; > + else ret = user_driver->pChangeDisplaySettingsEx( device_name, devmode, hwnd, flags, lparam ); I think you can remove CDS_UPDATEREGISTRY here. Otherwise, user_driver->pChangeDisplaySettingsEx() will try to find the full mode again. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/355#note_3098

July 1, 2022

Re: [PATCH 0/5] MR355: win32u: Move more display mode logic out of graphics drivers.

by Zhiyi Zhang (＠zhiyi)

Zhiyi Zhang (@zhiyi) commented about dlls/win32u/sysparams.c: > return ret; > } > > +static BOOL write_registry_settings( const WCHAR *adapter_path, const DEVMODEW *mode ) > +{ > + HANDLE mutex; > + HKEY hkey; > + BOOL ret; > + > + mutex = get_display_device_init_mutex(); > + > + if (!config_key && !(config_key = reg_open_key( NULL, config_keyW, sizeof(config_keyW) ))) return FALSE; mutex is not released here. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/355#note_3097

July 1, 2022

Re: [PATCH 0/5] MR355: win32u: Move more display mode logic out of graphics drivers.

by Zhiyi Zhang (＠zhiyi)

On Fri Jul 1 07:23:12 2022 +0000, Rémi Bernon wrote: > It's copying from the local `mode` variable, which has been copied from > the cached modes within the mutex already. It's a bit redundant but I > used the local variable for ENUM_CURRENT_SETTINGS. > It's pretty much temporary anyway as I intend to move the available mode > enumeration to win32u entirely later and only keep ENUM_CURRENT_SETTINGS > in the drivers. Right, I misread that. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/355#note_3096

July 1, 2022

[PATCH 1/1] d2d1/tests: Add some tests for enum property structure.

by Nikolay Sivov

From: Nikolay Sivov <nsivov(a)codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/d2d1/tests/d2d1.c | 105 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 105 insertions(+) diff --git a/dlls/d2d1/tests/d2d1.c b/dlls/d2d1/tests/d2d1.c index 78f255c1f99..9dfe1e760ba 100644 --- a/dlls/d2d1/tests/d2d1.c +++ b/dlls/d2d1/tests/d2d1.c @@ -11343,6 +11343,72 @@ static void test_effect_properties(BOOL d3d11) release_test_context(&ctx); } +static void check_enum_property(ID2D1Effect *effect, UINT32 property, + const UINT32 *values, UINT32 count) +{ + ID2D1Properties *subproperties, *fields_subproperties, *field_subproperties; + D2D1_PROPERTY_TYPE prop_type; + UINT32 prop_count, value; + WCHAR buffer[64]; + unsigned int i; + HRESULT hr; + + prop_type = ID2D1Effect_GetType(effect, property); + ok(prop_type == D2D1_PROPERTY_TYPE_ENUM, "Unexpected type %d.\n", prop_type); + + hr = ID2D1Effect_GetSubProperties(effect, property, &subproperties); + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + + prop_type = ID2D1Properties_GetType(subproperties, D2D1_SUBPROPERTY_FIELDS); + ok(prop_type == D2D1_PROPERTY_TYPE_ARRAY, "Unexpected type %d.\n", prop_type); + + /* Number of enum members. */ + hr = ID2D1Properties_GetValue(subproperties, D2D1_SUBPROPERTY_FIELDS, D2D1_PROPERTY_TYPE_ARRAY, + (BYTE *)&value, sizeof(value)); + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + ok(value == count, "Unexpected value %u.\n", value); + + prop_count = ID2D1Properties_GetPropertyCount(subproperties); + ok(!prop_count, "Got unexpected property count %u.\n", count); + + /* Fields is an array of strings providing display names for members. */ + hr = ID2D1Properties_GetSubProperties(subproperties, D2D1_SUBPROPERTY_FIELDS, + &fields_subproperties); + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + prop_count = ID2D1Properties_GetPropertyCount(fields_subproperties); + ok(prop_count == count, "Got unexpected property count %u.\n", count); + + for (i = 0; i < prop_count; ++i) + { + winetest_push_context("Test %u", i); + + prop_type = ID2D1Properties_GetType(fields_subproperties, i); + ok(prop_type == D2D1_PROPERTY_TYPE_STRING, "Unexpected type %d.\n", prop_type); + + /* Field names are localized. */ + hr = ID2D1Properties_GetSubProperties(fields_subproperties, i, &field_subproperties); + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + prop_count = ID2D1Properties_GetPropertyCount(field_subproperties); + ok(prop_count == 1, "Got unexpected property count %u.\n", count); + + prop_type = ID2D1Properties_GetType(field_subproperties, 0); + ok(prop_type == D2D1_PROPERTY_TYPE_UINT32, "Unexpected type %d.\n", prop_type); + hr = ID2D1Properties_GetPropertyName(field_subproperties, 0, buffer, ARRAY_SIZE(buffer)); + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + ok(!wcscmp(buffer, L"Index"), "Unexpected name %s.\n", wine_dbgstr_w(buffer)); + hr = ID2D1Properties_GetValue(field_subproperties, 0, D2D1_PROPERTY_TYPE_UINT32, (BYTE *)&value, sizeof(value)); + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + ok(value == values[i], "Unexpected value %u, expected %u.\n", value, values[i]); + + ID2D1Properties_Release(field_subproperties); + + winetest_pop_context(); + } + + ID2D1Properties_Release(fields_subproperties); + ID2D1Properties_Release(subproperties); +} + static void test_effect_2d_affine(BOOL d3d11) { D2D1_MATRIX_3X2_F rotate, scale, skew; @@ -11357,6 +11423,7 @@ static void test_effect_2d_affine(BOOL d3d11) ID2D1Bitmap1 *bitmap; ID2D1Effect *effect; ID2D1Image *output; + UINT32 value; BOOL match; HRESULT hr; @@ -11400,6 +11467,22 @@ static void test_effect_2d_affine(BOOL d3d11) "BgEfAQQBIQECASMChAEA"}, }; + static const unsigned int border_modes[] = + { + D2D1_BORDER_MODE_SOFT, + D2D1_BORDER_MODE_HARD, + }; + + static const unsigned int interp_modes[] = + { + D2D1_2DAFFINETRANSFORM_INTERPOLATION_MODE_NEAREST_NEIGHBOR, + D2D1_2DAFFINETRANSFORM_INTERPOLATION_MODE_LINEAR, + D2D1_2DAFFINETRANSFORM_INTERPOLATION_MODE_CUBIC, + D2D1_2DAFFINETRANSFORM_INTERPOLATION_MODE_MULTI_SAMPLE_LINEAR, + D2D1_2DAFFINETRANSFORM_INTERPOLATION_MODE_ANISOTROPIC, + D2D1_2DAFFINETRANSFORM_INTERPOLATION_MODE_HIGH_QUALITY_CUBIC, + }; + memset(image_16x16, 0xff, sizeof(image_16x16)); set_matrix_identity(&rotate); set_matrix_identity(&scale); @@ -11432,6 +11515,28 @@ static void test_effect_2d_affine(BOOL d3d11) count = ID2D1Effect_GetPropertyCount(effect); todo_wine ok(count == 4, "Got unexpected property count %u.\n", count); + hr = ID2D1Effect_GetValue(effect, D2D1_2DAFFINETRANSFORM_PROP_INTERPOLATION_MODE, + D2D1_PROPERTY_TYPE_ENUM, (BYTE *)&value, sizeof(value)); + todo_wine + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + if (SUCCEEDED(hr)) + { + ok(value == D2D1_2DAFFINETRANSFORM_INTERPOLATION_MODE_LINEAR, "Unexpected value %u.\n", value); + check_enum_property(effect, D2D1_2DAFFINETRANSFORM_PROP_INTERPOLATION_MODE, interp_modes, + ARRAY_SIZE(interp_modes)); + } + + hr = ID2D1Effect_GetValue(effect, D2D1_2DAFFINETRANSFORM_PROP_BORDER_MODE, D2D1_PROPERTY_TYPE_ENUM, + (BYTE *)&value, sizeof(value)); + todo_wine + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + if (SUCCEEDED(hr)) + { + ok(value == D2D1_BORDER_MODE_SOFT, "Unexpected value %u.\n", value); + check_enum_property(effect, D2D1_2DAFFINETRANSFORM_PROP_BORDER_MODE, border_modes, + ARRAY_SIZE(border_modes)); + } + for (i = 0; i < ARRAY_SIZE(effect_2d_affine_tests); ++i) { const struct effect_2d_affine_test *test = &effect_2d_affine_tests[i]; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/357

July 1, 2022

[PATCH 0/1] MR357: d2d1/tests: Add some tests for enum property structure.

by Nikolay Sivov (＠nsivov)

Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> -- https://gitlab.winehq.org/wine/wine/-/merge_requests/357

July 1, 2022

Re: [PATCH 0/5] MR355: win32u: Move more display mode logic out of graphics drivers.

by Rémi Bernon

On Fri Jul 1 07:10:03 2022 +0000, Zhiyi Zhang wrote: > You should move this memcpy() before pthread_mutex_unlock(). It's copying from the local `mode` variable, which has been copied from the cached modes within the mutex already. It's a bit redundant but I used the local variable for ENUM_CURRENT_SETTINGS. It's pretty much temporary anyway as I intend to move the existing mode enumeration to win32u entirely later and only keep ENUM_CURRENT_SETTINGS in the drivers. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/355#note_3094

July 1, 2022

Re: [PATCH 0/5] MR355: win32u: Move more display mode logic out of graphics drivers.

by Zhiyi Zhang (＠zhiyi)

Zhiyi Zhang (@zhiyi) commented about dlls/win32u/sysparams.c: > return FALSE; > } > > - user_driver->pUpdateDisplayDevices( &device_manager, TRUE, &ctx ); > + if (!user_driver->pUpdateDisplayDevices( &device_manager, TRUE, &ctx )) > + { > + static const struct gdi_gpu gpu; > + static const struct gdi_adapter adapter = > + { > + .state_flags = DISPLAY_DEVICE_ATTACHED_TO_DESKTOP | DISPLAY_DEVICE_PRIMARY_DEVICE | DISPLAY_DEVICE_VGA_COMPATIBLE, > + }; > + struct gdi_monitor monitor = > + { > + .state_flags = DISPLAY_DEVICE_ACTIVE | DISPLAY_DEVICE_ATTACHED, > + }; You can initialize rc_monitor and rc_work here as well. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/355#note_3093

July 1, 2022

Re: [PATCH 0/5] MR355: win32u: Move more display mode logic out of graphics drivers.

by Zhiyi Zhang (＠zhiyi)

Zhiyi Zhang (@zhiyi) commented about dlls/winex11.drv/display.c: > return FALSE; > } > > - memcpy(devmode, (BYTE *)cached_modes + (sizeof(*cached_modes) + cached_modes[0].dmDriverExtra) * n, sizeof(*devmode)); > + mode = *(DEVMODEW *)((BYTE *)cached_modes + (sizeof(*cached_modes) + cached_modes[0].dmDriverExtra) * n); > pthread_mutex_unlock( &settings_mutex ); > > -done: > - /* Set generic fields */ > - devmode->dmSize = FIELD_OFFSET(DEVMODEW, dmICMMethod); > - devmode->dmDriverExtra = 0; > - devmode->dmSpecVersion = DM_SPECVERSION; > - devmode->dmDriverVersion = DM_SPECVERSION; > - lstrcpyW(devmode->dmDeviceName, dev_name); > + memcpy( &devmode->dmFields, &mode.dmFields, devmode->dmSize - offsetof(DEVMODEW, dmFields) ); > return TRUE; You should move this memcpy() before pthread_mutex_unlock(). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/355#note_3092

July 1, 2022