st/nine: Rework UpdateTexture Checks

Our code did match the user documentation of the function
quite well (except for format check).

However the DDI documentation and wine tests show that
documentation was not correct. Thus adapt our code to
fit the best possible to the -real- spec.

Signed-off-by: Axel Davy <axel.davy@ens.fr>

diff --git a/src/gallium/state_trackers/nine/device9.c b/src/gallium/state_trackers/nine/device9.c
index 61a4a0c2b9f6b29d7947b4ec630a21d23a7b36dc..3e00ceed4243ba7cdd5819b02fd0975f08ee28f3 100644 (file)
--- a/src/gallium/state_trackers/nine/device9.c
+++ b/src/gallium/state_trackers/nine/device9.c
@@ -1318,51 +1318,61 @@ NineDevice9_UpdateTexture( struct NineDevice9 *This,
     struct NineBaseTexture9 *dstb = NineBaseTexture9(pDestinationTexture);
     struct NineBaseTexture9 *srcb = NineBaseTexture9(pSourceTexture);
     unsigned l, m;
-    unsigned last_level = dstb->base.info.last_level;
+    unsigned last_src_level, last_dst_level;
     RECT rect;
 
     DBG("This=%p pSourceTexture=%p pDestinationTexture=%p\n", This,
         pSourceTexture, pDestinationTexture);
 
+    user_assert(pSourceTexture && pDestinationTexture, D3DERR_INVALIDCALL);
     user_assert(pSourceTexture != pDestinationTexture, D3DERR_INVALIDCALL);
 
     user_assert(dstb->base.pool == D3DPOOL_DEFAULT, D3DERR_INVALIDCALL);
     user_assert(srcb->base.pool == D3DPOOL_SYSTEMMEM, D3DERR_INVALIDCALL);
-
-    if (dstb->base.usage & D3DUSAGE_AUTOGENMIPMAP) {
-        /* Only the first level is updated, the others regenerated. */
-        last_level = 0;
-        /* if the source has D3DUSAGE_AUTOGENMIPMAP, we have to ignore
-         * the sublevels, thus level 0 has to match */
-        user_assert(!(srcb->base.usage & D3DUSAGE_AUTOGENMIPMAP) ||
-                    (srcb->base.info.width0 == dstb->base.info.width0 &&
-                     srcb->base.info.height0 == dstb->base.info.height0 &&
-                     srcb->base.info.depth0 == dstb->base.info.depth0),
-                    D3DERR_INVALIDCALL);
-    } else {
-        user_assert(!(srcb->base.usage & D3DUSAGE_AUTOGENMIPMAP), D3DERR_INVALIDCALL);
-    }
-
     user_assert(dstb->base.type == srcb->base.type, D3DERR_INVALIDCALL);
+    user_assert(!(srcb->base.usage & D3DUSAGE_AUTOGENMIPMAP) ||
+                dstb->base.usage & D3DUSAGE_AUTOGENMIPMAP, D3DERR_INVALIDCALL);
+
+    /* Spec: Failure if
+     * . Different formats
+     * . Fewer src levels than dst levels (if the opposite, only matching levels
+     *   are supposed to be copied)
+     * . Levels do not match
+     * DDI: Actually the above should pass because of legacy applications
+     * Do what you want about these, but you shouldn't crash.
+     * However driver can expect that the top dimension is greater for src than dst.
+     * Wine tests: Every combination that passes the initial checks should pass.
+     * . Different formats => conversion driver and format dependent.
+     * . 1 level, but size not matching => copy is done (and even crash if src bigger
+     * than dst. For the case where dst bigger, wine doesn't test if a stretch is applied
+     * or if a subrect is copied).
+     * . 8x8 4 sublevels -> 7x7 2 sublevels => driver dependent, On NV seems to be 4x4 subrect
+     * copied to 7x7.
+     *
+     * From these, the proposal is:
+     * . Different formats -> use util_format_translate to translate if possible for surfaces.
+     * Accept ARGB/XRGB for Volumes. Do nothing for the other combinations
+     * . First level copied -> the first level such that src is smaller or equal to dst first level
+     * . number of levels copied -> as long as it fits and textures have levels
+     * That should satisfy the constraints (and instead of crashing for some cases we return D3D_OK)
+     */
 
-    /* Find src level that matches dst level 0: */
-    user_assert(srcb->base.info.width0 >= dstb->base.info.width0 &&
-                srcb->base.info.height0 >= dstb->base.info.height0 &&
-                srcb->base.info.depth0 >= dstb->base.info.depth0,
-                D3DERR_INVALIDCALL);
-    for (m = 0; m <= srcb->base.info.last_level; ++m) {
+    last_src_level = (srcb->base.usage & D3DUSAGE_AUTOGENMIPMAP) ? 0 : srcb->base.info.last_level;
+    last_dst_level = (dstb->base.usage & D3DUSAGE_AUTOGENMIPMAP) ? 0 : dstb->base.info.last_level;
+
+    for (m = 0; m <= last_src_level; ++m) {
         unsigned w = u_minify(srcb->base.info.width0, m);
         unsigned h = u_minify(srcb->base.info.height0, m);
         unsigned d = u_minify(srcb->base.info.depth0, m);
 
-        if (w == dstb->base.info.width0 &&
-            h == dstb->base.info.height0 &&
-            d == dstb->base.info.depth0)
+        if (w <= dstb->base.info.width0 &&
+            h <= dstb->base.info.height0 &&
+            d <= dstb->base.info.depth0)
             break;
     }
-    user_assert(m <= srcb->base.info.last_level, D3DERR_INVALIDCALL);
+    user_assert(m <= last_src_level, D3D_OK);
 
-    last_level = MIN2(last_level, srcb->base.info.last_level - m);
+    last_dst_level = MIN2(srcb->base.info.last_level - m, last_dst_level);
 
     if (dstb->base.type == D3DRTYPE_TEXTURE) {
         struct NineTexture9 *dst = NineTexture9(dstb);
@@ -1375,7 +1385,7 @@ NineDevice9_UpdateTexture( struct NineDevice9 *This,
         for (l = 0; l < m; ++l)
             rect_minify_inclusive(&rect);
 
-        for (l = 0; l <= last_level; ++l, ++m) {
+        for (l = 0; l <= last_dst_level; ++l, ++m) {
             fit_rect_format_inclusive(dst->base.base.info.format,
                                       &rect,
                                       dst->surfaces[l]->desc.Width,
@@ -1402,7 +1412,7 @@ NineDevice9_UpdateTexture( struct NineDevice9 *This,
             for (l = 0; l < m; ++l)
                 rect_minify_inclusive(&rect);
 
-            for (l = 0; l <= last_level; ++l, ++m) {
+            for (l = 0; l <= last_dst_level; ++l, ++m) {
                 fit_rect_format_inclusive(dst->base.base.info.format,
                                           &rect,
                                           dst->surfaces[l * 6 + z]->desc.Width,
@@ -1421,9 +1431,17 @@ NineDevice9_UpdateTexture( struct NineDevice9 *This,
         struct NineVolumeTexture9 *dst = NineVolumeTexture9(dstb);
         struct NineVolumeTexture9 *src = NineVolumeTexture9(srcb);
 
+        /* Wine tests say XRGB -> ARGB should actually do something.
+         * For now do this improper conversion, but in the future it
+         * would be better to implement it properly */
+        user_assert(srcb->format == dstb->format ||
+                    (srcb->format == D3DFMT_A8R8G8B8 && dstb->format == D3DFMT_X8R8G8B8) ||
+                    (srcb->format == D3DFMT_X8R8G8B8 && dstb->format == D3DFMT_A8R8G8B8),
+                    D3D_OK);
+
         if (src->dirty_box.width == 0)
             return D3D_OK;
-        for (l = 0; l <= last_level; ++l, ++m)
+        for (l = 0; l <= last_dst_level; ++l, ++m)
             NineVolume9_CopyMemToDefault(dst->volumes[l],
                                          src->volumes[m], 0, 0, 0, NULL);
         u_box_3d(0, 0, 0, 0, 0, 0, &src->dirty_box);

diff --git a/src/gallium/state_trackers/nine/surface9.c b/src/gallium/state_trackers/nine/surface9.c
index 4c4234bfe277bb9fb7c6f30253425dec97341fb9..da2b46ec3b9e61fe8d619e390a867da4564db1d0 100644 (file)
--- a/src/gallium/state_trackers/nine/surface9.c
+++ b/src/gallium/state_trackers/nine/surface9.c
@@ -480,9 +480,10 @@ NineSurface9_CopyMemToDefault( struct NineSurface9 *This,
                                const RECT *pSourceRect )
 {
     struct pipe_context *pipe = This->pipe;
+    struct pipe_transfer *transfer = NULL;
     struct pipe_resource *r_dst = This->base.resource;
     struct pipe_box dst_box;
-    const uint8_t *p_src;
+    uint8_t *map = NULL;
     int src_x, src_y, dst_x, dst_y, copy_width, copy_height;
 
     assert(This->base.pool == D3DPOOL_DEFAULT &&
@@ -511,11 +512,25 @@ NineSurface9_CopyMemToDefault( struct NineSurface9 *This,
     u_box_2d_zslice(dst_x, dst_y, This->layer,
                     copy_width, copy_height, &dst_box);
 
-    p_src = NineSurface9_GetSystemMemPointer(From, src_x, src_y);
+    map = pipe->transfer_map(pipe,
+                             r_dst,
+                             This->level,
+                             PIPE_TRANSFER_WRITE | PIPE_TRANSFER_DISCARD_RANGE,
+                             &dst_box, &transfer);
+    if (!map)
+        return;
 
-    pipe->transfer_inline_write(pipe, r_dst, This->level,
-                                0, /* WRITE|DISCARD are implicit */
-                                &dst_box, p_src, From->stride, 0);
+    /* Note: if formats are the sames, it will revert
+     * to normal memcpy */
+    (void) util_format_translate(r_dst->format,
+                                 map, transfer->stride,
+                                 0, 0,
+                                 From->base.info.format,
+                                 From->data, From->stride,
+                                 src_x, src_y,
+                                 copy_width, copy_height);
+
+    pipe_transfer_unmap(pipe, transfer);
 
     NineSurface9_MarkContainerDirty(This);
 }