Am 2017-05-24 um 15:49 schrieb Masanori Kakura:
for (j = 0; j < layer_count; ++j){wined3d_cs_emit_blt_sub_resource(cs,&dst_texture->resource, j * dst_level_count + i, &box,&src_texture->resource, j * src_level_count + i + src_skip_levels, &box,0, NULL, WINED3D_TEXF_POINT);}
I don't like emitting new operations in the CS handler instead of doing the actual operation. Maybe this can be avoided with some refactoring of wined3d_cs_exec_blt_sub_resource?
On 24 May 2017 at 16:08, Stefan Dösinger stefandoesinger@gmail.com wrote:
Am 2017-05-24 um 15:49 schrieb Masanori Kakura:
for (j = 0; j < layer_count; ++j){wined3d_cs_emit_blt_sub_resource(cs,&dst_texture->resource, j * dst_level_count + i, &box,&src_texture->resource, j * src_level_count + i + src_skip_levels, &box,0, NULL, WINED3D_TEXF_POINT);}I don't like emitting new operations in the CS handler instead of doing the actual operation. Maybe this can be avoided with some refactoring of wined3d_cs_exec_blt_sub_resource?
Yeah, that's a consideration, although it probably shouldn't be the focus right now.
On Wed, 24 May 2017 16:08:30 +0200 Stefan Dösinger stefandoesinger@gmail.com wrote:
Am 2017-05-24 um 15:49 schrieb Masanori Kakura:
for (j = 0; j < layer_count; ++j){wined3d_cs_emit_blt_sub_resource(cs,&dst_texture->resource, j * dst_level_count + i, &box,&src_texture->resource, j * src_level_count + i + src_skip_levels, &box,0, NULL, WINED3D_TEXF_POINT);}I don't like emitting new operations in the CS handler instead of doing the actual operation. Maybe this can be avoided with some refactoring of wined3d_cs_exec_blt_sub_resource?
I wrote 2 new patches but I'm not sure which is better.
----- Patch 1 (modify wined3d_cs_exec_blt_sub_resource) ----- static void wined3d_cs_exec_blt_sub_resource(struct wined3d_cs *cs, const void *data) { /* ... */
error: if (op->opcode == WINED3D_CS_OP_BLT_SUB_RESOURCE) { if (op->src_resource) wined3d_resource_release(op->src_resource); wined3d_resource_release(op->dst_resource); } }
static void wined3d_cs_exec_update_texture(struct wined3d_cs *cs, const void *data) { /* ... */
for (i = 0; i < level_count; ++i) { width = wined3d_texture_get_level_width(dst_texture, i); height = wined3d_texture_get_level_height(dst_texture, i); depth = wined3d_texture_get_level_depth(dst_texture, i); wined3d_box_set(&box, 0, 0, width, height, 0, depth);
for (j = 0; j < layer_count; ++j) { const struct wined3d_cs_blt_sub_resource op_blt = {WINED3D_CS_OP_UPDATE_TEXTURE, &dst_texture->resource, j * dst_level_count + i, box, &src_texture->resource, j * src_level_count + i + src_skip_levels, box, 0, NULL, WINED3D_TEXF_POINT};
wined3d_cs_exec_blt_sub_resource(cs, &op_blt); } }
wined3d_resource_release(&src_texture->resource); wined3d_resource_release(&dst_texture->resource); }
----- Patch 2 (write codes directly) ----- static void wined3d_cs_exec_update_texture(struct wined3d_cs *cs, const void *data) { /* ... */
/* Update every surface level of the texture. */ for (i = 0; i < level_count; ++i) { width = wined3d_texture_get_level_width(dst_texture, i); height = wined3d_texture_get_level_height(dst_texture, i); depth = wined3d_texture_get_level_depth(dst_texture, i); wined3d_box_set(&box, 0, 0, width, height, 0, depth);
for (j = 0; j < layer_count; ++j) { struct wined3d_resource *dst_resource = &dst_texture->resource; unsigned int dst_sub_resource_idx = j * dst_level_count + i; unsigned int src_sub_resource_idx = j * src_level_count + i + src_skip_levels;
if (dst_resource->type == WINED3D_RTYPE_TEXTURE_2D) { struct wined3d_surface *dst_surface, *src_surface;
dst_surface = dst_texture->sub_resources[dst_sub_resource_idx].u.surface; src_surface = src_texture->sub_resources[src_sub_resource_idx].u.surface;
if (FAILED(wined3d_surface_blt(dst_surface, (const RECT *)&box, src_surface, (const RECT *)&box, 0, NULL, WINED3D_TEXF_POINT))) FIXME("Blit failed.\n"); } else if (dst_resource->type == WINED3D_RTYPE_TEXTURE_3D) { unsigned int update_w, update_h, update_d; unsigned int row_pitch, slice_pitch; struct wined3d_context *context; struct wined3d_bo_address addr;
update_w = box.right - box.left; update_h = box.bottom - box.top; update_d = box.back - box.front;
if (box.left || box.top || box.front) { FIXME("Source box %s not supported for %s resources.\n", debug_box(&box), debug_d3dresourcetype(dst_resource->type)); continue; }
context = context_acquire(cs->device, NULL, 0);
if (!wined3d_texture_load_location(src_texture, src_sub_resource_idx, context, src_texture->resource.map_binding)) { ERR("Failed to load source sub-resource into %s.\n", wined3d_debug_location(src_texture->resource.map_binding)); context_release(context); continue; }
if (update_w == width && update_h == height && update_d == depth) { wined3d_texture_prepare_texture(dst_texture, context, FALSE); } else if (!wined3d_texture_load_location(dst_texture, dst_sub_resource_idx, context, WINED3D_LOCATION_TEXTURE_RGB)) { ERR("Failed to load destination sub-resource.\n"); context_release(context); continue; }
wined3d_texture_get_memory(src_texture, src_sub_resource_idx, &addr, src_texture->resource.map_binding); wined3d_texture_get_pitch(src_texture, src_sub_resource_idx % src_texture->level_count, &row_pitch, &slice_pitch);
wined3d_texture_bind_and_dirtify(dst_texture, context, FALSE); wined3d_texture_upload_data(dst_texture, dst_sub_resource_idx, context, &box, wined3d_const_bo_address(&addr), row_pitch, slice_pitch); wined3d_texture_validate_location(dst_texture, dst_sub_resource_idx, WINED3D_LOCATION_TEXTURE_RGB); wined3d_texture_invalidate_location(dst_texture, dst_sub_resource_idx, ~WINED3D_LOCATION_TEXTURE_RGB);
context_release(context); } else { FIXME("Not implemented for %s resources.\n", debug_d3dresourcetype(dst_resource->type)); } } }
wined3d_resource_release(&src_texture->resource); wined3d_resource_release(&dst_texture->resource); }
Hi Masanori,
I am sorry, I forgot to reply until I saw your patch resend this morning.
Henri's reply (https://www.winehq.org/pipermail/wine-patches/2017-May/162123.html) suggests that he doesn't see the command emitting as the pressing issue in this patch series. So your original approach is fine for now and we can adjust this specific aspect later.
Since he didn't sign off on the patches I assume there is something else he doesn't like. I guess it is still the lack of integration with location tracking (https://www.winehq.org/pipermail/wine-devel/2017-April/117453.html)
Regarding the options with blit emitting (and again, it's not important right now), I was thinking about something like this:
/* FIXME: Better name */ static void wined3d_cs_blt_helper(dst_resource, src_resource, ...) { /* Most code from wined3d_cs_exec_blt_sub_resource goes here */ /* but not wined3d_resource_release() calls */ }
static void wined3d_cs_exec_blt_sub_resource(struct wined3d_cs *cs, const void *data) { const struct wined3d_cs_blt_sub_resource *op = data;
wined3d_cs_blt_helper(op->src, op->dst, ...);
if (op->src_resource) wined3d_resource_release(op->src_resource); wined3d_resource_release(op->dst_resource); }
static void wined3d_cs_exec_update_texture(struct wined3d_cs *cs, const void *data) { /* This looks very similar to your code from may 25, except that it calls wined3d_cs_blt_helper instead of emit_blt_sub_resource. */ wined3d_resource_release(src); wined3d_resource_release(dst); }
/* Of course you still need the individual wined3d_cs_emit() calls */
Am 2017-05-25 um 16:19 schrieb Masanori Kakura:
On Wed, 24 May 2017 16:08:30 +0200 Stefan Dösinger stefandoesinger@gmail.com wrote:
Am 2017-05-24 um 15:49 schrieb Masanori Kakura:
for (j = 0; j < layer_count; ++j){wined3d_cs_emit_blt_sub_resource(cs,&dst_texture->resource, j * dst_level_count + i, &box,&src_texture->resource, j * src_level_count + i + src_skip_levels, &box,0, NULL, WINED3D_TEXF_POINT);}I don't like emitting new operations in the CS handler instead of doing the actual operation. Maybe this can be avoided with some refactoring of wined3d_cs_exec_blt_sub_resource?
I wrote 2 new patches but I'm not sure which is better.
----- Patch 1 (modify wined3d_cs_exec_blt_sub_resource) ----- static void wined3d_cs_exec_blt_sub_resource(struct wined3d_cs *cs, const void *data) { /* ... */
error: if (op->opcode == WINED3D_CS_OP_BLT_SUB_RESOURCE) { if (op->src_resource) wined3d_resource_release(op->src_resource); wined3d_resource_release(op->dst_resource); } }
static void wined3d_cs_exec_update_texture(struct wined3d_cs *cs, const void *data) { /* ... */
for (i = 0; i < level_count; ++i) { width = wined3d_texture_get_level_width(dst_texture, i); height = wined3d_texture_get_level_height(dst_texture, i); depth = wined3d_texture_get_level_depth(dst_texture, i); wined3d_box_set(&box, 0, 0, width, height, 0, depth); for (j = 0; j < layer_count; ++j) { const struct wined3d_cs_blt_sub_resource op_blt = {WINED3D_CS_OP_UPDATE_TEXTURE, &dst_texture->resource, j * dst_level_count + i, box, &src_texture->resource, j * src_level_count + i + src_skip_levels, box, 0, NULL, WINED3D_TEXF_POINT}; wined3d_cs_exec_blt_sub_resource(cs, &op_blt); } } wined3d_resource_release(&src_texture->resource); wined3d_resource_release(&dst_texture->resource);}
----- Patch 2 (write codes directly) ----- static void wined3d_cs_exec_update_texture(struct wined3d_cs *cs, const void *data) { /* ... */
/* Update every surface level of the texture. */ for (i = 0; i < level_count; ++i) { width = wined3d_texture_get_level_width(dst_texture, i); height = wined3d_texture_get_level_height(dst_texture, i); depth = wined3d_texture_get_level_depth(dst_texture, i); wined3d_box_set(&box, 0, 0, width, height, 0, depth); for (j = 0; j < layer_count; ++j) { struct wined3d_resource *dst_resource = &dst_texture->resource; unsigned int dst_sub_resource_idx = j * dst_level_count + i; unsigned int src_sub_resource_idx = j * src_level_count + i + src_skip_levels; if (dst_resource->type == WINED3D_RTYPE_TEXTURE_2D) { struct wined3d_surface *dst_surface, *src_surface; dst_surface = dst_texture->sub_resources[dst_sub_resource_idx].u.surface; src_surface = src_texture->sub_resources[src_sub_resource_idx].u.surface; if (FAILED(wined3d_surface_blt(dst_surface, (const RECT *)&box, src_surface, (const RECT *)&box, 0, NULL, WINED3D_TEXF_POINT))) FIXME("Blit failed.\n"); } else if (dst_resource->type == WINED3D_RTYPE_TEXTURE_3D) { unsigned int update_w, update_h, update_d; unsigned int row_pitch, slice_pitch; struct wined3d_context *context; struct wined3d_bo_address addr; update_w = box.right - box.left; update_h = box.bottom - box.top; update_d = box.back - box.front; if (box.left || box.top || box.front) { FIXME("Source box %s not supported for %s resources.\n", debug_box(&box), debug_d3dresourcetype(dst_resource->type)); continue; } context = context_acquire(cs->device, NULL, 0); if (!wined3d_texture_load_location(src_texture, src_sub_resource_idx, context, src_texture->resource.map_binding)) { ERR("Failed to load source sub-resource into %s.\n", wined3d_debug_location(src_texture->resource.map_binding)); context_release(context); continue; } if (update_w == width && update_h == height && update_d == depth) { wined3d_texture_prepare_texture(dst_texture, context, FALSE); } else if (!wined3d_texture_load_location(dst_texture, dst_sub_resource_idx, context, WINED3D_LOCATION_TEXTURE_RGB)) { ERR("Failed to load destination sub-resource.\n"); context_release(context); continue; } wined3d_texture_get_memory(src_texture, src_sub_resource_idx, &addr, src_texture->resource.map_binding); wined3d_texture_get_pitch(src_texture, src_sub_resource_idx % src_texture->level_count, &row_pitch, &slice_pitch); wined3d_texture_bind_and_dirtify(dst_texture, context, FALSE); wined3d_texture_upload_data(dst_texture, dst_sub_resource_idx, context, &box, wined3d_const_bo_address(&addr), row_pitch, slice_pitch); wined3d_texture_validate_location(dst_texture, dst_sub_resource_idx, WINED3D_LOCATION_TEXTURE_RGB); wined3d_texture_invalidate_location(dst_texture, dst_sub_resource_idx, ~WINED3D_LOCATION_TEXTURE_RGB); context_release(context); } else { FIXME("Not implemented for %s resources.\n", debug_d3dresourcetype(dst_resource->type)); } } } wined3d_resource_release(&src_texture->resource); wined3d_resource_release(&dst_texture->resource);}
-
Henri Verbeet -
Masanori Kakura -
Stefan Dösinger