Hi Masanori, Thank you for your patches! I have a few comments: Regarding the big picture I think it would be great if we had a scheme that also worked for partially mapping resources. It's also necessary to make the D3DLOCK_NO_DIRTY_UPDATE test on managed textures work. However, doing this is a lot more work than your patchset and complicates location management a lot. If Henri tells you something different follow his advice. He is the authority for decisions in d3d :-) . Patch 1: Am 2017-04-17 um 15:47 schrieb Masanori Kakura:
+struct wined3d_dirty_regions +{ + UINT left; + UINT top; + UINT right; + UINT bottom; + UINT front; + UINT back; + struct wined3d_dirty_regions *next; +}; A standard wine list would be better IMHO.
Patch 2:
diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h index 785105d7b9..711f98a0a6 100644 --- a/dlls/wined3d/wined3d_private.h +++ b/dlls/wined3d/wined3d_private.h @@ -2765,6 +2765,27 @@ struct wined3d_dirty_regions struct wined3d_dirty_regions *next; };
+static inline BOOL wined3d_dirty_regions_add(struct wined3d_dirty_regions **regions, + UINT left, UINT top, UINT right, UINT bottom, UINT front, UINT back) I don't think we want this in the header file, I think it'd be better placed in resource.c. Also note that buffers already have a related mechanism (buffer->modified_areas).
You can predeclare the functions in the header if you need to call them from multiple files.
+{ + struct wined3d_dirty_regions *new_region; + + if (!(new_region = HeapAlloc(GetProcessHeap(), 0, sizeof(struct wined3d_dirty_regions)))) + return FALSE; HeapAlloc is expensive. I think it is OK for the start, but in the long run a way to keep some old dirty region elements around and reuse them instead of always calling Alloc / Free.
Patch 5:
+ if (!wined3d_dirty_regions_add(&resource->dirty_regions, 0, 0, width, height, 0, depth)) Why not use wined3d_resource_maximize_dirty_region here?
wined3d_resource_acquire(resource); + wined3d_dirty_regions_clear(&resource->dirty_regions); wined3d_cs_destroy_object(resource->device->cs, wined3d_resource_destroy_object, resource);
Regions need to be cleared from inside the cs thread in case there are pending update_texture commands. Similarly patch 6 needs to put the region modifying into wined3d_cs_exec_add_dirty_texture_region. Patch 7:
+ if (box->left < box->right && box->top < box->bottom && box->front < box->back) + { You don't need to check the box, texture_resource_sub_resource_map already does it before it reaches this place of the code. See wined3d_texture_check_box_dimensions.
Patch 8: I am not sure if this is necessary at all. In d3d9 it doesn't matter because a UpdateSurface destination can never be an UpdateTexture source. Patch 9/10: These should be merged, and I don't think patch 10 looks right. I think dirty regions should be honored on sublevels as well, just adjusted (i.e., divide by 2 for every level).
+ if (src_texture->resource.width <= dst_texture->resource.width + && src_texture->resource.height <= dst_texture->resource.height + && src_texture->resource.depth <= dst_texture->resource.depth) Why are you adding this check? Is it because the src_skip_levels broke the if (i == 0) root level detection? If you apply the dirty region to all levels this problem should go away.