[v3 PATCH 1/3] wined3d: Manage source texture resolve in texture2d_blt() for colour blits.
Signed-off-by: Paul Gofman gofmanp@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,
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@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);
Signed-off-by: Paul Gofman gofmanp@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;
On Tue, 10 Mar 2020 at 15:02, Paul Gofman gofmanp@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@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@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@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@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@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@gmail.com wrote:
On 3/11/20 15:22, Henri Verbeet wrote:
On Tue, 10 Mar 2020 at 15:01, Paul Gofman gofmanp@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@gmail.com wrote:
On Wed, 11 Mar 2020 at 16:25, Paul Gofman gofmanp@gmail.com wrote:
On 3/11/20 15:22, Henri Verbeet wrote:
On Tue, 10 Mar 2020 at 15:01, Paul Gofman gofmanp@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@gmail.com wrote:
On Wed, 11 Mar 2020 at 18:25 Henri Verbeet hverbeet@gmail.com wrote:
On Wed, 11 Mar 2020 at 16:25, Paul Gofman gofmanp@gmail.com wrote:
On 3/11/20 15:22, Henri Verbeet wrote:
On Tue, 10 Mar 2020 at 15:01, Paul Gofman gofmanp@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.
-
Henri Verbeet -
Paul Gofman