Secondhand Lands calls d3d9_device_GetDeviceCaps() frequently. Introduce a wined3d_output_get_ordinal() to get output ordinal only instead of using the more expensive wined3d_output_get_desc().
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=50096 Signed-off-by: Zhiyi Zhang zzhang@codeweavers.com --- dlls/d3d9/device.c | 16 ++++------------ dlls/wined3d/directx.c | 8 +++++++- dlls/wined3d/wined3d.spec | 1 + include/wine/wined3d.h | 2 +- 4 files changed, 13 insertions(+), 14 deletions(-)
diff --git a/dlls/d3d9/device.c b/dlls/d3d9/device.c index fec631e0e67..44a4ecdebc1 100644 --- a/dlls/d3d9/device.c +++ b/dlls/d3d9/device.c @@ -378,10 +378,8 @@ void d3d9_caps_from_wined3dcaps(const struct d3d9 *d3d9, unsigned int adapter_or D3DPTFILTERCAPS_MAGFLINEAR |D3DPTFILTERCAPS_MAGFANISOTROPIC|D3DPTFILTERCAPS_MAGFPYRAMIDALQUAD| D3DPTFILTERCAPS_MAGFGAUSSIANQUAD; struct wined3d_adapter *wined3d_adapter; - struct wined3d_output_desc output_desc; + unsigned int output_idx, output_ordinal; struct wined3d_output *wined3d_output; - unsigned int output_idx; - HRESULT hr;
caps->DeviceType = (D3DDEVTYPE)wined3d_caps->DeviceType; caps->AdapterOrdinal = adapter_ordinal; @@ -535,17 +533,11 @@ void d3d9_caps_from_wined3dcaps(const struct d3d9 *d3d9, unsigned int adapter_or wined3d_output = d3d9->wined3d_outputs[output_idx];
wined3d_mutex_lock(); - hr = wined3d_output_get_desc(wined3d_output, &output_desc); + output_ordinal = wined3d_output_get_ordinal(wined3d_output); wined3d_mutex_unlock();
- if (FAILED(hr)) - { - ERR("Failed to get output desc, hr %#x.\n", hr); - return; - } - - caps->MasterAdapterOrdinal = output_idx - output_desc.ordinal; - caps->AdapterOrdinalInGroup = output_desc.ordinal; + caps->MasterAdapterOrdinal = output_idx - output_ordinal; + caps->AdapterOrdinalInGroup = output_ordinal; if (!caps->AdapterOrdinalInGroup) { wined3d_adapter = wined3d_output_get_adapter(wined3d_output); diff --git a/dlls/wined3d/directx.c b/dlls/wined3d/directx.c index 50b1376a75b..439720032f9 100644 --- a/dlls/wined3d/directx.c +++ b/dlls/wined3d/directx.c @@ -1007,12 +1007,18 @@ HRESULT CDECL wined3d_output_get_desc(const struct wined3d_output *output, TRACE("output %p, desc %p.\n", output, desc);
memset(desc, 0, sizeof(*desc)); - desc->ordinal = output->ordinal; lstrcpyW(desc->device_name, output->device_name); EnumDisplayMonitors(NULL, NULL, enum_monitor_proc, (LPARAM)desc); return wined3d_output_get_display_mode(output, &mode, &desc->rotation); }
+unsigned int CDECL wined3d_output_get_ordinal(const struct wined3d_output *output) +{ + TRACE("output %p.\n", output); + + return output->ordinal; +} + /* FIXME: GetAdapterModeCount and EnumAdapterModes currently only returns modes of the same bpp but different resolutions */
diff --git a/dlls/wined3d/wined3d.spec b/dlls/wined3d/wined3d.spec index 44008119a8d..e8d74e7ad84 100644 --- a/dlls/wined3d/wined3d.spec +++ b/dlls/wined3d/wined3d.spec @@ -170,6 +170,7 @@ @ cdecl wined3d_output_get_display_mode(ptr ptr ptr) @ cdecl wined3d_output_get_mode(ptr long long long ptr) @ cdecl wined3d_output_get_mode_count(ptr long long) +@ cdecl wined3d_output_get_ordinal(ptr) @ cdecl wined3d_output_get_raster_status(ptr ptr) @ cdecl wined3d_output_release_ownership(ptr) @ cdecl wined3d_output_set_display_mode(ptr ptr) diff --git a/include/wine/wined3d.h b/include/wine/wined3d.h index 4a06b511211..ac51fdf3528 100644 --- a/include/wine/wined3d.h +++ b/include/wine/wined3d.h @@ -2153,7 +2153,6 @@ struct wined3d_view_desc
struct wined3d_output_desc { - unsigned int ordinal; WCHAR device_name[CCHDEVICENAME]; RECT desktop_rect; BOOL attached_to_desktop; @@ -2557,6 +2556,7 @@ HRESULT __cdecl wined3d_output_get_mode(const struct wined3d_output *output, unsigned int mode_idx, struct wined3d_display_mode *mode); unsigned int __cdecl wined3d_output_get_mode_count(const struct wined3d_output *output, enum wined3d_format_id format_id, enum wined3d_scanline_ordering scanline_ordering); +unsigned int __cdecl wined3d_output_get_ordinal(const struct wined3d_output *output); HRESULT __cdecl wined3d_output_get_raster_status(const struct wined3d_output *output, struct wined3d_raster_status *raster_status); void __cdecl wined3d_output_release_ownership(const struct wined3d_output *output);
On Fri, 13 Nov 2020 at 11:19, Zhiyi Zhang zzhang@codeweavers.com wrote:
Secondhand Lands calls d3d9_device_GetDeviceCaps() frequently. Introduce a wined3d_output_get_ordinal() to get output ordinal only instead of using the more expensive wined3d_output_get_desc().
Avoiding calls to EnumDisplaySettingsExW() is fine in principle, but I don't think these two patches are quite enough to avoid the underlying issue. In particular, wined3d_output_get_raster_status() is a potential source of frequent wined3d_output_get_display_mode() calls, either through d3d9_swapchain_GetRasterStatus() or ddraw7_GetVerticalBlankStatus()/ddraw7_GetScanLine(). See also bug 30538 and related bugs.
On 2020/11/23 21:32, Henri Verbeet wrote:
On Fri, 13 Nov 2020 at 11:19, Zhiyi Zhang zzhang@codeweavers.com wrote:
Secondhand Lands calls d3d9_device_GetDeviceCaps() frequently. Introduce a wined3d_output_get_ordinal() to get output ordinal only instead of using the more expensive wined3d_output_get_desc().
Avoiding calls to EnumDisplaySettingsExW() is fine in principle, but I don't think these two patches are quite enough to avoid the underlying issue. In particular, wined3d_output_get_raster_status() is a potential source of frequent wined3d_output_get_display_mode() calls, either through d3d9_swapchain_GetRasterStatus() or ddraw7_GetVerticalBlankStatus()/ddraw7_GetScanLine(). See also bug 30538 and related bugs.
Yes. I am aware of the root cause being that EnumDisplaySettingsExW() is too expensive. I plan to optimize it later. Meanwhile, I think these two patches are reasonable. Are you proposing another way to fix this?
Thanks, Zhiyi
On Mon, 23 Nov 2020 at 18:06, Zhiyi Zhang zzhang@codeweavers.com wrote:
On 2020/11/23 21:32, Henri Verbeet wrote:
On Fri, 13 Nov 2020 at 11:19, Zhiyi Zhang zzhang@codeweavers.com wrote:
Secondhand Lands calls d3d9_device_GetDeviceCaps() frequently. Introduce a wined3d_output_get_ordinal() to get output ordinal only instead of using the more expensive wined3d_output_get_desc().
Avoiding calls to EnumDisplaySettingsExW() is fine in principle, but I don't think these two patches are quite enough to avoid the underlying issue. In particular, wined3d_output_get_raster_status() is a potential source of frequent wined3d_output_get_display_mode() calls, either through d3d9_swapchain_GetRasterStatus() or ddraw7_GetVerticalBlankStatus()/ddraw7_GetScanLine(). See also bug 30538 and related bugs.
Yes. I am aware of the root cause being that EnumDisplaySettingsExW() is too expensive. I plan to optimize it later. Meanwhile, I think these two patches are reasonable. Are you proposing another way to fix this?
Mostly just making sure that you're aware there are other places affected by this. I also don't particularly love introducing an extra wined3d export for this though; shouldn't the 2/2 patch on its own be enough to fix the immediate regression?
On 2020/11/23 22:54, Henri Verbeet wrote:
On Mon, 23 Nov 2020 at 18:06, Zhiyi Zhang zzhang@codeweavers.com wrote:
On 2020/11/23 21:32, Henri Verbeet wrote:
On Fri, 13 Nov 2020 at 11:19, Zhiyi Zhang zzhang@codeweavers.com wrote:
Secondhand Lands calls d3d9_device_GetDeviceCaps() frequently. Introduce a wined3d_output_get_ordinal() to get output ordinal only instead of using the more expensive wined3d_output_get_desc().
Avoiding calls to EnumDisplaySettingsExW() is fine in principle, but I don't think these two patches are quite enough to avoid the underlying issue. In particular, wined3d_output_get_raster_status() is a potential source of frequent wined3d_output_get_display_mode() calls, either through d3d9_swapchain_GetRasterStatus() or ddraw7_GetVerticalBlankStatus()/ddraw7_GetScanLine(). See also bug 30538 and related bugs.
Yes. I am aware of the root cause being that EnumDisplaySettingsExW() is too expensive. I plan to optimize it later. Meanwhile, I think these two patches are reasonable. Are you proposing another way to fix this?
Mostly just making sure that you're aware there are other places affected by this. I also don't particularly love introducing an extra wined3d export for this though; shouldn't the 2/2 patch on its own be enough to fix the immediate regression?
Yes, for that bug, the 2/2 patch is enough to make the game run at 30fps limit. I just think that d3d9_device_GetDeviceCaps() only needs output ordinals. So calling wined3d_output_get_desc() will do a monitor enumeration that can be avoided, which should improve performance for other applications.
On Mon, 23 Nov 2020 at 18:50, Zhiyi Zhang zzhang@codeweavers.com wrote:
On 2020/11/23 22:54, Henri Verbeet wrote:
On Mon, 23 Nov 2020 at 18:06, Zhiyi Zhang zzhang@codeweavers.com wrote:
On 2020/11/23 21:32, Henri Verbeet wrote:
On Fri, 13 Nov 2020 at 11:19, Zhiyi Zhang zzhang@codeweavers.com wrote:
Secondhand Lands calls d3d9_device_GetDeviceCaps() frequently. Introduce a wined3d_output_get_ordinal() to get output ordinal only instead of using the more expensive wined3d_output_get_desc().
Avoiding calls to EnumDisplaySettingsExW() is fine in principle, but I don't think these two patches are quite enough to avoid the underlying issue. In particular, wined3d_output_get_raster_status() is a potential source of frequent wined3d_output_get_display_mode() calls, either through d3d9_swapchain_GetRasterStatus() or ddraw7_GetVerticalBlankStatus()/ddraw7_GetScanLine(). See also bug 30538 and related bugs.
Yes. I am aware of the root cause being that EnumDisplaySettingsExW() is too expensive. I plan to optimize it later. Meanwhile, I think these two patches are reasonable. Are you proposing another way to fix this?
Mostly just making sure that you're aware there are other places affected by this. I also don't particularly love introducing an extra wined3d export for this though; shouldn't the 2/2 patch on its own be enough to fix the immediate regression?
Yes, for that bug, the 2/2 patch is enough to make the game run at 30fps limit. I just think that d3d9_device_GetDeviceCaps() only needs output ordinals. So calling wined3d_output_get_desc() will do a monitor enumeration that can be avoided, which should improve performance for other applications.
If it ends up taking significant CPU time even after patch 2/2, perhaps it would make sense to simply store the converted D3DCAPS9 structure in struct d3d9; its contents should be stable across calls.
-
Henri Verbeet -
Zhiyi Zhang