[v3 PATCH 1/3] wined3d: Manage source texture resolve in texture2d_blt() for colour blits.
Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> --- v2: - add patch. v3: - introduce wined3d_texture_is_multisample_location(); - move resolve logic to texture2d_blt(). dlls/wined3d/surface.c | 17 ++++++++--------- dlls/wined3d/wined3d_private.h | 10 ++++++++++ 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/dlls/wined3d/surface.c b/dlls/wined3d/surface.c index 50f0c9c06c..49ac894db0 100644 --- a/dlls/wined3d/surface.c +++ b/dlls/wined3d/surface.c @@ -175,13 +175,6 @@ void texture2d_blt_fbo(struct wined3d_device *device, struct wined3d_context *co break; } - /* Resolve the source surface first if needed. */ - if (wined3d_texture_gl_is_multisample_location(wined3d_texture_gl(src_texture), src_location) - && (src_texture->resource.format->id != dst_texture->resource.format->id - || abs(src_rect->bottom - src_rect->top) != abs(dst_rect->bottom - dst_rect->top) - || abs(src_rect->right - src_rect->left) != abs(dst_rect->right - dst_rect->left))) - src_location = WINED3D_LOCATION_RB_RESOLVED; - /* Make sure the locations are up-to-date. Loading the destination * surface isn't required if the entire surface is overwritten. (And is * in fact harmful if we're being called by surface_load_location() with @@ -2402,7 +2395,7 @@ HRESULT texture2d_blt(struct wined3d_texture *dst_texture, unsigned int dst_sub_ struct wined3d_device *device = dst_texture->resource.device; struct wined3d_swapchain *src_swapchain, *dst_swapchain; const struct wined3d_color_key *colour_key = NULL; - DWORD dst_location, valid_locations; + DWORD src_location, dst_location, valid_locations; DWORD src_ds_flags, dst_ds_flags; struct wined3d_context *context; enum wined3d_blit_op blit_op; @@ -2629,8 +2622,14 @@ HRESULT texture2d_blt(struct wined3d_texture *dst_texture, unsigned int dst_sub_ dst_location = dst_texture->resource.map_binding; context = context_acquire(device, dst_texture, dst_sub_resource_idx); + scale = abs(src_box->right - src_box->left) != abs(dst_box->right - dst_box->left) + || abs(src_box->bottom - src_box->top) != abs(dst_box->bottom - dst_box->top); + src_location = (scale || convert || blit_op != WINED3D_BLIT_OP_COLOR_BLIT) + && wined3d_texture_is_multisample_location(src_texture, + src_texture->resource.draw_binding) ? WINED3D_LOCATION_RB_RESOLVED + : src_texture->resource.draw_binding; valid_locations = device->blitter->ops->blitter_blit(device->blitter, blit_op, context, - src_texture, src_sub_resource_idx, src_texture->resource.draw_binding, &src_rect, + src_texture, src_sub_resource_idx, src_location, &src_rect, dst_texture, dst_sub_resource_idx, dst_location, &dst_rect, colour_key, filter); context_release(context); diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h index 0ab6bcfa83..3410e06bb9 100644 --- a/dlls/wined3d/wined3d_private.h +++ b/dlls/wined3d/wined3d_private.h @@ -3808,6 +3808,16 @@ static inline BOOL wined3d_texture_gl_is_multisample_location(const struct wined return texture_gl->target == GL_TEXTURE_2D_MULTISAMPLE || texture_gl->target == GL_TEXTURE_2D_MULTISAMPLE_ARRAY; } +static inline BOOL wined3d_texture_is_multisample_location(const struct wined3d_texture *texture, + DWORD location) +{ + if (location == WINED3D_LOCATION_RB_MULTISAMPLE) + return TRUE; + if (location != WINED3D_LOCATION_TEXTURE_RGB && location != WINED3D_LOCATION_TEXTURE_SRGB) + return FALSE; + return texture->resource.multisample_type != WINED3D_MULTISAMPLE_NONE; +} + void wined3d_texture_gl_apply_sampler_desc(struct wined3d_texture_gl *texture_gl, const struct wined3d_sampler_desc *sampler_desc, const struct wined3d_context_gl *context_gl) DECLSPEC_HIDDEN; void wined3d_texture_gl_bind(struct wined3d_texture_gl *texture_gl, -- 2.24.1
If either source or destination is multisampled scaled FBO blit results in GL_INVALID_OPERATION. Fixes black screen in 'BlazBlue Calamity Trigger'. Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> --- v2: - handle resolve in fbo_blitter_blit(). v3: - handle resolve in texture2d_blt(). dlls/d3d9/tests/visual.c | 88 +++++++++++++++++++++++++++++----------- dlls/wined3d/surface.c | 13 ++++-- 2 files changed, 74 insertions(+), 27 deletions(-) diff --git a/dlls/d3d9/tests/visual.c b/dlls/d3d9/tests/visual.c index 62c053b212..0e71e23018 100644 --- a/dlls/d3d9/tests/visual.c +++ b/dlls/d3d9/tests/visual.c @@ -4332,7 +4332,7 @@ static void test_multisample_stretch_rect(void) D3DTEXF_POINT, D3DTEXF_LINEAR, }; - IDirect3DSurface9 *rt, *ms_rt, *rt_r5g6b5; + IDirect3DSurface9 *rt, *ms_rt, *ms_rt2, *rt_r5g6b5; struct surface_readback rb; IDirect3DDevice9 *device; DWORD quality_levels; @@ -4367,27 +4367,30 @@ static void test_multisample_stretch_rect(void) hr = IDirect3DDevice9_CreateRenderTarget(device, 128, 128, D3DFMT_A8R8G8B8, D3DMULTISAMPLE_NONE, 0, FALSE, &rt, NULL); - ok(hr == S_OK, "Failed to create render target, hr %#x.\n", hr); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); hr = IDirect3DDevice9_CreateRenderTarget(device, 128, 128, D3DFMT_A8R8G8B8, D3DMULTISAMPLE_2_SAMPLES, quality_levels - 1, FALSE, &ms_rt, NULL); - ok(hr == S_OK, "Failed to create render target, hr %#x.\n", hr); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + hr = IDirect3DDevice9_CreateRenderTarget(device, 128, 128, + D3DFMT_A8R8G8B8, D3DMULTISAMPLE_2_SAMPLES, quality_levels - 1, FALSE, &ms_rt2, NULL); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); hr = IDirect3DDevice9_SetRenderTarget(device, 0, ms_rt); - ok(hr == D3D_OK, "Failed to set render target, hr %#x.\n", hr); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); hr = IDirect3DDevice9_Clear(device, 0, NULL, D3DCLEAR_TARGET, 0x00ff00ff, 0.0f, 0); - ok(hr == D3D_OK, "Failed to clear, hr %#x.\n", hr); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); hr = IDirect3DDevice9_SetRenderTarget(device, 0, rt); - ok(hr == D3D_OK, "Failed to set render target, hr %#x.\n", hr); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); for (i = 0; i < ARRAY_SIZE(filters); ++i) { hr = IDirect3DDevice9_Clear(device, 0, NULL, D3DCLEAR_TARGET, 0xffffffff, 0.0f, 0); - ok(hr == D3D_OK, "Test %u: Failed to clear, hr %#x.\n", i, hr); + ok(hr == D3D_OK, "Test %u, got unexpected hr %#x.\n", i, hr); hr = IDirect3DDevice9_StretchRect(device, ms_rt, NULL, rt, NULL, filters[i]); - ok(hr == S_OK, "Test %u: Failed to stretch rect, hr %#x.\n", i, hr); + ok(hr == D3D_OK, "Test %u, got unexpected hr %#x.\n", i, hr); color = getPixelColor(device, 64, 64); - ok(color == 0x00ff00ff, "Test %u: Got color 0x%08x.\n", i, color); + ok(color == 0x00ff00ff, "Test %u, got unexpected color 0x%08x.\n", i, color); } /* Scaling */ @@ -4395,29 +4398,67 @@ static void test_multisample_stretch_rect(void) for (i = 0; i < ARRAY_SIZE(filters); ++i) { hr = IDirect3DDevice9_Clear(device, 0, NULL, D3DCLEAR_TARGET, 0xffffffff, 0.0f, 0); - ok(hr == D3D_OK, "Test %u: Failed to clear, hr %#x.\n", i, hr); + ok(hr == D3D_OK, "Test %u, got unexpected hr %#x.\n", i, hr); + hr = IDirect3DDevice9_StretchRect(device, rt, NULL, ms_rt2, NULL, D3DTEXF_NONE); + ok(hr == D3D_OK, "Test %u, got unexpected hr %#x.\n", i, hr); + hr = IDirect3DDevice9_StretchRect(device, ms_rt, NULL, rt, &rect, filters[i]); - ok(hr == S_OK, "Test %u: Failed to stretch rect, hr %#x.\n", i, hr); + ok(hr == D3D_OK, "Test %u, got unexpected hr %#x.\n", i, hr); + get_rt_readback(rt, &rb); + color = get_readback_color(&rb, 32, 32); + ok(color == 0x00ff00ff, "Test %u, got unexpected color 0x%08x.\n", i, color); + color = get_readback_color(&rb, 64, 64); + ok(color == 0xffffffff, "Test %u, got unexpected color 0x%08x.\n", i, color); + color = get_readback_color(&rb, 96, 96); + ok(color == 0xffffffff, "Test %u, got unexpected color 0x%08x.\n", i, color); + release_surface_readback(&rb); + + hr = IDirect3DDevice9_Clear(device, 0, NULL, D3DCLEAR_TARGET, 0xffffffff, 0.0f, 0); + ok(hr == D3D_OK, "Test %u, got unexpected hr %#x.\n", i, hr); + hr = IDirect3DDevice9_StretchRect(device, ms_rt, &rect, rt, NULL, filters[i]); + ok(hr == D3D_OK, "Test %u, got unexpected hr %#x.\n", i, hr); get_rt_readback(rt, &rb); color = get_readback_color(&rb, 32, 32); - ok(color == 0x00ff00ff, "Test %u: Got color 0x%08x.\n", i, color); + ok(color == 0x00ff00ff, "Test %u, got unexpected color 0x%08x.\n", i, color); color = get_readback_color(&rb, 64, 64); - ok(color == 0xffffffff, "Test %u: Got color 0x%08x.\n", i, color); + ok(color == 0x00ff00ff, "Test %u, got unexpected color 0x%08x.\n", i, color); color = get_readback_color(&rb, 96, 96); - ok(color == 0xffffffff, "Test %u: Got color 0x%08x.\n", i, color); + ok(color == 0x00ff00ff, "Test %u, got unexpected color 0x%08x.\n", i, color); release_surface_readback(&rb); + hr = IDirect3DDevice9_Clear(device, 0, NULL, D3DCLEAR_TARGET, 0xffffff00, 0.0f, 0); + ok(hr == D3D_OK, "Test %u, got unexpected hr %#x.\n", i, hr); + hr = IDirect3DDevice9_StretchRect(device, rt, NULL, ms_rt, &rect, filters[i]); + ok(hr == D3D_OK, "Test %u, got unexpected hr %#x.\n", i, hr); hr = IDirect3DDevice9_Clear(device, 0, NULL, D3DCLEAR_TARGET, 0xffffffff, 0.0f, 0); - ok(hr == D3D_OK, "Test %u: Failed to clear, hr %#x.\n", i, hr); + ok(hr == D3D_OK, "Test %u, got unexpected hr %#x.\n", i, hr); hr = IDirect3DDevice9_StretchRect(device, ms_rt, &rect, rt, NULL, filters[i]); - ok(hr == S_OK, "Test %u: Failed to stretch rect, hr %#x.\n", i, hr); + ok(hr == D3D_OK, "Test %u, got unexpected hr %#x.\n", i, hr); + get_rt_readback(rt, &rb); + color = get_readback_color(&rb, 32, 32); + ok(color == 0xffffff00, "Test %u, got unexpected color 0x%08x.\n", i, color); + color = get_readback_color(&rb, 64, 64); + ok(color == 0xffffff00, "Test %u, got unexpected color 0x%08x.\n", i, color); + color = get_readback_color(&rb, 96, 96); + ok(color == 0xffffff00, "Test %u, got unexpected color 0x%08x.\n", i, color); + release_surface_readback(&rb); + + hr = IDirect3DDevice9_Clear(device, 0, NULL, D3DCLEAR_TARGET, 0x00ff00ff, 0.0f, 0); + ok(hr == D3D_OK, "Test %u, got unexpected hr %#x.\n", i, hr); + hr = IDirect3DDevice9_StretchRect(device, rt, NULL, ms_rt, NULL, D3DTEXF_NONE); + ok(hr == D3D_OK, "Test %u, got unexpected hr %#x.\n", i, hr); + + hr = IDirect3DDevice9_StretchRect(device, ms_rt, &rect, ms_rt2, NULL, filters[i]); + ok(hr == D3D_OK, "Test %u, got unexpected hr %#x.\n", i, hr); + hr = IDirect3DDevice9_StretchRect(device, ms_rt2, &rect, rt, NULL, filters[i]); + ok(hr == D3D_OK, "Test %u, got unexpected hr %#x.\n", i, hr); get_rt_readback(rt, &rb); color = get_readback_color(&rb, 32, 32); - ok(color == 0x00ff00ff, "Test %u: Got color 0x%08x.\n", i, color); + ok(color == 0x00ff00ff, "Test %u, got unexpected color 0x%08x.\n", i, color); color = get_readback_color(&rb, 64, 64); - ok(color == 0x00ff00ff, "Test %u: Got color 0x%08x.\n", i, color); + ok(color == 0x00ff00ff, "Test %u, got unexpected color 0x%08x.\n", i, color); color = get_readback_color(&rb, 96, 96); - ok(color == 0x00ff00ff, "Test %u: Got color 0x%08x.\n", i, color); + ok(color == 0x00ff00ff, "Test %u, got unexpected color 0x%08x.\n", i, color); release_surface_readback(&rb); } @@ -4433,18 +4474,19 @@ static void test_multisample_stretch_rect(void) for (i = 0; i < ARRAY_SIZE(filters); ++i) { hr = IDirect3DDevice9_Clear(device, 0, NULL, D3DCLEAR_TARGET, 0xffffffff, 0.0f, 0); - ok(hr == D3D_OK, "Test %u: Failed to clear, hr %#x.\n", i, hr); + ok(hr == D3D_OK, "Test %u, got unexpected hr %#x.\n", i, hr); hr = IDirect3DDevice9_StretchRect(device, ms_rt, NULL, rt_r5g6b5, NULL, filters[i]); - ok(hr == S_OK, "Test %u: Failed to stretch rect, hr %#x.\n", i, hr); + ok(hr == D3D_OK, "Test %u, got unexpected hr %#x.\n", i, hr); hr = IDirect3DDevice9_StretchRect(device, rt_r5g6b5, NULL, rt, NULL, filters[i]); - ok(hr == S_OK, "Test %u: Failed to stretch rect, hr %#x.\n", i, hr); + ok(hr == D3D_OK, "Test %u, got unexpected hr %#x.\n", i, hr); color = getPixelColor(device, 64, 64); - ok(color == 0x00ff00ff, "Test %u: Got color 0x%08x.\n", i, color); + ok(color == 0x00ff00ff, "Test %u, got unexpected color 0x%08x.\n", i, color); } IDirect3DSurface9_Release(rt_r5g6b5); done: + IDirect3DSurface9_Release(ms_rt2); IDirect3DSurface9_Release(ms_rt); IDirect3DSurface9_Release(rt); refcount = IDirect3DDevice9_Release(device); diff --git a/dlls/wined3d/surface.c b/dlls/wined3d/surface.c index 49ac894db0..8616626162 100644 --- a/dlls/wined3d/surface.c +++ b/dlls/wined3d/surface.c @@ -2624,10 +2624,15 @@ HRESULT texture2d_blt(struct wined3d_texture *dst_texture, unsigned int dst_sub_ context = context_acquire(device, dst_texture, dst_sub_resource_idx); scale = abs(src_box->right - src_box->left) != abs(dst_box->right - dst_box->left) || abs(src_box->bottom - src_box->top) != abs(dst_box->bottom - dst_box->top); - src_location = (scale || convert || blit_op != WINED3D_BLIT_OP_COLOR_BLIT) - && wined3d_texture_is_multisample_location(src_texture, - src_texture->resource.draw_binding) ? WINED3D_LOCATION_RB_RESOLVED - : src_texture->resource.draw_binding; + src_location = src_texture->resource.draw_binding; + if (scale || convert || blit_op != WINED3D_BLIT_OP_COLOR_BLIT) + { + if (wined3d_texture_is_multisample_location(dst_texture, dst_location)) + dst_location = WINED3D_LOCATION_RB_RESOLVED; + + if (wined3d_texture_is_multisample_location(src_texture, src_location)) + src_location = WINED3D_LOCATION_RB_RESOLVED; + } valid_locations = device->blitter->ops->blitter_blit(device->blitter, blit_op, context, src_texture, src_sub_resource_idx, src_location, &src_rect, dst_texture, dst_sub_resource_idx, dst_location, &dst_rect, colour_key, filter); -- 2.24.1
Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> --- v3: - introduce d3d_info->scaled_resolve and handle it in texture2d_blt(). dlls/wined3d/adapter_gl.c | 4 ++++ dlls/wined3d/surface.c | 12 +++++++++--- dlls/wined3d/wined3d_gl.h | 1 + dlls/wined3d/wined3d_private.h | 1 + 4 files changed, 15 insertions(+), 3 deletions(-) diff --git a/dlls/wined3d/adapter_gl.c b/dlls/wined3d/adapter_gl.c index 6e60c5323e..7b5d121402 100644 --- a/dlls/wined3d/adapter_gl.c +++ b/dlls/wined3d/adapter_gl.c @@ -179,6 +179,8 @@ static const struct wined3d_extension_map gl_extension_map[] = {"GL_EXT_fog_coord", EXT_FOG_COORD }, {"GL_EXT_framebuffer_blit", EXT_FRAMEBUFFER_BLIT }, {"GL_EXT_framebuffer_multisample", EXT_FRAMEBUFFER_MULTISAMPLE }, + {"GL_EXT_framebuffer_multisample_blit_scaled", + EXT_FRAMEBUFFER_MULTISAMPLE_BLIT_SCALED}, {"GL_EXT_framebuffer_object", EXT_FRAMEBUFFER_OBJECT }, {"GL_EXT_memory_object", EXT_MEMORY_OBJECT }, {"GL_EXT_gpu_program_parameters", EXT_GPU_PROGRAM_PARAMETERS }, @@ -5162,6 +5164,8 @@ static void wined3d_adapter_gl_init_d3d_info(struct wined3d_adapter_gl *adapter_ d3d_info->srgb_write_control = !!gl_info->supported[ARB_FRAMEBUFFER_SRGB]; d3d_info->clip_control = !!gl_info->supported[ARB_CLIP_CONTROL]; d3d_info->full_ffp_varyings = !!(shader_caps.wined3d_caps & WINED3D_SHADER_CAP_FULL_FFP_VARYINGS); + d3d_info->scaled_resolve = gl_info->supported[EXT_FRAMEBUFFER_MULTISAMPLE_BLIT_SCALED] + || !gl_info->supported[ARB_TEXTURE_MULTISAMPLE]; d3d_info->feature_level = feature_level_from_caps(gl_info, &shader_caps, &fragment_caps); if (gl_info->supported[ARB_TEXTURE_MULTISAMPLE]) diff --git a/dlls/wined3d/surface.c b/dlls/wined3d/surface.c index 8616626162..ed8512240a 100644 --- a/dlls/wined3d/surface.c +++ b/dlls/wined3d/surface.c @@ -150,6 +150,7 @@ void texture2d_blt_fbo(struct wined3d_device *device, struct wined3d_context *co const struct wined3d_gl_info *gl_info; struct wined3d_context_gl *context_gl; unsigned int restore_idx; + BOOL use_resolve_filter; GLenum gl_filter; GLenum buffer; RECT s, d; @@ -160,10 +161,14 @@ void texture2d_blt_fbo(struct wined3d_device *device, struct wined3d_context *co wined3d_debug_location(src_location), wine_dbgstr_rect(src_rect), dst_texture, dst_sub_resource_idx, wined3d_debug_location(dst_location), wine_dbgstr_rect(dst_rect)); + use_resolve_filter = wined3d_texture_gl_is_multisample_location(wined3d_texture_gl(src_texture), src_location) + && (abs(src_rect->bottom - src_rect->top) != abs(dst_rect->bottom - dst_rect->top) + || abs(src_rect->right - src_rect->left) != abs(dst_rect->right - dst_rect->left)); + switch (filter) { case WINED3D_TEXF_LINEAR: - gl_filter = GL_LINEAR; + gl_filter = use_resolve_filter ? GL_SCALED_RESOLVE_NICEST_EXT : GL_LINEAR; break; default: @@ -171,7 +176,7 @@ void texture2d_blt_fbo(struct wined3d_device *device, struct wined3d_context *co /* fall through */ case WINED3D_TEXF_NONE: case WINED3D_TEXF_POINT: - gl_filter = GL_NEAREST; + gl_filter = use_resolve_filter ? GL_SCALED_RESOLVE_FASTEST_EXT : GL_NEAREST; break; } @@ -2630,7 +2635,8 @@ HRESULT texture2d_blt(struct wined3d_texture *dst_texture, unsigned int dst_sub_ if (wined3d_texture_is_multisample_location(dst_texture, dst_location)) dst_location = WINED3D_LOCATION_RB_RESOLVED; - if (wined3d_texture_is_multisample_location(src_texture, src_location)) + if ((!context->d3d_info->scaled_resolve || blit_op != WINED3D_BLIT_OP_COLOR_BLIT || convert) + && wined3d_texture_is_multisample_location(src_texture, src_location)) src_location = WINED3D_LOCATION_RB_RESOLVED; } valid_locations = device->blitter->ops->blitter_blit(device->blitter, blit_op, context, diff --git a/dlls/wined3d/wined3d_gl.h b/dlls/wined3d/wined3d_gl.h index 3372b4b6be..2cb25a3776 100644 --- a/dlls/wined3d/wined3d_gl.h +++ b/dlls/wined3d/wined3d_gl.h @@ -160,6 +160,7 @@ enum wined3d_gl_extension EXT_FOG_COORD, EXT_FRAMEBUFFER_BLIT, EXT_FRAMEBUFFER_MULTISAMPLE, + EXT_FRAMEBUFFER_MULTISAMPLE_BLIT_SCALED, EXT_FRAMEBUFFER_OBJECT, EXT_GPU_PROGRAM_PARAMETERS, EXT_GPU_SHADER4, diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h index 3410e06bb9..848996abe0 100644 --- a/dlls/wined3d/wined3d_private.h +++ b/dlls/wined3d/wined3d_private.h @@ -226,6 +226,7 @@ struct wined3d_d3d_info uint32_t srgb_write_control : 1; uint32_t clip_control : 1; uint32_t full_ffp_varyings : 1; + uint32_t scaled_resolve : 1; enum wined3d_feature_level feature_level; DWORD multisample_draw_location; -- 2.24.1
On Tue, 10 Mar 2020 at 15:02, Paul Gofman <gofmanp(a)gmail.com> wrote:
@@ -5162,6 +5164,8 @@ static void wined3d_adapter_gl_init_d3d_info(struct wined3d_adapter_gl *adapter_ d3d_info->srgb_write_control = !!gl_info->supported[ARB_FRAMEBUFFER_SRGB]; d3d_info->clip_control = !!gl_info->supported[ARB_CLIP_CONTROL]; d3d_info->full_ffp_varyings = !!(shader_caps.wined3d_caps & WINED3D_SHADER_CAP_FULL_FFP_VARYINGS); + d3d_info->scaled_resolve = gl_info->supported[EXT_FRAMEBUFFER_MULTISAMPLE_BLIT_SCALED] + || !gl_info->supported[ARB_TEXTURE_MULTISAMPLE]; Is the ARB_TEXTURE_MULTISAMPLE check correct?
@@ -2630,7 +2635,8 @@ HRESULT texture2d_blt(struct wined3d_texture *dst_texture, unsigned int dst_sub_ if (wined3d_texture_is_multisample_location(dst_texture, dst_location)) dst_location = WINED3D_LOCATION_RB_RESOLVED;
- if (wined3d_texture_is_multisample_location(src_texture, src_location)) + if ((!context->d3d_info->scaled_resolve || blit_op != WINED3D_BLIT_OP_COLOR_BLIT || convert) + && wined3d_texture_is_multisample_location(src_texture, src_location)) src_location = WINED3D_LOCATION_RB_RESOLVED; } If scaled resolves are supported, we don't need to modify "dst_location" either, right?
On 3/11/20 15:23, Henri Verbeet wrote:
On Tue, 10 Mar 2020 at 15:02, Paul Gofman <gofmanp(a)gmail.com> wrote:
@@ -5162,6 +5164,8 @@ static void wined3d_adapter_gl_init_d3d_info(struct wined3d_adapter_gl *adapter_ d3d_info->srgb_write_control = !!gl_info->supported[ARB_FRAMEBUFFER_SRGB]; d3d_info->clip_control = !!gl_info->supported[ARB_CLIP_CONTROL]; d3d_info->full_ffp_varyings = !!(shader_caps.wined3d_caps & WINED3D_SHADER_CAP_FULL_FFP_VARYINGS); + d3d_info->scaled_resolve = gl_info->supported[EXT_FRAMEBUFFER_MULTISAMPLE_BLIT_SCALED] + || !gl_info->supported[ARB_TEXTURE_MULTISAMPLE]; Is the ARB_TEXTURE_MULTISAMPLE check correct? The idea was that if ARB_TEXTURE_MULTISAMPLE is not supported we should not getting multisampled source texture while wined3d_texture_is_multisample_location() from previous patch may say otherwise in this case (as you noted previously). But it indeed looks much more straightforward to just improve wined3d_texture_is_multisample_location() instead.
@@ -2630,7 +2635,8 @@ HRESULT texture2d_blt(struct wined3d_texture *dst_texture, unsigned int dst_sub_ if (wined3d_texture_is_multisample_location(dst_texture, dst_location)) dst_location = WINED3D_LOCATION_RB_RESOLVED;
- if (wined3d_texture_is_multisample_location(src_texture, src_location)) + if ((!context->d3d_info->scaled_resolve || blit_op != WINED3D_BLIT_OP_COLOR_BLIT || convert) + && wined3d_texture_is_multisample_location(src_texture, src_location)) src_location = WINED3D_LOCATION_RB_RESOLVED; } If scaled resolves are supported, we don't need to modify "dst_location" either, right?
No, we have to. Scaled resolve allows to scale from multisampled texture, but if destination is multisampled it always fails for me. There is explicit check for zero samples count on destination I spotted in Mesa code, and it seems to fail the same way on Nvidia.
On Wed, 11 Mar 2020 at 16:30, Paul Gofman <gofmanp(a)gmail.com> wrote:
@@ -2630,7 +2635,8 @@ HRESULT texture2d_blt(struct wined3d_texture *dst_texture, unsigned int dst_sub_ if (wined3d_texture_is_multisample_location(dst_texture, dst_location)) dst_location = WINED3D_LOCATION_RB_RESOLVED;
- if (wined3d_texture_is_multisample_location(src_texture, src_location)) + if ((!context->d3d_info->scaled_resolve || blit_op != WINED3D_BLIT_OP_COLOR_BLIT || convert) + && wined3d_texture_is_multisample_location(src_texture, src_location)) src_location = WINED3D_LOCATION_RB_RESOLVED; } If scaled resolves are supported, we don't need to modify "dst_location" either, right?
No, we have to. Scaled resolve allows to scale from multisampled texture, but if destination is multisampled it always fails for me. There is explicit check for zero samples count on destination I spotted in Mesa code, and it seems to fail the same way on Nvidia.
Right, I checked the spec and it does say that. That limits the benefit of EXT_framebuffer_multisample_blit_scaled a bit, but I guess it's fine.
On 3/11/20 18:25, Henri Verbeet wrote:
On Wed, 11 Mar 2020 at 16:30, Paul Gofman <gofmanp(a)gmail.com> wrote:
No, we have to. Scaled resolve allows to scale from multisampled texture, but if destination is multisampled it always fails for me. There is explicit check for zero samples count on destination I spotted in Mesa code, and it seems to fail the same way on Nvidia.
Right, I checked the spec and it does say that. That limits the benefit of EXT_framebuffer_multisample_blit_scaled a bit, but I guess it's fine.
I can guess it was not immediately clear how and why resample coverage masks other than resolve it first and then stretch. I hope even with this limitation it is still beneficial for the majority of practical cases as resolving source to destination is probably a more common use case (for which it works). Also if the destination surface is fully overwritten we avoid an extra blit for resolving destination surface by using _prepare_location() instead of _load_location() in texture2d_blt_fbo().
On Tue, 10 Mar 2020 at 15:01, Paul Gofman <gofmanp(a)gmail.com> wrote:
@@ -2629,8 +2622,14 @@ HRESULT texture2d_blt(struct wined3d_texture *dst_texture, unsigned int dst_sub_ dst_location = dst_texture->resource.map_binding;
context = context_acquire(device, dst_texture, dst_sub_resource_idx); + scale = abs(src_box->right - src_box->left) != abs(dst_box->right - dst_box->left) + || abs(src_box->bottom - src_box->top) != abs(dst_box->bottom - dst_box->top); "scale" is already initialised earlier in the function. It's true that this variant would allow flips and mirrors whereas the variant earlier in the texture2d_blt() doesn't, but flipped/mirrored boxes aren't valid for texture2d_blt(). (In particular, wined3d_texture_blt() calls wined3d_texture_check_box_dimensions(); callers would use WINEDDBLTFX_MIRROR* to do flipped/mirrored blits.)
+ src_location = (scale || convert || blit_op != WINED3D_BLIT_OP_COLOR_BLIT) + && wined3d_texture_is_multisample_location(src_texture, + src_texture->resource.draw_binding) ? WINED3D_LOCATION_RB_RESOLVED + : src_texture->resource.draw_binding; is the "blit_op" check needed?
+static inline BOOL wined3d_texture_is_multisample_location(const struct wined3d_texture *texture, + DWORD location) +{ + if (location == WINED3D_LOCATION_RB_MULTISAMPLE) + return TRUE; + if (location != WINED3D_LOCATION_TEXTURE_RGB && location != WINED3D_LOCATION_TEXTURE_SRGB) + return FALSE; + return texture->resource.multisample_type != WINED3D_MULTISAMPLE_NONE; +} return location == d3d_info->multisample_draw_location && texture->resource.multisample_type != WINED3D_MULTISAMPLE_NONE;
Otherwise, calling the above would give an incorrect result when called with e.g. WINED3D_LOCATION_TEXTURE_RGB on a multisampled texture when ARB_texture_multisample is not used.
On 3/11/20 15:22, Henri Verbeet wrote:
On Tue, 10 Mar 2020 at 15:01, Paul Gofman <gofmanp(a)gmail.com> wrote:
@@ -2629,8 +2622,14 @@ HRESULT texture2d_blt(struct wined3d_texture *dst_texture, unsigned int dst_sub_ dst_location = dst_texture->resource.map_binding;
context = context_acquire(device, dst_texture, dst_sub_resource_idx); + scale = abs(src_box->right - src_box->left) != abs(dst_box->right - dst_box->left) + || abs(src_box->bottom - src_box->top) != abs(dst_box->bottom - dst_box->top); "scale" is already initialised earlier in the function. It's true that this variant would allow flips and mirrors whereas the variant earlier in the texture2d_blt() doesn't, but flipped/mirrored boxes aren't valid for texture2d_blt(). (In particular, wined3d_texture_blt() calls wined3d_texture_check_box_dimensions(); callers would use WINEDDBLTFX_MIRROR* to do flipped/mirrored blits.)
+ src_location = (scale || convert || blit_op != WINED3D_BLIT_OP_COLOR_BLIT) + && wined3d_texture_is_multisample_location(src_texture, + src_texture->resource.draw_binding) ? WINED3D_LOCATION_RB_RESOLVED + : src_texture->resource.draw_binding; is the "blit_op" check needed? I think it is not strictly necessary as colour key or alpha test blits won't end up in FBO blitter anyway, but I thought it is less fragile and more clear to be explicit here. Should I remove that? +static inline BOOL wined3d_texture_is_multisample_location(const struct wined3d_texture *texture, + DWORD location) +{ + if (location == WINED3D_LOCATION_RB_MULTISAMPLE) + return TRUE; + if (location != WINED3D_LOCATION_TEXTURE_RGB && location != WINED3D_LOCATION_TEXTURE_SRGB) + return FALSE; + return texture->resource.multisample_type != WINED3D_MULTISAMPLE_NONE; +} return location == d3d_info->multisample_draw_location && texture->resource.multisample_type != WINED3D_MULTISAMPLE_NONE;
Otherwise, calling the above would give an incorrect result when called with e.g. WINED3D_LOCATION_TEXTURE_RGB on a multisampled texture when ARB_texture_multisample is not used.
I might have missed something but can't WINED3D_LOCATION_TEXTURE_SRGB be a multisample location while d3dinfo->multisample_draw_location will be WINED3D_LOCATION_TEXTURE_RGB if ARB_texture_multisample is used? If that is the case, maybe this should be 'return texture->resource.multisample_type != WINED3D_MULTISAMPLE_NONE && (d3d_info->multisample_draw_location == WINED3D_LOCATION_TEXTURE_RGB)'?
On Wed, 11 Mar 2020 at 16:25, Paul Gofman <gofmanp(a)gmail.com> wrote:
On 3/11/20 15:22, Henri Verbeet wrote:
On Tue, 10 Mar 2020 at 15:01, Paul Gofman <gofmanp(a)gmail.com> wrote:
@@ -2629,8 +2622,14 @@ HRESULT texture2d_blt(struct wined3d_texture *dst_texture, unsigned int dst_sub_ dst_location = dst_texture->resource.map_binding;
context = context_acquire(device, dst_texture, dst_sub_resource_idx); + scale = abs(src_box->right - src_box->left) != abs(dst_box->right - dst_box->left) + || abs(src_box->bottom - src_box->top) != abs(dst_box->bottom - dst_box->top); "scale" is already initialised earlier in the function. It's true that this variant would allow flips and mirrors whereas the variant earlier in the texture2d_blt() doesn't, but flipped/mirrored boxes aren't valid for texture2d_blt(). (In particular, wined3d_texture_blt() calls wined3d_texture_check_box_dimensions(); callers would use WINEDDBLTFX_MIRROR* to do flipped/mirrored blits.)
+ src_location = (scale || convert || blit_op != WINED3D_BLIT_OP_COLOR_BLIT) + && wined3d_texture_is_multisample_location(src_texture, + src_texture->resource.draw_binding) ? WINED3D_LOCATION_RB_RESOLVED + : src_texture->resource.draw_binding; is the "blit_op" check needed? I think it is not strictly necessary as colour key or alpha test blits won't end up in FBO blitter anyway, but I thought it is less fragile and more clear to be explicit here. Should I remove that?
To the extent that it makes a difference for those blit types, I think using the resolved location makes sense there as well.
+static inline BOOL wined3d_texture_is_multisample_location(const struct wined3d_texture *texture, + DWORD location) +{ + if (location == WINED3D_LOCATION_RB_MULTISAMPLE) + return TRUE; + if (location != WINED3D_LOCATION_TEXTURE_RGB && location != WINED3D_LOCATION_TEXTURE_SRGB) + return FALSE; + return texture->resource.multisample_type != WINED3D_MULTISAMPLE_NONE; +} return location == d3d_info->multisample_draw_location && texture->resource.multisample_type != WINED3D_MULTISAMPLE_NONE;
Otherwise, calling the above would give an incorrect result when called with e.g. WINED3D_LOCATION_TEXTURE_RGB on a multisampled texture when ARB_texture_multisample is not used.
I might have missed something but can't WINED3D_LOCATION_TEXTURE_SRGB be a multisample location while d3dinfo->multisample_draw_location will be WINED3D_LOCATION_TEXTURE_RGB if ARB_texture_multisample is used? If that is the case, maybe this should be
'return texture->resource.multisample_type != WINED3D_MULTISAMPLE_NONE && (d3d_info->multisample_draw_location == WINED3D_LOCATION_TEXTURE_RGB)'?
I think you're right, although ARB_texture_multisample without EXT_texture_sRGB_decode isn't terribly likely. Note that in the caller that essentially reduces to the following though: if (!(dst_texture->resource.access & WINED3D_RESOURCE_ACCESS_GPU)) dst_location = dst_texture->resource.map_binding; else if ((scale || convert) && dst_texture->resource.multisample_type != WINED3D_MULTISAMPLE_NONE) dst_location = WINED3D_LOCATION_RB_RESOLVED; else dst_location = dst_texture->resource.draw_binding;
On Wed, 11 Mar 2020 at 18:25 Henri Verbeet <hverbeet(a)gmail.com> wrote:
On Wed, 11 Mar 2020 at 16:25, Paul Gofman <gofmanp(a)gmail.com> wrote:
On 3/11/20 15:22, Henri Verbeet wrote:
On Tue, 10 Mar 2020 at 15:01, Paul Gofman <gofmanp(a)gmail.com> wrote:
@@ -2629,8 +2622,14 @@ HRESULT texture2d_blt(struct wined3d_texture *dst_texture, unsigned int dst_sub_ dst_location = dst_texture->resource.map_binding;
context = context_acquire(device, dst_texture, dst_sub_resource_idx); + scale = abs(src_box->right - src_box->left) != abs(dst_box->right - dst_box->left) + || abs(src_box->bottom - src_box->top) != abs(dst_box->bottom - dst_box->top); "scale" is already initialised earlier in the function. It's true that this variant would allow flips and mirrors whereas the variant earlier in the texture2d_blt() doesn't, but flipped/mirrored boxes aren't valid for texture2d_blt(). (In particular, wined3d_texture_blt() calls wined3d_texture_check_box_dimensions(); callers would use WINEDDBLTFX_MIRROR* to do flipped/mirrored blits.)
+ src_location = (scale || convert || blit_op != WINED3D_BLIT_OP_COLOR_BLIT) + && wined3d_texture_is_multisample_location(src_texture, + src_texture->resource.draw_binding) ? WINED3D_LOCATION_RB_RESOLVED + : src_texture->resource.draw_binding; is the "blit_op" check needed? I think it is not strictly necessary as colour key or alpha test blits won't end up in FBO blitter anyway, but I thought it is less fragile and more clear to be explicit here. Should I remove that?
To the extent that it makes a difference for those blit types, I think using the resolved location makes sense there as well.
Yes, that what I was trying to do, and additionally resolve for special blits even if the size and format is the same, that’s what I put blit_op check for. Should I remove that? Anyway, it is probably a separate change.
On Wed, 11 Mar 2020 at 19:10, Paul Gofman <gofmanp(a)gmail.com> wrote:
On Wed, 11 Mar 2020 at 18:25 Henri Verbeet <hverbeet(a)gmail.com> wrote:
On Wed, 11 Mar 2020 at 16:25, Paul Gofman <gofmanp(a)gmail.com> wrote:
On 3/11/20 15:22, Henri Verbeet wrote:
On Tue, 10 Mar 2020 at 15:01, Paul Gofman <gofmanp(a)gmail.com> wrote:
+ src_location = (scale || convert || blit_op != WINED3D_BLIT_OP_COLOR_BLIT) + && wined3d_texture_is_multisample_location(src_texture, + src_texture->resource.draw_binding) ? WINED3D_LOCATION_RB_RESOLVED + : src_texture->resource.draw_binding; is the "blit_op" check needed? I think it is not strictly necessary as colour key or alpha test blits won't end up in FBO blitter anyway, but I thought it is less fragile and more clear to be explicit here. Should I remove that?
To the extent that it makes a difference for those blit types, I think using the resolved location makes sense there as well.
Yes, that what I was trying to do, and additionally resolve for special blits even if the size and format is the same, that’s what I put blit_op check for. Should I remove that? Anyway, it is probably a separate change.
I think keeping the "blit_op" check makes sense.
participants (2)
-
Henri Verbeet -
Paul Gofman