[PATCH v3 0/5] MR109: wined3d: Add a memory_colour_fill path for clearing the entire level to zero.
Signed-off-by: Stefan Dösinger stefan@codeweavers.com
-- v3: d3d9/tests: Extend color_fill_test. wined3d: Store clear colors in subresources. wined3d: Handle WINED3DFMT_D32_FLOAT in wined3d_format_convert_from_float. wined3d: Prepare sysmem LOCATION_CLEARED handling for non-zero colors (v2). wined3d: Add a memory_colour_fill path for clearing the entire level to zero.
From: Stefan Dösinger stefan@codeweavers.com
Signed-off-by: Stefan Dösinger stefan@codeweavers.com --- dlls/wined3d/resource.c | 11 ++++++++++- dlls/wined3d/surface.c | 6 +++++- dlls/wined3d/wined3d_private.h | 2 +- 3 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/dlls/wined3d/resource.c b/dlls/wined3d/resource.c index aeece1ab512..857750d6bd7 100644 --- a/dlls/wined3d/resource.c +++ b/dlls/wined3d/resource.c @@ -618,13 +618,22 @@ void *resource_offset_map_pointer(struct wined3d_resource *resource, unsigned in
void wined3d_resource_memory_colour_fill(struct wined3d_resource *resource, const struct wined3d_map_desc *map, const struct wined3d_color *colour, - const struct wined3d_box *box) + const struct wined3d_box *box, bool full_subresource) { const struct wined3d_format *format = resource->format; unsigned int w, h, d, x, y, z, bpp; uint8_t *dst, *dst2; uint32_t c[4];
+ /* Fast and simple path for setting everything to zero. The C library's memset is + * more sophisticated than our code below. Also this works for block formats, which + * we still need to zero-initialize for newly created resources. */ + if (full_subresource && !colour->r && !colour->g && !colour->b && !colour->a) + { + memset(map->data, 0, map->slice_pitch * box->back); + return; + } + w = box->right - box->left; h = box->bottom - box->top; d = box->back - box->front; diff --git a/dlls/wined3d/surface.c b/dlls/wined3d/surface.c index 20a06b32d03..7d62af5b31f 100644 --- a/dlls/wined3d/surface.c +++ b/dlls/wined3d/surface.c @@ -1181,8 +1181,10 @@ static void surface_cpu_blt_colour_fill(struct wined3d_rendertarget_view *view, struct wined3d_context *context; struct wined3d_texture *texture; struct wined3d_bo_address data; + struct wined3d_box level_box; struct wined3d_map_desc map; struct wined3d_range range; + bool full_subresource; unsigned int level; DWORD map_binding;
@@ -1208,6 +1210,8 @@ static void surface_cpu_blt_colour_fill(struct wined3d_rendertarget_view *view,
texture = texture_from_resource(view->resource); level = view->sub_resource_idx % texture->level_count; + wined3d_texture_get_level_box(texture_from_resource(view->resource), level, &level_box); + full_subresource = !memcmp(box, &level_box, sizeof(*box));
map_binding = texture->resource.map_binding; if (!wined3d_texture_load_location(texture, view->sub_resource_idx, context, map_binding)) @@ -1220,7 +1224,7 @@ static void surface_cpu_blt_colour_fill(struct wined3d_rendertarget_view *view, range.offset = 0; range.size = texture->sub_resources[view->sub_resource_idx].size;
- wined3d_resource_memory_colour_fill(view->resource, &map, colour, box); + wined3d_resource_memory_colour_fill(view->resource, &map, colour, box, full_subresource);
wined3d_context_unmap_bo_address(context, &data, 1, &range); context_release(context); diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h index ce4d7384066..dfb2d76c08a 100644 --- a/dlls/wined3d/wined3d_private.h +++ b/dlls/wined3d/wined3d_private.h @@ -4429,7 +4429,7 @@ BOOL wined3d_resource_prepare_sysmem(struct wined3d_resource *resource) DECLSPEC void wined3d_resource_update_draw_binding(struct wined3d_resource *resource) DECLSPEC_HIDDEN; void wined3d_resource_memory_colour_fill(struct wined3d_resource *resource, const struct wined3d_map_desc *map, const struct wined3d_color *colour, - const struct wined3d_box *box) DECLSPEC_HIDDEN; + const struct wined3d_box *box, bool full_subresource) DECLSPEC_HIDDEN;
/* Tests show that the start address of resources is 32 byte aligned */ #define RESOURCE_ALIGNMENT 16
From: Stefan Dösinger stefan@codeweavers.com
Signed-off-by: Stefan Dösinger stefan@codeweavers.com
---
Version 2: Remove the FIXME from wined3d_resource_memory_colour_fill. If it accidentally gets called to fill a blocky format with a non-zero color, wined3d_format_convert_from_float will write a FIXME anyhow. --- dlls/wined3d/texture.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/dlls/wined3d/texture.c b/dlls/wined3d/texture.c index 99cb7101aed..6b80fed0c1b 100644 --- a/dlls/wined3d/texture.c +++ b/dlls/wined3d/texture.c @@ -815,7 +815,6 @@ BOOL wined3d_texture_load_location(struct wined3d_texture *texture, { struct wined3d_bo_address source, destination; struct wined3d_range range; - void *map_ptr;
if (!wined3d_texture_prepare_location(texture, sub_resource_idx, context, location)) return FALSE; @@ -824,12 +823,21 @@ BOOL wined3d_texture_load_location(struct wined3d_texture *texture, range.size = texture->sub_resources[sub_resource_idx].size; if (current & WINED3D_LOCATION_CLEARED) { + static const struct wined3d_color black; + unsigned int level_idx = sub_resource_idx % texture->level_count; + struct wined3d_map_desc map; + struct wined3d_box box; + + wined3d_texture_get_pitch(texture, level_idx, &map.row_pitch, &map.slice_pitch); if (destination.buffer_object) - map_ptr = wined3d_context_map_bo_address(context, &destination, range.size, + map.data = wined3d_context_map_bo_address(context, &destination, range.size, WINED3D_MAP_WRITE | WINED3D_MAP_DISCARD); else - map_ptr = destination.addr; - memset(map_ptr, 0, range.size); + map.data = destination.addr; + + wined3d_texture_get_level_box(texture, level_idx, &box); + wined3d_resource_memory_colour_fill(&texture->resource, &map, &black, &box, true); + if (destination.buffer_object) wined3d_context_unmap_bo_address(context, &destination, 1, &range); }
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=115526
Your paranoid android.
=== debian11 (build log) ===
../wine/dlls/wined3d/texture.c:839:13: error: too many arguments to function ‘wined3d_resource_memory_colour_fill’ Task: The win32 Wine build failed
=== debian11 (build log) ===
../wine/dlls/wined3d/texture.c:839:13: error: too many arguments to function ‘wined3d_resource_memory_colour_fill’ Task: The wow64 Wine build failed
Am Donnerstag, 26. Mai 2022, 00:55:56 EAT schrieb Marvin:
../wine/dlls/wined3d/texture.c:839:13: error: too many arguments to function ‘wined3d_resource_memory_colour_fill’ Task: The win32 Wine build failed
I have no idea what Marvin is smoking here. Did the testbot confused different versions of the merge request?
I can't reproduce it here, and the invocation and prototype look like they match too:
void wined3d_resource_memory_colour_fill(struct wined3d_resource *resource, const struct wined3d_map_desc *map, const struct wined3d_color *colour, const struct wined3d_box *box, bool full_subresource) DECLSPEC_HIDDEN;
wined3d_resource_memory_colour_fill(&texture->resource, &map, &black, &box, true);
From: Stefan Dösinger stefan@codeweavers.com
Signed-off-by: Stefan Dösinger stefan@codeweavers.com --- dlls/wined3d/utils.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/dlls/wined3d/utils.c b/dlls/wined3d/utils.c index 5c0f1c5e376..21b8d8a0919 100644 --- a/dlls/wined3d/utils.c +++ b/dlls/wined3d/utils.c @@ -6133,8 +6133,10 @@ void wined3d_format_convert_from_float(const struct wined3d_format *format, return; }
- /* 32 bit float formats. We don't handle D32_FLOAT and D32_FLOAT_S8X24_UINT for now. */ - if ((format->attrs & WINED3D_FORMAT_ATTR_FLOAT) && format->red_size == 32) + /* 32 bit float formats. We don't handle D32_FLOAT_S8X24_UINT for now. */ + if ((format->attrs & WINED3D_FORMAT_ATTR_FLOAT) + && (format->red_size == 32 || format->depth_size == 32) + && !format->stencil_size) { float *ret_f = ret;
From: Stefan Dösinger stefan@codeweavers.com
Signed-off-by: Stefan Dösinger stefan@codeweavers.com
---
Version 4: Be mindful about typeless formats. Version 3: Add an ERR check in the GL path. Version 2: Take the new wined3d_texture_vk_clear into account.
@Zeb: I can't write a FIXME in wined3d_buffer_load_location for non-zero colors because buffers don't (yet) store clear colors. The proper place to write a FIXME would be when delaying a buffer clear, but right now we'll never enter the clear code for buffers. (wined3d_device_clear is <= d3d9 only and wined3d_device_context_clear_rendertarget_view has a FIXME("Not implemented for buffer resources.\n"); --- dlls/wined3d/context_vk.c | 41 +++++++++++-- dlls/wined3d/texture.c | 103 ++++++++++++++++++++++----------- dlls/wined3d/utils.c | 19 ++++++ dlls/wined3d/wined3d_private.h | 11 ++++ 4 files changed, 136 insertions(+), 38 deletions(-)
diff --git a/dlls/wined3d/context_vk.c b/dlls/wined3d/context_vk.c index aa3b45d54fd..c257b5f3c86 100644 --- a/dlls/wined3d/context_vk.c +++ b/dlls/wined3d/context_vk.c @@ -2509,7 +2509,7 @@ static bool wined3d_context_vk_begin_render_pass(struct wined3d_context_vk *cont VkCommandBuffer vk_command_buffer, const struct wined3d_state *state, const struct wined3d_vk_info *vk_info) { struct wined3d_device_vk *device_vk = wined3d_device_vk(context_vk->c.device); - static const VkClearValue clear_values[WINED3D_MAX_RENDER_TARGETS + 1]; + VkClearValue clear_values[WINED3D_MAX_RENDER_TARGETS + 1]; VkImageView vk_views[WINED3D_MAX_RENDER_TARGETS + 1]; unsigned int fb_width, fb_height, fb_layer_count; struct wined3d_rendertarget_view_vk *rtv_vk; @@ -2518,6 +2518,7 @@ static bool wined3d_context_vk_begin_render_pass(struct wined3d_context_vk *cont struct wined3d_query_vk *query_vk; VkRenderPassBeginInfo begin_info; unsigned int attachment_count, i; + struct wined3d_texture *texture; VkFramebufferCreateInfo fb_desc; VkResult vr;
@@ -2549,10 +2550,25 @@ static bool wined3d_context_vk_begin_render_pass(struct wined3d_context_vk *cont if (view->layer_count < fb_layer_count) fb_layer_count = view->layer_count; context_vk->rt_count = i + 1; - ++attachment_count;
if (wined3d_rendertarget_view_get_locations(view) & WINED3D_LOCATION_CLEARED) - begin_info.clearValueCount = attachment_count; + { + VkClearColorValue *c = &clear_values[attachment_count].color; + + if (view->resource->type == WINED3D_RTYPE_BUFFER) + { + c->int32[0] = c->int32[1] = c->int32[2] = c->int32[3] = 0; + } + else + { + texture = texture_from_resource(view->resource); + wined3d_format_colour_to_vk(view->format, + &texture->sub_resources[view->sub_resource_idx].clear_value.colour, c); + } + + begin_info.clearValueCount = attachment_count + 1; + } + ++attachment_count; }
if ((view = state->fb.depth_stencil)) @@ -2568,10 +2584,25 @@ static bool wined3d_context_vk_begin_render_pass(struct wined3d_context_vk *cont fb_height = view->height; if (view->layer_count < fb_layer_count) fb_layer_count = view->layer_count; - ++attachment_count;
if (wined3d_rendertarget_view_get_locations(view) & WINED3D_LOCATION_CLEARED) - begin_info.clearValueCount = attachment_count; + { + VkClearDepthStencilValue *c = &clear_values[attachment_count].depthStencil; + + if (view->resource->type == WINED3D_RTYPE_BUFFER) + { + c->depth = 0.0f; + c->stencil = 0; + } + else + { + texture = texture_from_resource(view->resource); + c->depth = texture->sub_resources[view->sub_resource_idx].clear_value.depth; + c->stencil = texture->sub_resources[view->sub_resource_idx].clear_value.stencil; + } + begin_info.clearValueCount = attachment_count + 1; + } + ++attachment_count; }
if (!(context_vk->vk_render_pass = wined3d_context_vk_get_render_pass(context_vk, &state->fb, diff --git a/dlls/wined3d/texture.c b/dlls/wined3d/texture.c index 6b80fed0c1b..224252c7f73 100644 --- a/dlls/wined3d/texture.c +++ b/dlls/wined3d/texture.c @@ -823,10 +823,22 @@ BOOL wined3d_texture_load_location(struct wined3d_texture *texture, range.size = texture->sub_resources[sub_resource_idx].size; if (current & WINED3D_LOCATION_CLEARED) { - static const struct wined3d_color black; unsigned int level_idx = sub_resource_idx % texture->level_count; struct wined3d_map_desc map; struct wined3d_box box; + struct wined3d_color c; + + if (texture->resource.format->caps[WINED3D_GL_RES_TYPE_TEX_2D] + & WINED3D_FORMAT_CAP_DEPTH_STENCIL) + { + c.r = texture->sub_resources[sub_resource_idx].clear_value.depth; + c.g = texture->sub_resources[sub_resource_idx].clear_value.stencil; + c.b = c.a = 0.0f; + } + else + { + c = texture->sub_resources[sub_resource_idx].clear_value.colour; + }
wined3d_texture_get_pitch(texture, level_idx, &map.row_pitch, &map.slice_pitch); if (destination.buffer_object) @@ -836,7 +848,7 @@ BOOL wined3d_texture_load_location(struct wined3d_texture *texture, map.data = destination.addr;
wined3d_texture_get_level_box(texture, level_idx, &box); - wined3d_resource_memory_colour_fill(&texture->resource, &map, &black, &box, true); + wined3d_resource_memory_colour_fill(&texture->resource, &map, &c, &box, true);
if (destination.buffer_object) wined3d_context_unmap_bo_address(context, &destination, 1, &range); @@ -3369,6 +3381,27 @@ static bool wined3d_texture_gl_clear(struct wined3d_texture *texture, const struct wined3d_gl_info *gl_info = context_gl->gl_info; struct wined3d_bo_address addr;
+ /* The code that delays clears is Vulkan-specific, so here we should only + * encounter WINED3D_LOCATION_CLEARED on newly created resources and thus + * a zero clear value. */ + if (!format->depth_size && !format->stencil_size) + { + if (sub_resource->clear_value.colour.r || sub_resource->clear_value.colour.g + || sub_resource->clear_value.colour.b || sub_resource->clear_value.colour.a) + { + ERR("Unexpected color clear value r=%08e, g=%08e, b=%08e, a=%08e.\n", + sub_resource->clear_value.colour.r, sub_resource->clear_value.colour.g, + sub_resource->clear_value.colour.b, sub_resource->clear_value.colour.a); + } + } + else + { + if (format->depth_size && sub_resource->clear_value.depth) + ERR("Unexpected depth clear value %08e.\n", sub_resource->clear_value.depth); + if (format->stencil_size && sub_resource->clear_value.stencil) + ERR("Unexpected stencil clear value %x.\n", sub_resource->clear_value.stencil); + } + if (use_ffp_clear(texture, location)) { GLbitfield clear_mask = 0; @@ -5279,16 +5312,20 @@ static bool wined3d_texture_vk_clear(struct wined3d_texture_vk *texture_vk, struct wined3d_context_vk *context_vk = wined3d_context_vk(context); const struct wined3d_format *format = texture_vk->t.resource.format; const struct wined3d_vk_info *vk_info = context_vk->vk_info; - static const VkClearDepthStencilValue depth_value; - static const VkClearColorValue colour_value; + VkClearDepthStencilValue depth_value; VkCommandBuffer vk_command_buffer; VkImageSubresourceRange vk_range; + VkClearColorValue colour_value; VkImageAspectFlags aspect_mask; VkImage vk_image;
if (texture_vk->t.resource.format_attrs & WINED3D_FORMAT_ATTR_COMPRESSED) { struct wined3d_bo_address addr; + struct wined3d_color *c = &sub_resource->clear_value.colour; + + if (c->r || c->g || c-> b || c->a) + FIXME("Compressed resource %p is cleared to a non-zero color.\n", &texture_vk->t);
if (!wined3d_texture_prepare_location(&texture_vk->t, sub_resource_idx, context, WINED3D_LOCATION_SYSMEM)) return false; @@ -5322,11 +5359,18 @@ static bool wined3d_texture_vk_clear(struct wined3d_texture_vk *texture_vk, texture_vk->layout, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, vk_image, &vk_range);
if (format->depth_size || format->stencil_size) + { + depth_value.depth = sub_resource->clear_value.depth; + depth_value.stencil = sub_resource->clear_value.stencil; VK_CALL(vkCmdClearDepthStencilImage(vk_command_buffer, vk_image, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, &depth_value, 1, &vk_range)); + } else + { + wined3d_format_colour_to_vk(format, &sub_resource->clear_value.colour, &colour_value); VK_CALL(vkCmdClearColorImage(vk_command_buffer, vk_image, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, &colour_value, 1, &vk_range)); + }
wined3d_context_vk_image_barrier(context_vk, vk_command_buffer, VK_PIPELINE_STAGE_TRANSFER_BIT, VK_PIPELINE_STAGE_ALL_COMMANDS_BIT, @@ -6635,20 +6679,23 @@ static void vk_blitter_clear_rendertargets(struct wined3d_context_vk *context_vk if (!(view = fb->render_targets[i])) continue;
- if (is_full_clear(view, draw_rect, clear_rects)) + /* Don't delay typeless clears because the data written into the resource depends on the + * view format. Except all-zero clears, those should result in zeros in either case. + * + * We could store the clear format along with the clear value, but then we'd have to + * create a matching RTV at draw time, which would need its own render pass, thus mooting + * the point of the delayed clear. (Unless we are lucky enough that the application + * draws with the same RTV as it clears.) */ + if (is_full_clear(view, draw_rect, clear_rects) + && (!wined3d_format_is_typeless(view->resource->format) || (!colour->r && !colour->g + && !colour->b && !colour->a))) { - if (!colour->r && !colour->g && !colour->b && !colour->a) - { - wined3d_rendertarget_view_validate_location(view, WINED3D_LOCATION_CLEARED); - wined3d_rendertarget_view_invalidate_location(view, ~WINED3D_LOCATION_CLEARED); - delay_count++; - continue; - } - else - { - TRACE_(d3d_perf)("non-zero clear\n"); - wined3d_rendertarget_view_prepare_location(view, &context_vk->c, view->resource->draw_binding); - } + struct wined3d_texture *texture = texture_from_resource(view->resource); + wined3d_rendertarget_view_validate_location(view, WINED3D_LOCATION_CLEARED); + wined3d_rendertarget_view_invalidate_location(view, ~WINED3D_LOCATION_CLEARED); + texture->sub_resources[view->sub_resource_idx].clear_value.colour = *colour; + delay_count++; + continue; } else { @@ -6662,20 +6709,7 @@ static void vk_blitter_clear_rendertargets(struct wined3d_context_vk *context_vk wined3d_rendertarget_view_vk_barrier(rtv_vk, context_vk, WINED3D_BIND_RENDER_TARGET);
c = &clear_values[attachment_count].color; - if (view->format_attrs & WINED3D_FORMAT_ATTR_INTEGER) - { - c->int32[0] = colour->r; - c->int32[1] = colour->g; - c->int32[2] = colour->b; - c->int32[3] = colour->a; - } - else - { - c->float32[0] = colour->r; - c->float32[1] = colour->g; - c->float32[2] = colour->b; - c->float32[3] = colour->a; - } + wined3d_format_colour_to_vk(view->format, colour, c);
if (view->layer_count > layer_count) layer_count = view->layer_count; @@ -6699,8 +6733,8 @@ static void vk_blitter_clear_rendertargets(struct wined3d_context_vk *context_vk if (view->format->stencil_size) full_flags |= WINED3DCLEAR_STENCIL;
- if (!is_full_clear(view, draw_rect, clear_rects) - || depth || stencil || (flags & full_flags) != full_flags) + if (!is_full_clear(view, draw_rect, clear_rects) || (flags & full_flags) != full_flags + || (wined3d_format_is_typeless(view->resource->format) && (depth || stencil))) { wined3d_rendertarget_view_load_location(view, &context_vk->c, view->resource->draw_binding); wined3d_rendertarget_view_validate_location(view, view->resource->draw_binding); @@ -6721,6 +6755,9 @@ static void vk_blitter_clear_rendertargets(struct wined3d_context_vk *context_vk } else { + struct wined3d_texture *texture = texture_from_resource(view->resource); + texture->sub_resources[view->sub_resource_idx].clear_value.depth = depth; + texture->sub_resources[view->sub_resource_idx].clear_value.stencil = stencil; wined3d_rendertarget_view_validate_location(view, WINED3D_LOCATION_CLEARED); wined3d_rendertarget_view_invalidate_location(view, ~WINED3D_LOCATION_CLEARED); flags &= ~(WINED3DCLEAR_ZBUFFER | WINED3DCLEAR_STENCIL); diff --git a/dlls/wined3d/utils.c b/dlls/wined3d/utils.c index 21b8d8a0919..381c1ae7bc6 100644 --- a/dlls/wined3d/utils.c +++ b/dlls/wined3d/utils.c @@ -6009,6 +6009,25 @@ uint32_t wined3d_format_pack(const struct wined3d_format *format, const struct w return p; }
+void wined3d_format_colour_to_vk(const struct wined3d_format *format, const struct wined3d_color *c, + VkClearColorValue *retval) +{ + if (format->attrs & WINED3D_FORMAT_ATTR_INTEGER) + { + retval->int32[0] = c->r; + retval->int32[1] = c->g; + retval->int32[2] = c->b; + retval->int32[3] = c->a; + } + else + { + retval->float32[0] = c->r; + retval->float32[1] = c->g; + retval->float32[2] = c->b; + retval->float32[3] = c->a; + } +} + /* Note: It's the caller's responsibility to ensure values can be expressed * in the requested format. UNORM formats for example can only express values * in the range 0.0f -> 1.0f. diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h index dfb2d76c08a..1e3888cd30d 100644 --- a/dlls/wined3d/wined3d_private.h +++ b/dlls/wined3d/wined3d_private.h @@ -4560,6 +4560,15 @@ struct wined3d_texture uint32_t map_flags; DWORD locations; struct wined3d_bo *bo; + union + { + struct wined3d_color colour; + struct + { + float depth; + unsigned int stencil; + }; + } clear_value;
void *user_memory; } *sub_resources; @@ -6218,6 +6227,8 @@ void wined3d_format_calculate_pitch(const struct wined3d_format *format, unsigne unsigned int width, unsigned int height, unsigned int *row_pitch, unsigned int *slice_pitch) DECLSPEC_HIDDEN; UINT wined3d_format_calculate_size(const struct wined3d_format *format, UINT alignment, UINT width, UINT height, UINT depth) DECLSPEC_HIDDEN; +void wined3d_format_colour_to_vk(const struct wined3d_format *format, const struct wined3d_color *c, + VkClearColorValue *retval) DECLSPEC_HIDDEN; void wined3d_format_convert_from_float(const struct wined3d_format *format, const struct wined3d_color *color, void *ret) DECLSPEC_HIDDEN; void wined3d_format_copy_data(const struct wined3d_format *format, const uint8_t *src,
From: Stefan Dösinger stefan@codeweavers.com
So it covers more formats and provokes the clear-to-sysmem path with the Vulkan renderer.
Signed-off-by: Stefan Dösinger stefan@codeweavers.com
---
Yes, the d3d9 support in the Vulkan renderer is good enough for this test to run and produce useful results for map-after-clear. --- dlls/d3d9/tests/visual.c | 132 +++++++++++++++++++++++++++++++-------- 1 file changed, 106 insertions(+), 26 deletions(-)
diff --git a/dlls/d3d9/tests/visual.c b/dlls/d3d9/tests/visual.c index 2913583916a..5d8fe10fc79 100644 --- a/dlls/d3d9/tests/visual.c +++ b/dlls/d3d9/tests/visual.c @@ -1548,10 +1548,10 @@ done:
static void color_fill_test(void) { + unsigned int fill_a, expected_a; IDirect3DSurface9 *surface; IDirect3DTexture9 *texture; D3DCOLOR fill_color, color; - DWORD fill_a, expected_a; IDirect3DDevice9 *device; IDirect3D9 *d3d; ULONG refcount; @@ -1580,20 +1580,39 @@ static void color_fill_test(void) { CHECK_FILL_VALUE = 0x1, BLOCKS = 0x2, + FLOAT_VALUES = 0x4, } flags; - DWORD fill_value; + unsigned int fill_i[4]; + float fill_f[4]; } formats[] = { - {D3DFMT_A8R8G8B8, "D3DFMT_A8R8G8B8", CHECK_FILL_VALUE, 0xdeadbeef}, + {D3DFMT_A8R8G8B8, "D3DFMT_A8R8G8B8", CHECK_FILL_VALUE, + {0xdeadbeef, 0xdeadbeef, 0xdeadbeef, 0xdeadbeef}}, /* D3DFMT_X8R8G8B8 either set X = A or X = 0, depending on the driver. */ - {D3DFMT_R5G6B5, "D3DFMT_R5G6B5", CHECK_FILL_VALUE, 0xadfdadfd}, - {D3DFMT_G16R16, "D3DFMT_G16R16", CHECK_FILL_VALUE, 0xbebeadad}, + {D3DFMT_R5G6B5, "D3DFMT_R5G6B5", CHECK_FILL_VALUE, + {0xadfdadfd, 0xadfdadfd, 0xadfdadfd, 0xadfdadfd}}, + {D3DFMT_G16R16, "D3DFMT_G16R16", CHECK_FILL_VALUE, + {0xbebeadad, 0xbebeadad, 0xbebeadad, 0xbebeadad}}, + {D3DFMT_A16B16G16R16, "D3DFMT_A16B16G16R16", CHECK_FILL_VALUE, + {0xbebeadad, 0xdedeefef, 0xbebeadad, 0xdedeefef}}, /* Real hardware reliably fills the surface with the blue channel but * the testbot fills it with 0x00. Wine incorrectly uses the alpha * channel. Don't bother checking the result because P8 surfaces are * essentially useless in d3d9. */ - {D3DFMT_P8, "D3DFMT_P8", 0, 0xefefefef}, + {D3DFMT_P8, "D3DFMT_P8", 0, + {0xefefefef, 0xefefefef, 0xefefefef, 0xefefefef}}, + /* Float formats. */ + {D3DFMT_R32F, "D3DFMT_R32F", CHECK_FILL_VALUE | FLOAT_VALUES, + {0, 0, 0, 0}, {0xad / 255.0f, 0xad / 255.0f, 0xad / 255.0f, 0xad / 255.0f}}, + {D3DFMT_A32B32G32R32F, "D3DFMT_A32B32G32R32F", CHECK_FILL_VALUE | FLOAT_VALUES, + {0, 0, 0, 0}, {0xad / 255.0f, 0xbe / 255.0f, 0xef / 255.0f, 0xde / 255.0f}}, + {D3DFMT_R16F, "D3DFMT_R16F", CHECK_FILL_VALUE, + {0x396d396d, 0x396d396d, 0x396d396d, 0x396d396d}}, + {D3DFMT_G16R16F, "D3DFMT_G16R16F", CHECK_FILL_VALUE, + {0x39f5396d, 0x39f5396d, 0x39f5396d, 0x39f5396d}}, + {D3DFMT_A16B16G16R16F, "D3DFMT_A16B16G16R16F", CHECK_FILL_VALUE, + {0x39f5396d, 0x3af63b7f, 0x39f5396d, 0x3af63b7f}}, /* Windows drivers produce different results for these formats. * No driver produces a YUV value that matches the input RGB * value, and no driver produces a proper DXT compression block. @@ -1603,17 +1622,16 @@ static void color_fill_test(void) * * The YUV tests are disabled because they produce a driver-dependent * result on Wine. - * {D3DFMT_YUY2, "D3DFMT_YUY2", BLOCKS, 0}, - * {D3DFMT_UYVY, "D3DFMT_UYVY", BLOCKS, 0}, */ - {D3DFMT_DXT1, "D3DFMT_DXT1", BLOCKS, 0x00000000}, + * {D3DFMT_YUY2, "D3DFMT_YUY2", BLOCKS}, + * {D3DFMT_UYVY, "D3DFMT_UYVY", BLOCKS}, */ + {D3DFMT_DXT1, "D3DFMT_DXT1", BLOCKS}, /* Vendor-specific formats like ATI2N are a non-issue here since they're not * supported as offscreen plain surfaces and do not support D3DUSAGE_RENDERTARGET * when created as texture. */ }; - unsigned int i; - D3DLOCKED_RECT locked_rect; - DWORD *surface_data; static const RECT rect = {4, 4, 8, 8}, rect2 = {5, 5, 7, 7}; + D3DLOCKED_RECT locked_rect; + unsigned int i, j;
window = create_window(); d3d = Direct3DCreate9(D3D_SDK_VERSION); @@ -1732,22 +1750,84 @@ static void color_fill_test(void) ok(SUCCEEDED(hr), "Failed to color fill, hr %#x, fmt=%s.\n", hr, formats[i].name); }
- if (formats[i].flags & CHECK_FILL_VALUE) + if (!(formats[i].flags & CHECK_FILL_VALUE)) { - hr = IDirect3DSurface9_LockRect(surface, &locked_rect, NULL, D3DLOCK_READONLY); - ok(SUCCEEDED(hr), "Failed to lock surface, hr %#x, fmt=%s.\n", hr, formats[i].name); - surface_data = locked_rect.pBits; - fill_a = (surface_data[0] & 0xff000000) >> 24; - expected_a = (formats[i].fill_value & 0xff000000) >> 24; - /* Windows drivers disagree on how to promote the 8 bit per channel - * input argument to 16 bit for D3DFMT_G16R16. */ - ok(color_match(surface_data[0], formats[i].fill_value, 2) && - compare_uint(expected_a, fill_a, 2), - "Expected clear value 0x%08x, got 0x%08x, fmt=%s.\n", - formats[i].fill_value, surface_data[0], formats[i].name); - hr = IDirect3DSurface9_UnlockRect(surface); - ok(SUCCEEDED(hr), "Failed to unlock surface, hr %#x, fmt=%s.\n", hr, formats[i].name); + IDirect3DSurface9_Release(surface); + continue; + } + + hr = IDirect3DSurface9_LockRect(surface, &locked_rect, NULL, 0); + ok(SUCCEEDED(hr), "Failed to lock surface, hr %#x, fmt=%s.\n", hr, formats[i].name); + /* Windows drivers disagree on how to promote the 8 bit per channel + * input argument to 16 bit for D3DFMT_G16R16. */ + if (formats[i].flags & FLOAT_VALUES) + { + const struct vec4 *surface_data = locked_rect.pBits; + ok(compare_vec4(surface_data, formats[i].fill_f[0], formats[i].fill_f[1], + formats[i].fill_f[2], formats[i].fill_f[3], 1), + "Expected clear values %f %f %f %f, got %f %f %f %f, fmt=%s\n", + formats[i].fill_f[0], formats[i].fill_f[1], + formats[i].fill_f[2], formats[i].fill_f[3], + surface_data->x, surface_data->y, surface_data->z, surface_data->w, + formats[i].name); } + else + { + const unsigned int *surface_data = locked_rect.pBits; + for (j = 0; j < 4; ++j) + { + fill_a = (surface_data[j] & 0xff000000) >> 24; + expected_a = (formats[i].fill_i[j] & 0xff000000) >> 24; + ok(color_match(surface_data[j], formats[i].fill_i[j], 2) && + compare_uint(expected_a, fill_a, 2), + "Expected clear value 0x%08x, got 0x%08x, fmt=%s, j=%u.\n", + formats[i].fill_i[j], surface_data[j], formats[i].name, j); + } + } + + /* Fill the surface with something else to make sure the test below doesn't pass + * due to stale contents by accident. */ + memset(locked_rect.pBits, 0x55, locked_rect.Pitch * 32); + + hr = IDirect3DSurface9_UnlockRect(surface); + ok(SUCCEEDED(hr), "Failed to unlock surface, hr %#x, fmt=%s.\n", hr, formats[i].name); + + /* Test clearing "to sysmem". Wined3d's delayed clear will perform the actual clear + * in the lock call and try to fill the sysmem buffer instead of clearing on the + * GPU and downloading it. */ + hr = IDirect3DDevice9_ColorFill(device, surface, NULL, 0xdeadbeef); + ok(SUCCEEDED(hr), "Failed to color fill, hr %#x, fmt=%s.\n", hr, formats[i].name); + hr = IDirect3DSurface9_LockRect(surface, &locked_rect, NULL, D3DLOCK_READONLY); + ok(SUCCEEDED(hr), "Failed to lock surface, hr %#x, fmt=%s.\n", hr, formats[i].name); + + if (formats[i].flags & FLOAT_VALUES) + { + const struct vec4 *surface_data = locked_rect.pBits; + ok(compare_vec4(surface_data, formats[i].fill_f[0], formats[i].fill_f[1], + formats[i].fill_f[2], formats[i].fill_f[3], 1), + "Expected clear values %f %f %f %f, got %f %f %f %f, fmt=%s\n", + formats[i].fill_f[0], formats[i].fill_f[1], + formats[i].fill_f[2], formats[i].fill_f[3], + surface_data->x, surface_data->y, surface_data->z, surface_data->w, + formats[i].name); + } + else + { + const unsigned int *surface_data = locked_rect.pBits; + for (j = 0; j < 4; ++j) + { + fill_a = (surface_data[j] & 0xff000000) >> 24; + expected_a = (formats[i].fill_i[j] & 0xff000000) >> 24; + ok(color_match(surface_data[j], formats[i].fill_i[j], 2) && + compare_uint(expected_a, fill_a, 2), + "Expected clear value 0x%08x, got 0x%08x, fmt=%s, j=%u.\n", + formats[i].fill_i[j], surface_data[j], formats[i].name, j); + } + } + + + hr = IDirect3DSurface9_UnlockRect(surface); + ok(SUCCEEDED(hr), "Failed to unlock surface, hr %#x, fmt=%s.\n", hr, formats[i].name);
IDirect3DSurface9_Release(surface); }
On Wed May 25 14:21:30 2022 +0000, Jan Sikorski wrote:
I ran the d3d11 tests and on my machine this breaks a few of them slightly, for example, in test_clear_render_target_view_2d() I get 0xbf4d801a instead of 0xbf4c7f19 in line 15945. Not sure how big of an issue that could be. Some others look more badly broken, like test_clear_depth_stencil_view() lines 16116 and later, got 0.0. `02f0:fixme:d3d:wined3d_format_convert_from_float Conversion for format WINED3DFMT_D32_FLOAT not implemented.` might be relevant. Let me know if you can reproduce that, if not, I'll take a closer look.
Jan, my new version should address those two issues. It adds support for D32FLOAT and is more cautious about typeless formats. The test_clear_render_target_view_2d failure was about sRGB vs RGB views on a typeless resource.
I have a pile of other test failures when running with Vulkan and some of them are random. Afaics there are no further new failures introduced by the new patchset, but can you double-check please?
On Wed May 25 14:21:30 2022 +0000, Stefan Dösinger wrote:
Jan, my new version should address those two issues. It adds support for D32FLOAT and is more cautious about typeless formats. The test_clear_render_target_view_2d failure was about sRGB vs RGB views on a typeless resource. I have a pile of other test failures when running with Vulkan and some of them are random. Afaics there are no further new failures introduced by the new patchset, but can you double-check please?
Actually I still get a couple new ones:
d3d11.c:5963: Test failed: Got unexpected IAVertices count: 7. d3d11.c:5964: Test failed: Got unexpected IAPrimitives count: 3. d3d11.c:5965: Test failed: Got unexpected VSInvocations count: 7. d3d11.c:5966: Test failed: Got unexpected GSInvocations count: 3. d3d11.c:5968: Test failed: Got unexpected CInvocations count: 3. d3d11.c:5969: Test failed: Got unexpected CPrimitives count: 3.
I missed it the last time because I think they used to be flaky, but now that I ran it a few times they seem robust. It might be by chance, it doesn't make much sense to me that your changes would report extra GS invocations?
On Thu May 26 07:54:22 2022 +0000, Jan Sikorski wrote:
Actually I still get a couple new ones: d3d11.c:5963: Test failed: Got unexpected IAVertices count: 7. d3d11.c:5964: Test failed: Got unexpected IAPrimitives count: 3. d3d11.c:5965: Test failed: Got unexpected VSInvocations count: 7. d3d11.c:5966: Test failed: Got unexpected GSInvocations count: 3. d3d11.c:5968: Test failed: Got unexpected CInvocations count: 3. d3d11.c:5969: Test failed: Got unexpected CPrimitives count: 3. I missed it the last time because I think they used to be flaky, but now that I ran it a few times they seem robust. It might be by chance, it doesn't make much sense to me that your changes would report extra GS invocations?
I get those failures all the time, even with GL.
This merge request was approved by Jan Sikorski.
This merge request was approved by Zebediah Figura.
-
Jan Sikorski (＠jsikorski) -
Marvin -
Stefan Dösinger
-
Stefan Dösinger -
Stefan Dösinger (＠stefan)
-
Zebediah Figura (＠zfigura)