[PATCH 01/11] ddraw: Fix setting current viewport for ddraw1.

[Paul Gofman]

Signed-off-by: Paul Gofman <gofmanp(a)gmail.com>
---
 dlls/ddraw/ddraw_private.h |  2 +-
 dlls/ddraw/device.c        |  6 +++++-
 dlls/ddraw/executebuffer.c | 13 +------------
 3 files changed, 7 insertions(+), 14 deletions(-)

diff --git a/dlls/ddraw/ddraw_private.h b/dlls/ddraw/ddraw_private.h
index 19f21a8923..7137f78dad 100644
--- a/dlls/ddraw/ddraw_private.h
+++ b/dlls/ddraw/ddraw_private.h
@@ -555,7 +555,7 @@ struct d3d_execute_buffer *unsafe_impl_from_IDirect3DExecuteBuffer(IDirect3DExec
 
 /* The execute function */
 HRESULT d3d_execute_buffer_execute(struct d3d_execute_buffer *execute_buffer,
-        struct d3d_device *device, struct d3d_viewport *viewport) DECLSPEC_HIDDEN;
+        struct d3d_device *device) DECLSPEC_HIDDEN;
 
 /*****************************************************************************
  * IDirect3DVertexBuffer
diff --git a/dlls/ddraw/device.c b/dlls/ddraw/device.c
index 18e585407c..ade56bbc9e 100644
--- a/dlls/ddraw/device.c
+++ b/dlls/ddraw/device.c
@@ -712,9 +712,13 @@ static HRESULT WINAPI d3d_device1_Execute(IDirect3DDevice *iface,
     if(!buffer)
         return DDERR_INVALIDPARAMS;
 
+    if (FAILED(hr = IDirect3DDevice3_SetCurrentViewport
+            (&device->IDirect3DDevice3_iface, &viewport_impl->IDirect3DViewport3_iface)))
+        return hr;
+
     /* Execute... */
     wined3d_mutex_lock();
-    hr = d3d_execute_buffer_execute(buffer, device, viewport_impl);
+    hr = d3d_execute_buffer_execute(buffer, device);
     wined3d_mutex_unlock();
 
     return hr;
diff --git a/dlls/ddraw/executebuffer.c b/dlls/ddraw/executebuffer.c
index f9082dc028..637c2bdc5f 100644
--- a/dlls/ddraw/executebuffer.c
+++ b/dlls/ddraw/executebuffer.c
@@ -48,8 +48,7 @@ static void _dump_D3DEXECUTEBUFFERDESC(const D3DEXECUTEBUFFERDESC *lpDesc) {
     TRACE("lpData       : %p\n", lpDesc->lpData);
 }
 
-HRESULT d3d_execute_buffer_execute(struct d3d_execute_buffer *buffer,
-        struct d3d_device *device, struct d3d_viewport *viewport)
+HRESULT d3d_execute_buffer_execute(struct d3d_execute_buffer *buffer, struct d3d_device *device)
 {
     DWORD is = buffer->data.dwInstructionOffset;
     char *instr = (char *)buffer->desc.lpData + is;
@@ -58,16 +57,6 @@ HRESULT d3d_execute_buffer_execute(struct d3d_execute_buffer *buffer,
     struct wined3d_box box = {0};
     HRESULT hr;
 
-    if (viewport->active_device != device)
-    {
-        WARN("Viewport %p active device is %p.\n",
-                viewport, viewport->active_device);
-        return DDERR_INVALIDPARAMS;
-    }
-
-    /* Activate the viewport */
-    viewport_activate(viewport, FALSE);
-
     TRACE("ExecuteData :\n");
     if (TRACE_ON(ddraw))
         _dump_executedata(&(buffer->data));
-- 
2.21.0

[PATCH 02/11] ddraw: Enable lighting only if material is set in d3d_execute_buffer_execute().

[Paul Gofman]

Signed-off-by: Paul Gofman <gofmanp(a)gmail.com>
---
 dlls/ddraw/executebuffer.c | 20 +++++---------------
 1 file changed, 5 insertions(+), 15 deletions(-)

diff --git a/dlls/ddraw/executebuffer.c b/dlls/ddraw/executebuffer.c
index 637c2bdc5f..d5de513c27 100644
--- a/dlls/ddraw/executebuffer.c
+++ b/dlls/ddraw/executebuffer.c
@@ -306,21 +306,11 @@ HRESULT d3d_execute_buffer_execute(struct d3d_execute_buffer *buffer, struct d3d
                         case D3DPROCESSVERTICES_TRANSFORM:
                             wined3d_device_set_stream_source(device->wined3d_device, 0,
                                     buffer->src_vertex_buffer, buffer->src_vertex_pos, sizeof(D3DVERTEX));
-                            if (op == D3DPROCESSVERTICES_TRANSFORMLIGHT)
-                            {
-                                wined3d_device_set_vertex_declaration(device->wined3d_device,
-                                        ddraw_find_decl(device->ddraw, D3DFVF_VERTEX));
-                                wined3d_device_set_render_state(device->wined3d_device,
-                                        WINED3D_RS_LIGHTING, TRUE);
-                            }
-                            else
-                            {
-                                wined3d_device_set_vertex_declaration(device->wined3d_device,
-                                        ddraw_find_decl(device->ddraw, D3DFVF_LVERTEX));
-                                wined3d_device_set_render_state(device->wined3d_device,
-                                        WINED3D_RS_LIGHTING, FALSE);
-                            }
-
+                            wined3d_device_set_render_state(device->wined3d_device, WINED3D_RS_LIGHTING,
+                                    op == D3DPROCESSVERTICES_TRANSFORMLIGHT && !!device->material);
+                            wined3d_device_set_vertex_declaration(device->wined3d_device,
+                                    ddraw_find_decl(device->ddraw, op == D3DPROCESSVERTICES_TRANSFORMLIGHT
+                                    ? D3DFVF_VERTEX : D3DFVF_LVERTEX));
                             wined3d_device_process_vertices(device->wined3d_device, ci->wStart, ci->wDest,
                                     ci->dwCount, buffer->dst_vertex_buffer, NULL, 0, D3DFVF_TLVERTEX);
                             break;
-- 
2.21.0

[PATCH 02/11] ddraw: Enable lighting only if material is set in d3d_execute_buffer_execute().

[Lukáš Krejčí]

On Wed, 2019-05-15 at 18:06 +0300, Paul Gofman wrote:
> Signed-off-by: Paul Gofman <gofmanp(a)gmail.com>
> ---
>  dlls/ddraw/executebuffer.c | 20 +++++---------------
>  1 file changed, 5 insertions(+), 15 deletions(-)
>
> diff --git a/dlls/ddraw/executebuffer.c b/dlls/ddraw/executebuffer.c
> index 637c2bdc5f..d5de513c27 100644
> --- a/dlls/ddraw/executebuffer.c
> +++ b/dlls/ddraw/executebuffer.c
> @@ -306,21 +306,11 @@ HRESULT d3d_execute_buffer_execute(struct d3d_execute_buffer *buffer, struct d3d
>                          case D3DPROCESSVERTICES_TRANSFORM:
>                              wined3d_device_set_stream_source(device->wined3d_device, 0,
>                                      buffer->src_vertex_buffer, buffer->src_vertex_pos, sizeof(D3DVERTEX));
> -                            if (op == D3DPROCESSVERTICES_TRANSFORMLIGHT)
> -                            {
> -                                wined3d_device_set_vertex_declaration(device->wined3d_device,
> -                                        ddraw_find_decl(device->ddraw, D3DFVF_VERTEX));
> -                                wined3d_device_set_render_state(device->wined3d_device,
> -                                        WINED3D_RS_LIGHTING, TRUE);
> -                            }
> -                            else
> -                            {
> -                                wined3d_device_set_vertex_declaration(device->wined3d_device,
> -                                        ddraw_find_decl(device->ddraw, D3DFVF_LVERTEX));
> -                                wined3d_device_set_render_state(device->wined3d_device,
> -                                        WINED3D_RS_LIGHTING, FALSE);
> -                            }
> -
> +                            wined3d_device_set_render_state(device->wined3d_device, WINED3D_RS_LIGHTING,
> +                                    op == D3DPROCESSVERTICES_TRANSFORMLIGHT && !!device->material);
> +                            wined3d_device_set_vertex_declaration(device->wined3d_device,
> +                                    ddraw_find_decl(device->ddraw, op == D3DPROCESSVERTICES_TRANSFORMLIGHT
> +                                    ? D3DFVF_VERTEX : D3DFVF_LVERTEX));
>                              wined3d_device_process_vertices(device->wined3d_device, ci->wStart, ci->wDest,
>                                      ci->dwCount, buffer->dst_vertex_buffer, NULL, 0, D3DFVF_TLVERTEX);
>                              break;

I briefly tested your patch set in Tomb Raider 4, and it looks like a
patch is missing that enables lighting based on 'vertex_op' in
d3d_vertex_buffer7_ProcessVertices().

Something like this should do the trick:

diff --git a/dlls/ddraw/vertexbuffer.c b/dlls/ddraw/vertexbuffer.c
index d2c2f8ff80..c44664e514 100644
--- a/dlls/ddraw/vertexbuffer.c
+++ b/dlls/ddraw/vertexbuffer.c
@@ -260,7 +260,7 @@ static HRESULT WINAPI d3d_vertex_buffer7_ProcessVertices(IDirect3DVertexBuffer7
     struct d3d_device *device_impl = dst_buffer_impl->version == 7
             ? unsafe_impl_from_IDirect3DDevice7(device)
             : unsafe_impl_from_IDirect3DDevice3((IDirect3DDevice3 *)device);
-    BOOL oldClip, doClip;
+    BOOL oldClip, doClip, oldLight, doLight;
     HRESULT hr;
 
     TRACE("iface %p, vertex_op %#x, dst_idx %u, count %u, src_buffer %p, src_idx %u, device %p, flags %#x.\n",
@@ -290,6 +290,11 @@ static HRESULT WINAPI d3d_vertex_buffer7_ProcessVertices(IDirect3DVertexBuffer7
     if (doClip != oldClip)
         wined3d_device_set_render_state(device_impl->wined3d_device, WINED3D_RS_CLIPPING, doClip);
 
+    doLight = !!(vertex_op & D3DVOP_LIGHT);
+    oldLight = wined3d_device_get_render_state(device_impl->wined3d_device, WINED3D_RS_LIGHTING);
+    if (doLight != oldLight)
+        wined3d_device_set_render_state(device_impl->wined3d_device, WINED3D_RS_LIGHTING, doLight);
+
     wined3d_device_set_stream_source(device_impl->wined3d_device,
             0, src_buffer_impl->wined3d_buffer, 0, get_flexible_vertex_size(src_buffer_impl->fvf));
     wined3d_device_set_vertex_declaration(device_impl->wined3d_device, src_buffer_impl->wined3d_declaration);
@@ -300,6 +305,9 @@ static HRESULT WINAPI d3d_vertex_buffer7_ProcessVertices(IDirect3DVertexBuffer7
     if (doClip != oldClip)
         wined3d_device_set_render_state(device_impl->wined3d_device, WINED3D_RS_CLIPPING, oldClip);
 
+    if (doLight != oldLight)
+        wined3d_device_set_render_state(device_impl->wined3d_device, WINED3D_RS_LIGHTING, oldLight);
+
     wined3d_mutex_unlock();
 
     return hr;

[PATCH 02/11] ddraw: Enable lighting only if material is set in d3d_execute_buffer_execute().

[Paul Gofman]

Yes, I tested Tomb Raider also, and it is missing a bit more than that. 
It also uses point and spot lights in the first place. It also uses up 
to 21 active lights (in my demo level where I tested that), and a 
considerably greater amount of overall lights which are not active 
together, while ddraw has an overall limit of 8 now. I have the patches 
for all of that and got it working normally, but it makes no sense to 
send them all at once and it will need more tests on the way than I have 
now.

On 5/16/19 13:34, Lukáš Krejčí wrote:
> I briefly tested your patch set in Tomb Raider 4, and it looks like a
> patch is missing that enables lighting based on 'vertex_op' in
> d3d_vertex_buffer7_ProcessVertices().

[PATCH 02/11] ddraw: Enable lighting only if material is set in d3d_execute_buffer_execute().

[Henri Verbeet]

Signed-off-by: Henri Verbeet <hverbeet(a)codeweavers.com>

[PATCH 02/11] ddraw: Enable lighting only if material is set in d3d_execute_buffer_execute().

[Henri Verbeet]

On Wed, 15 May 2019 at 19:37, Paul Gofman <gofmanp(a)gmail.com> wrote:
> +                            wined3d_device_set_render_state(device->wined3d_device, WINED3D_RS_LIGHTING,
> +                                    op == D3DPROCESSVERTICES_TRANSFORMLIGHT && !!device->material);
> The "!!" here is technically redundant, but shouldn't hurt.

[PATCH 03/11] wined3d: Move validate_material_colour_source() to wined3d_private.h.

[Paul Gofman]

Signed-off-by: Paul Gofman <gofmanp(a)gmail.com>
---
 dlls/wined3d/utils.c           | 10 ----------
 dlls/wined3d/wined3d_private.h | 10 ++++++++++
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/dlls/wined3d/utils.c b/dlls/wined3d/utils.c
index 669c8cbbbd..06ff97ce8f 100644
--- a/dlls/wined3d/utils.c
+++ b/dlls/wined3d/utils.c
@@ -6378,16 +6378,6 @@ int wined3d_ffp_frag_program_key_compare(const void *key, const struct wine_rb_e
     return memcmp(ka, kb, sizeof(*ka));
 }
 
-static enum wined3d_material_color_source validate_material_colour_source(WORD use_map,
-        enum wined3d_material_color_source source)
-{
-    if (source == WINED3D_MCS_COLOR1 && use_map & (1u << WINED3D_FFP_DIFFUSE))
-        return source;
-    if (source == WINED3D_MCS_COLOR2 && use_map & (1u << WINED3D_FFP_SPECULAR))
-        return source;
-    return WINED3D_MCS_MATERIAL;
-}
-
 void wined3d_ffp_get_vs_settings(const struct wined3d_context *context,
         const struct wined3d_state *state, struct wined3d_ffp_vs_settings *settings)
 {
diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h
index bb982f8afb..d76166690e 100644
--- a/dlls/wined3d/wined3d_private.h
+++ b/dlls/wined3d/wined3d_private.h
@@ -4840,6 +4840,16 @@ static inline void wined3d_not_from_cs(struct wined3d_cs *cs)
     assert(cs->thread_id != GetCurrentThreadId());
 }
 
+static inline enum wined3d_material_color_source validate_material_colour_source(WORD use_map,
+        enum wined3d_material_color_source source)
+{
+    if (source == WINED3D_MCS_COLOR1 && use_map & (1u << WINED3D_FFP_DIFFUSE))
+        return source;
+    if (source == WINED3D_MCS_COLOR2 && use_map & (1u << WINED3D_FFP_SPECULAR))
+        return source;
+    return WINED3D_MCS_MATERIAL;
+}
+
 /* The WNDCLASS-Name for the fake window which we use to retrieve the GL capabilities */
 #define WINED3D_OPENGL_WINDOW_CLASS_NAME "WineD3D_OpenGL"
 
-- 
2.21.0

[PATCH 03/11] wined3d: Move validate_material_colour_source() to wined3d_private.h.

[Henri Verbeet]

Signed-off-by: Henri Verbeet <hverbeet(a)codeweavers.com>

[PATCH 04/11] wined3d: Use color source for output colors in process_vertices_strided().

[Paul Gofman]

Signed-off-by: Paul Gofman <gofmanp(a)gmail.com>
---
 dlls/wined3d/device.c          | 127 +++++++++++++++++++++++----------
 dlls/wined3d/wined3d_private.h |  10 +++
 2 files changed, 100 insertions(+), 37 deletions(-)

diff --git a/dlls/wined3d/device.c b/dlls/wined3d/device.c
index 5272e2ef23..938c420f5b 100644
--- a/dlls/wined3d/device.c
+++ b/dlls/wined3d/device.c
@@ -3072,20 +3072,71 @@ static unsigned int wined3d_get_flexible_vertex_size(DWORD fvf)
     return size;
 }
 
+static void color_from_mcs(struct wined3d_color *color, enum wined3d_material_color_source mcs,
+        const struct wined3d_color *material_color, unsigned int index,
+        const struct wined3d_stream_info *stream_info)
+{
+    const struct wined3d_stream_info_element *element = NULL;
+
+    switch (mcs)
+    {
+        case WINED3D_MCS_MATERIAL:
+            *color = *material_color;
+            return;
+        case WINED3D_MCS_COLOR1:
+            if (!(stream_info->use_map & (1u << WINED3D_FFP_DIFFUSE)))
+            {
+                color->r = color->g = color->b = color->a = 1.0f;
+                return;
+            }
+            element = &stream_info->elements[WINED3D_FFP_DIFFUSE];
+            break;
+        case WINED3D_MCS_COLOR2:
+            if (!(stream_info->use_map & (1u << WINED3D_FFP_SPECULAR)))
+            {
+                color->r = color->g = color->b = 0.0f;
+                color->a = 1.0f;
+                return;
+            }
+            element = &stream_info->elements[WINED3D_FFP_SPECULAR];
+            break;
+        default:
+            ERR("Invalid material color source %#x.\n", mcs);
+            break;
+    }
+    wined3d_color_from_d3dcolor(color, element ? *(const DWORD *)(element->data.addr + index * element->stride) : 0);
+}
+
+static float clamp(float value, float min_value, float max_value)
+{
+    return value < min_value ? min_value : value > max_value ? max_value : value;
+}
+
+static void clamp_vec(float *dst, const float *src, unsigned int count,
+        float min_value, float max_value)
+{
+    unsigned int i;
+
+    for (i = 0; i < count; ++i)
+        dst[i] = clamp(src[i], min_value, max_value);
+}
+
 /* Context activation is done by the caller. */
 #define copy_and_next(dest, src, size) memcpy(dest, src, size); dest += (size)
 static HRESULT process_vertices_strided(const struct wined3d_device *device, DWORD dwDestIndex, DWORD dwCount,
         const struct wined3d_stream_info *stream_info, struct wined3d_buffer *dest, DWORD flags, DWORD dst_fvf)
 {
+    enum wined3d_material_color_source diffuse_source, specular_source;
     struct wined3d_matrix mat, proj_mat, view_mat, world_mat;
+    const struct wined3d_state *state = &device->state;
     struct wined3d_map_desc map_desc;
     struct wined3d_box box = {0};
     struct wined3d_viewport vp;
     unsigned int vertex_size;
+    BOOL doClip, lighting;
+    DWORD numTextures;
     unsigned int i;
     BYTE *dest_ptr;
-    BOOL doClip;
-    DWORD numTextures;
     HRESULT hr;
 
     if (stream_info->use_map & (1u << WINED3D_FFP_NORMAL))
@@ -3095,11 +3146,11 @@ static HRESULT process_vertices_strided(const struct wined3d_device *device, DWO
 
     if (!(stream_info->use_map & (1u << WINED3D_FFP_POSITION)))
     {
-        ERR("Source has no position mask\n");
+        ERR("Source has no position mask.\n");
         return WINED3DERR_INVALIDCALL;
     }
 
-    if (device->state.render_states[WINED3D_RS_CLIPPING])
+    if (state->render_states[WINED3D_RS_CLIPPING])
     {
         static BOOL warned = FALSE;
         /*
@@ -3156,11 +3207,34 @@ static HRESULT process_vertices_strided(const struct wined3d_device *device, DWO
     TRACE("viewport x %.8e, y %.8e, width %.8e, height %.8e, min_z %.8e, max_z %.8e.\n",
           vp.x, vp.y, vp.width, vp.height, vp.min_z, vp.max_z);
 
+    lighting = state->render_states[WINED3D_RS_LIGHTING]
+            && (dst_fvf & (WINED3DFVF_DIFFUSE | WINED3DFVF_SPECULAR));
+
     multiply_matrix(&mat,&view_mat,&world_mat);
     multiply_matrix(&mat,&proj_mat,&mat);
 
     numTextures = (dst_fvf & WINED3DFVF_TEXCOUNT_MASK) >> WINED3DFVF_TEXCOUNT_SHIFT;
 
+    if (lighting)
+    {
+        if (state->render_states[WINED3D_RS_COLORVERTEX])
+        {
+            diffuse_source = validate_material_colour_source(stream_info->use_map,
+                    state->render_states[WINED3D_RS_DIFFUSEMATERIALSOURCE]);
+            specular_source = validate_material_colour_source(stream_info->use_map,
+                    state->render_states[WINED3D_RS_SPECULARMATERIALSOURCE]);
+        }
+        else
+        {
+            diffuse_source = specular_source = WINED3D_MCS_MATERIAL;
+        }
+    }
+    else
+    {
+        diffuse_source = WINED3D_MCS_COLOR1;
+        specular_source = WINED3D_MCS_COLOR2;
+    }
+
     for (i = 0; i < dwCount; i+= 1) {
         unsigned int tex_index;
 
@@ -3278,47 +3352,26 @@ static HRESULT process_vertices_strided(const struct wined3d_device *device, DWO
 
         if (dst_fvf & WINED3DFVF_DIFFUSE)
         {
-            const struct wined3d_stream_info_element *element = &stream_info->elements[WINED3D_FFP_DIFFUSE];
-            const DWORD *color_d = (const DWORD *)(element->data.addr + i * element->stride);
-            if (!(stream_info->use_map & (1u << WINED3D_FFP_DIFFUSE)))
-            {
-                static BOOL warned = FALSE;
+            struct wined3d_color material_diffuse;
 
-                if(!warned) {
-                    ERR("No diffuse color in source, but destination has one\n");
-                    warned = TRUE;
-                }
+            color_from_mcs(&material_diffuse, diffuse_source, &state->material.diffuse, i, stream_info);
 
-                *( (DWORD *) dest_ptr) = 0xffffffff;
-                dest_ptr += sizeof(DWORD);
-            }
-            else
-            {
-                copy_and_next(dest_ptr, color_d, sizeof(DWORD));
-            }
+            clamp_vec(&material_diffuse.r, &material_diffuse.r, 4, 0.0f, 1.0f);
+            *((DWORD *)dest_ptr) = d3dcolor_from_wined3d_color(&material_diffuse);
+            dest_ptr += sizeof(DWORD);
         }
 
         if (dst_fvf & WINED3DFVF_SPECULAR)
         {
-            /* What's the color value in the feedback buffer? */
-            const struct wined3d_stream_info_element *element = &stream_info->elements[WINED3D_FFP_SPECULAR];
-            const DWORD *color_s = (const DWORD *)(element->data.addr + i * element->stride);
-            if (!(stream_info->use_map & (1u << WINED3D_FFP_SPECULAR)))
-            {
-                static BOOL warned = FALSE;
+            struct wined3d_color material_specular;
 
-                if(!warned) {
-                    ERR("No specular color in source, but destination has one\n");
-                    warned = TRUE;
-                }
+            memset(&material_specular, 0, sizeof(material_specular));
+            color_from_mcs(&material_specular, specular_source, state->render_states[WINED3D_RS_SPECULARENABLE]
+                    ? &state->material.specular : &material_specular, i, stream_info);
 
-                *(DWORD *)dest_ptr = 0xff000000;
-                dest_ptr += sizeof(DWORD);
-            }
-            else
-            {
-                copy_and_next(dest_ptr, color_s, sizeof(DWORD));
-            }
+            clamp_vec(&material_specular.r, &material_specular.r, 4, 0.0f, 1.0f);
+            *((DWORD *)dest_ptr) = d3dcolor_from_wined3d_color(&material_specular);
+            dest_ptr += sizeof(DWORD);
         }
 
         for (tex_index = 0; tex_index < numTextures; ++tex_index)
diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h
index d76166690e..d59d466cb3 100644
--- a/dlls/wined3d/wined3d_private.h
+++ b/dlls/wined3d/wined3d_private.h
@@ -1469,6 +1469,16 @@ static inline void wined3d_color_from_d3dcolor(struct wined3d_color *wined3d_col
     wined3d_color->a = D3DCOLOR_B_A(d3d_color) / 255.0f;
 }
 
+#define D3DCOLOR_R_B(dw) (((BYTE)(dw)) << 16)
+#define D3DCOLOR_G_B(dw) (((BYTE)(dw)) << 8)
+#define D3DCOLOR_A_B(dw) (((BYTE)(dw)) << 24)
+
+static inline DWORD d3dcolor_from_wined3d_color(const struct wined3d_color *wined3d_color)
+{
+    return D3DCOLOR_R_B(lrint(wined3d_color->r * 255.0f)) | D3DCOLOR_G_B(lrint(wined3d_color->g * 255.0f))
+            | D3DCOLOR_B_B(lrint(wined3d_color->b * 255.0f)) | D3DCOLOR_A_B(lrint(wined3d_color->a * 255.0f));
+}
+
 #define WINED3D_HIGHEST_TRANSFORM_STATE WINED3D_TS_WORLD_MATRIX(255) /* Highest value in wined3d_transform_state. */
 
 void wined3d_check_gl_call(const struct wined3d_gl_info *gl_info,
-- 
2.21.0

[PATCH 04/11] wined3d: Use color source for output colors in process_vertices_strided().

[Henri Verbeet]

On Wed, 15 May 2019 at 19:39, Paul Gofman <gofmanp(a)gmail.com> wrote:
> +static void color_from_mcs(struct wined3d_color *color, enum wined3d_material_color_source mcs,
> +        const struct wined3d_color *material_color, unsigned int index,
> +        const struct wined3d_stream_info *stream_info)
> +{
> +    const struct wined3d_stream_info_element *element = NULL;
> +
> +    switch (mcs)
> +    {
> +        case WINED3D_MCS_MATERIAL:
> +            *color = *material_color;
> +            return;
> +        case WINED3D_MCS_COLOR1:
> +            if (!(stream_info->use_map & (1u << WINED3D_FFP_DIFFUSE)))
> +            {
> +                color->r = color->g = color->b = color->a = 1.0f;
> +                return;
> +            }
> +            element = &stream_info->elements[WINED3D_FFP_DIFFUSE];
> +            break;
> +        case WINED3D_MCS_COLOR2:
> +            if (!(stream_info->use_map & (1u << WINED3D_FFP_SPECULAR)))
> +            {
> +                color->r = color->g = color->b = 0.0f;
> +                color->a = 1.0f;
> +                return;
> +            }
> +            element = &stream_info->elements[WINED3D_FFP_SPECULAR];
> +            break;
> +        default:
> +            ERR("Invalid material color source %#x.\n", mcs);
> +            break;
> +    }
> +    wined3d_color_from_d3dcolor(color, element ? *(const DWORD *)(element->data.addr + index * element->stride) : 0);
> +}
> The naming perhaps doesn't make it obvious, but
> wined3d_color_from_d3dcolor() assumes the source format is
> WINED3DFMT_B8G8R8A8_UNORM. While that should always be true for ddraw,
> it isn't necessarily for d3d9.
> +static void clamp_vec(float *dst, const float *src, unsigned int count,
> +        float min_value, float max_value)
> +{
> +    unsigned int i;
> +
> +    for (i = 0; i < count; ++i)
> +        dst[i] = clamp(src[i], min_value, max_value);
> +}
> The way this is used, that should probably be called "wined3d_color_clamp()".
> if (dst_fvf & WINED3DFVF_SPECULAR)
>          {
> -            /* What's the color value in the feedback buffer? */
> -            const struct wined3d_stream_info_element *element = &stream_info->elements[WINED3D_FFP_SPECULAR];
> -            const DWORD *color_s = (const DWORD *)(element->data.addr + i * element->stride);
> -            if (!(stream_info->use_map & (1u << WINED3D_FFP_SPECULAR)))
> -            {
> -                static BOOL warned = FALSE;
> +            struct wined3d_color material_specular;
>
> -                if(!warned) {
> -                    ERR("No specular color in source, but destination has one\n");
> -                    warned = TRUE;
> -                }
> +            memset(&material_specular, 0, sizeof(material_specular));
> +            color_from_mcs(&material_specular, specular_source, state->render_states[WINED3D_RS_SPECULARENABLE]
> +                    ? &state->material.specular : &material_specular, i, stream_info);

That memset() is redundant, right?
> +#define D3DCOLOR_R_B(dw) (((BYTE)(dw)) << 16)
> +#define D3DCOLOR_G_B(dw) (((BYTE)(dw)) << 8)
> +#define D3DCOLOR_A_B(dw) (((BYTE)(dw)) << 24)
> +
> +static inline DWORD d3dcolor_from_wined3d_color(const struct wined3d_color *wined3d_color)
> +{
> +    return D3DCOLOR_R_B(lrint(wined3d_color->r * 255.0f)) | D3DCOLOR_G_B(lrint(wined3d_color->g * 255.0f))
> +            | D3DCOLOR_B_B(lrint(wined3d_color->b * 255.0f)) | D3DCOLOR_A_B(lrint(wined3d_color->a * 255.0f));
> +}
> +
> You're probably looking for wined3d_format_convert_from_float().
> That's likely to be slightly slower than what you have here, but if
> performance were the main concern there would probably be other
> optimisations we could do first.

[PATCH 04/11] wined3d: Use color source for output colors in process_vertices_strided().

[Paul Gofman]

On 5/16/19 21:01, Henri Verbeet wrote:
> On Wed, 15 May 2019 at 19:39, Paul Gofman <gofmanp(a)gmail.com> wrote:
> > +static void color_from_mcs(struct wined3d_color *color, enum wined3d_material_color_source mcs,
> > +        const struct wined3d_color *material_color, unsigned int index,
> > +        const struct wined3d_stream_info *stream_info)
> > +{
> > +    const struct wined3d_stream_info_element *element = NULL;
> > +
> > +    switch (mcs)
> > +    {
> > +        case WINED3D_MCS_MATERIAL:
> > +            *color = *material_color;
> > +            return;
> > +        case WINED3D_MCS_COLOR1:
> > +            if (!(stream_info->use_map & (1u << WINED3D_FFP_DIFFUSE)))
> > +            {
> > +                color->r = color->g = color->b = color->a = 1.0f;
> > +                return;
> > +            }
> > +            element = &stream_info->elements[WINED3D_FFP_DIFFUSE];
> > +            break;
> > +        case WINED3D_MCS_COLOR2:
> > +            if (!(stream_info->use_map & (1u << WINED3D_FFP_SPECULAR)))
> > +            {
> > +                color->r = color->g = color->b = 0.0f;
> > +                color->a = 1.0f;
> > +                return;
> > +            }
> > +            element = &stream_info->elements[WINED3D_FFP_SPECULAR];
> > +            break;
> > +        default:
> > +            ERR("Invalid material color source %#x.\n", mcs);
> > +            break;
> > +    }
> > +    wined3d_color_from_d3dcolor(color, element ? *(const DWORD *)(element->data.addr + index * element->stride) : 0);
> > +}
> > The naming perhaps doesn't make it obvious, but
> > wined3d_color_from_d3dcolor() assumes the source format is
> > WINED3DFMT_B8G8R8A8_UNORM. While that should always be true for ddraw,
> > it isn't necessarily for d3d9.

Oh yeah, I should add a generic conversion function based on declaration 
type. Still, should I maybe add just FIXME now for if decltype is not 
_D3DCOLOR for now, and imlement a generic conversion later when I will 
be adding some tests for process vertices in higher Directx versions?
> > if (dst_fvf & WINED3DFVF_SPECULAR)
> >           {
> > -            /* What's the color value in the feedback buffer? */
> > -            const struct wined3d_stream_info_element *element = &stream_info->elements[WINED3D_FFP_SPECULAR];
> > -            const DWORD *color_s = (const DWORD *)(element->data.addr + i * element->stride);
> > -            if (!(stream_info->use_map & (1u << WINED3D_FFP_SPECULAR)))
> > -            {
> > -                static BOOL warned = FALSE;
> > +            struct wined3d_color material_specular;
> >
> > -                if(!warned) {
> > -                    ERR("No specular color in source, but destination has one\n");
> > -                    warned = TRUE;
> > -                }
> > +            memset(&material_specular, 0, sizeof(material_specular));
> > +            color_from_mcs(&material_specular, specular_source, state->render_states[WINED3D_RS_SPECULARENABLE]
> > +                    ? &state->material.specular : &material_specular, i, stream_info);
>
> That memset() is redundant, right?
> It's purpose is to provide zeroes for input material color if 
> _RS_SPECULARENABLE is not set. How it is currently written, 
> material_specular will end up uninitialized if I remove the memset and 
> specular lihgting is not enabled.

[PATCH 04/11] wined3d: Use color source for output colors in process_vertices_strided().

[Henri Verbeet]

On Thu, 16 May 2019 at 22:53, Paul Gofman <gofmanp(a)gmail.com> wrote:
> On 5/16/19 21:01, Henri Verbeet wrote:
> > The naming perhaps doesn't make it obvious, but
> > wined3d_color_from_d3dcolor() assumes the source format is
> > WINED3DFMT_B8G8R8A8_UNORM. While that should always be true for ddraw,
> > it isn't necessarily for d3d9.
>
> Oh yeah, I should add a generic conversion function based on declaration
> type. Still, should I maybe add just FIXME now for if decltype is not
> _D3DCOLOR for now, and imlement a generic conversion later when I will
> be adding some tests for process vertices in higher Directx versions?

Sure, that works. At the same time, it shouldn't be too hard to just
implement either. (See also color_to_float() and
wined3d_format_get_float_color_key().)
> > > if (dst_fvf & WINED3DFVF_SPECULAR)
> > >           {
> > > -            /* What's the color value in the feedback buffer? */
> > > -            const struct wined3d_stream_info_element *element = &stream_info->elements[WINED3D_FFP_SPECULAR];
> > > -            const DWORD *color_s = (const DWORD *)(element->data.addr + i * element->stride);
> > > -            if (!(stream_info->use_map & (1u << WINED3D_FFP_SPECULAR)))
> > > -            {
> > > -                static BOOL warned = FALSE;
> > > +            struct wined3d_color material_specular;
> > >
> > > -                if(!warned) {
> > > -                    ERR("No specular color in source, but destination has one\n");
> > > -                    warned = TRUE;
> > > -                }
> > > +            memset(&material_specular, 0, sizeof(material_specular));
> > > +            color_from_mcs(&material_specular, specular_source, state->render_states[WINED3D_RS_SPECULARENABLE]
> > > +                    ? &state->material.specular : &material_specular, i, stream_info);
> >
> > That memset() is redundant, right?
> > It's purpose is to provide zeroes for input material color if
> > _RS_SPECULARENABLE is not set. How it is currently written,
> > material_specular will end up uninitialized if I remove the memset and
> > specular lihgting is not enabled.

Hmm, right. It may still be best to move that out of the main loop though.

[PATCH 05/11] wined3d: Factor out compute_normal_matrix() function.

[Paul Gofman]

Signed-off-by: Paul Gofman <gofmanp(a)gmail.com>
---
 dlls/wined3d/glsl_shader.c     | 289 +-------------------------------
 dlls/wined3d/utils.c           | 294 +++++++++++++++++++++++++++++++++
 dlls/wined3d/wined3d_private.h |   5 +
 3 files changed, 301 insertions(+), 287 deletions(-)

diff --git a/dlls/wined3d/glsl_shader.c b/dlls/wined3d/glsl_shader.c
index 386ce78f41..5684e187b1 100644
--- a/dlls/wined3d/glsl_shader.c
+++ b/dlls/wined3d/glsl_shader.c
@@ -1296,281 +1296,6 @@ static void shader_glsl_load_np2fixup_constants(const struct glsl_ps_program *ps
     GL_EXTCALL(glUniform4fv(ps->np2_fixup_location, ps->np2_fixup_info->num_consts, &np2fixup_constants[0].sx));
 }
 
-/* Taken and adapted from Mesa. */
-static BOOL invert_matrix_3d(struct wined3d_matrix *out, const struct wined3d_matrix *in)
-{
-    float pos, neg, t, det;
-    struct wined3d_matrix temp;
-
-    /* Calculate the determinant of upper left 3x3 submatrix and
-     * determine if the matrix is singular. */
-    pos = neg = 0.0f;
-    t =  in->_11 * in->_22 * in->_33;
-    if (t >= 0.0f)
-        pos += t;
-    else
-        neg += t;
-
-    t =  in->_21 * in->_32 * in->_13;
-    if (t >= 0.0f)
-        pos += t;
-    else
-        neg += t;
-    t =  in->_31 * in->_12 * in->_23;
-    if (t >= 0.0f)
-        pos += t;
-    else
-        neg += t;
-
-    t = -in->_31 * in->_22 * in->_13;
-    if (t >= 0.0f)
-        pos += t;
-    else
-        neg += t;
-    t = -in->_21 * in->_12 * in->_33;
-    if (t >= 0.0f)
-        pos += t;
-    else
-        neg += t;
-
-    t = -in->_11 * in->_32 * in->_23;
-    if (t >= 0.0f)
-        pos += t;
-    else
-        neg += t;
-
-    det = pos + neg;
-
-    if (fabsf(det) < 1e-25f)
-        return FALSE;
-
-    det = 1.0f / det;
-    temp._11 =  (in->_22 * in->_33 - in->_32 * in->_23) * det;
-    temp._12 = -(in->_12 * in->_33 - in->_32 * in->_13) * det;
-    temp._13 =  (in->_12 * in->_23 - in->_22 * in->_13) * det;
-    temp._21 = -(in->_21 * in->_33 - in->_31 * in->_23) * det;
-    temp._22 =  (in->_11 * in->_33 - in->_31 * in->_13) * det;
-    temp._23 = -(in->_11 * in->_23 - in->_21 * in->_13) * det;
-    temp._31 =  (in->_21 * in->_32 - in->_31 * in->_22) * det;
-    temp._32 = -(in->_11 * in->_32 - in->_31 * in->_12) * det;
-    temp._33 =  (in->_11 * in->_22 - in->_21 * in->_12) * det;
-
-    *out = temp;
-    return TRUE;
-}
-
-static void swap_rows(float **a, float **b)
-{
-    float *tmp = *a;
-
-    *a = *b;
-    *b = tmp;
-}
-
-static BOOL invert_matrix(struct wined3d_matrix *out, const struct wined3d_matrix *m)
-{
-    float wtmp[4][8];
-    float m0, m1, m2, m3, s;
-    float *r0, *r1, *r2, *r3;
-
-    r0 = wtmp[0];
-    r1 = wtmp[1];
-    r2 = wtmp[2];
-    r3 = wtmp[3];
-
-    r0[0] = m->_11;
-    r0[1] = m->_12;
-    r0[2] = m->_13;
-    r0[3] = m->_14;
-    r0[4] = 1.0f;
-    r0[5] = r0[6] = r0[7] = 0.0f;
-
-    r1[0] = m->_21;
-    r1[1] = m->_22;
-    r1[2] = m->_23;
-    r1[3] = m->_24;
-    r1[5] = 1.0f;
-    r1[4] = r1[6] = r1[7] = 0.0f;
-
-    r2[0] = m->_31;
-    r2[1] = m->_32;
-    r2[2] = m->_33;
-    r2[3] = m->_34;
-    r2[6] = 1.0f;
-    r2[4] = r2[5] = r2[7] = 0.0f;
-
-    r3[0] = m->_41;
-    r3[1] = m->_42;
-    r3[2] = m->_43;
-    r3[3] = m->_44;
-    r3[7] = 1.0f;
-    r3[4] = r3[5] = r3[6] = 0.0f;
-
-    /* Choose pivot - or die. */
-    if (fabsf(r3[0]) > fabsf(r2[0]))
-        swap_rows(&r3, &r2);
-    if (fabsf(r2[0]) > fabsf(r1[0]))
-        swap_rows(&r2, &r1);
-    if (fabsf(r1[0]) > fabsf(r0[0]))
-        swap_rows(&r1, &r0);
-    if (r0[0] == 0.0f)
-        return FALSE;
-
-    /* Eliminate first variable. */
-    m1 = r1[0] / r0[0]; m2 = r2[0] / r0[0]; m3 = r3[0] / r0[0];
-    s = r0[1]; r1[1] -= m1 * s; r2[1] -= m2 * s; r3[1] -= m3 * s;
-    s = r0[2]; r1[2] -= m1 * s; r2[2] -= m2 * s; r3[2] -= m3 * s;
-    s = r0[3]; r1[3] -= m1 * s; r2[3] -= m2 * s; r3[3] -= m3 * s;
-    s = r0[4];
-    if (s != 0.0f)
-    {
-        r1[4] -= m1 * s;
-        r2[4] -= m2 * s;
-        r3[4] -= m3 * s;
-    }
-    s = r0[5];
-    if (s != 0.0f)
-    {
-        r1[5] -= m1 * s;
-        r2[5] -= m2 * s;
-        r3[5] -= m3 * s;
-    }
-    s = r0[6];
-    if (s != 0.0f)
-    {
-        r1[6] -= m1 * s;
-        r2[6] -= m2 * s;
-        r3[6] -= m3 * s;
-    }
-    s = r0[7];
-    if (s != 0.0f)
-    {
-        r1[7] -= m1 * s;
-        r2[7] -= m2 * s;
-        r3[7] -= m3 * s;
-    }
-
-    /* Choose pivot - or die. */
-    if (fabsf(r3[1]) > fabsf(r2[1]))
-        swap_rows(&r3, &r2);
-    if (fabsf(r2[1]) > fabsf(r1[1]))
-        swap_rows(&r2, &r1);
-    if (r1[1] == 0.0f)
-        return FALSE;
-
-    /* Eliminate second variable. */
-    m2 = r2[1] / r1[1]; m3 = r3[1] / r1[1];
-    r2[2] -= m2 * r1[2]; r3[2] -= m3 * r1[2];
-    r2[3] -= m2 * r1[3]; r3[3] -= m3 * r1[3];
-    s = r1[4];
-    if (s != 0.0f)
-    {
-        r2[4] -= m2 * s;
-        r3[4] -= m3 * s;
-    }
-    s = r1[5];
-    if (s != 0.0f)
-    {
-        r2[5] -= m2 * s;
-        r3[5] -= m3 * s;
-    }
-    s = r1[6];
-    if (s != 0.0f)
-    {
-        r2[6] -= m2 * s;
-        r3[6] -= m3 * s;
-    }
-    s = r1[7];
-    if (s != 0.0f)
-    {
-        r2[7] -= m2 * s;
-        r3[7] -= m3 * s;
-    }
-
-    /* Choose pivot - or die. */
-    if (fabsf(r3[2]) > fabsf(r2[2]))
-        swap_rows(&r3, &r2);
-    if (r2[2] == 0.0f)
-        return FALSE;
-
-    /* Eliminate third variable. */
-    m3 = r3[2] / r2[2];
-    r3[3] -= m3 * r2[3];
-    r3[4] -= m3 * r2[4];
-    r3[5] -= m3 * r2[5];
-    r3[6] -= m3 * r2[6];
-    r3[7] -= m3 * r2[7];
-
-    /* Last check. */
-    if (r3[3] == 0.0f)
-        return FALSE;
-
-    /* Back substitute row 3. */
-    s = 1.0f / r3[3];
-    r3[4] *= s;
-    r3[5] *= s;
-    r3[6] *= s;
-    r3[7] *= s;
-
-    /* Back substitute row 2. */
-    m2 = r2[3];
-    s = 1.0f / r2[2];
-    r2[4] = s * (r2[4] - r3[4] * m2);
-    r2[5] = s * (r2[5] - r3[5] * m2);
-    r2[6] = s * (r2[6] - r3[6] * m2);
-    r2[7] = s * (r2[7] - r3[7] * m2);
-    m1 = r1[3];
-    r1[4] -= r3[4] * m1;
-    r1[5] -= r3[5] * m1;
-    r1[6] -= r3[6] * m1;
-    r1[7] -= r3[7] * m1;
-    m0 = r0[3];
-    r0[4] -= r3[4] * m0;
-    r0[5] -= r3[5] * m0;
-    r0[6] -= r3[6] * m0;
-    r0[7] -= r3[7] * m0;
-
-    /* Back substitute row 1. */
-    m1 = r1[2];
-    s = 1.0f / r1[1];
-    r1[4] = s * (r1[4] - r2[4] * m1);
-    r1[5] = s * (r1[5] - r2[5] * m1);
-    r1[6] = s * (r1[6] - r2[6] * m1);
-    r1[7] = s * (r1[7] - r2[7] * m1);
-    m0 = r0[2];
-    r0[4] -= r2[4] * m0;
-    r0[5] -= r2[5] * m0;
-    r0[6] -= r2[6] * m0;
-    r0[7] -= r2[7] * m0;
-
-    /* Back substitute row 0. */
-    m0 = r0[1];
-    s = 1.0f / r0[0];
-    r0[4] = s * (r0[4] - r1[4] * m0);
-    r0[5] = s * (r0[5] - r1[5] * m0);
-    r0[6] = s * (r0[6] - r1[6] * m0);
-    r0[7] = s * (r0[7] - r1[7] * m0);
-
-    out->_11 = r0[4];
-    out->_12 = r0[5];
-    out->_13 = r0[6];
-    out->_14 = r0[7];
-    out->_21 = r1[4];
-    out->_22 = r1[5];
-    out->_23 = r1[6];
-    out->_24 = r1[7];
-    out->_31 = r2[4];
-    out->_32 = r2[5];
-    out->_33 = r2[6];
-    out->_34 = r2[7];
-    out->_41 = r3[4];
-    out->_42 = r3[5];
-    out->_43 = r3[6];
-    out->_44 = r3[7];
-
-    return TRUE;
-}
-
 static void transpose_matrix(struct wined3d_matrix *out, const struct wined3d_matrix *m)
 {
     struct wined3d_matrix temp;
@@ -1587,24 +1312,14 @@ static void shader_glsl_ffp_vertex_normalmatrix_uniform(const struct wined3d_con
         const struct wined3d_state *state, struct glsl_shader_prog_link *prog)
 {
     const struct wined3d_gl_info *gl_info = context->gl_info;
-    float mat[3 * 3];
     struct wined3d_matrix mv;
-    unsigned int i, j;
+    float mat[3 * 3];
 
     if (prog->vs.normal_matrix_location == -1)
         return;
 
     get_modelview_matrix(context, state, 0, &mv);
-    if (context->d3d_info->wined3d_creation_flags & WINED3D_LEGACY_FFP_LIGHTING)
-        invert_matrix_3d(&mv, &mv);
-    else
-        invert_matrix(&mv, &mv);
-    /* Tests show that singular modelview matrices are used unchanged as normal
-     * matrices on D3D3 and older. There seems to be no clearly consistent
-     * behavior on newer D3D versions so always follow older ddraw behavior. */
-    for (i = 0; i < 3; ++i)
-        for (j = 0; j < 3; ++j)
-            mat[i * 3 + j] = (&mv._11)[j * 4 + i];
+    compute_normal_matrix(mat, context->d3d_info->wined3d_creation_flags & WINED3D_LEGACY_FFP_LIGHTING, &mv);
 
     GL_EXTCALL(glUniformMatrix3fv(prog->vs.normal_matrix_location, 1, FALSE, mat));
     checkGLcall("glUniformMatrix3fv");
diff --git a/dlls/wined3d/utils.c b/dlls/wined3d/utils.c
index 06ff97ce8f..ef469100fd 100644
--- a/dlls/wined3d/utils.c
+++ b/dlls/wined3d/utils.c
@@ -6713,3 +6713,297 @@ BOOL wined3d_array_reserve(void **elements, SIZE_T *capacity, SIZE_T count, SIZE
     *capacity = new_capacity;
     return TRUE;
 }
+
+static void swap_rows(float **a, float **b)
+{
+    float *tmp = *a;
+
+    *a = *b;
+    *b = tmp;
+}
+
+BOOL invert_matrix(struct wined3d_matrix *out, const struct wined3d_matrix *m)
+{
+    float wtmp[4][8];
+    float m0, m1, m2, m3, s;
+    float *r0, *r1, *r2, *r3;
+
+    r0 = wtmp[0];
+    r1 = wtmp[1];
+    r2 = wtmp[2];
+    r3 = wtmp[3];
+
+    r0[0] = m->_11;
+    r0[1] = m->_12;
+    r0[2] = m->_13;
+    r0[3] = m->_14;
+    r0[4] = 1.0f;
+    r0[5] = r0[6] = r0[7] = 0.0f;
+
+    r1[0] = m->_21;
+    r1[1] = m->_22;
+    r1[2] = m->_23;
+    r1[3] = m->_24;
+    r1[5] = 1.0f;
+    r1[4] = r1[6] = r1[7] = 0.0f;
+
+    r2[0] = m->_31;
+    r2[1] = m->_32;
+    r2[2] = m->_33;
+    r2[3] = m->_34;
+    r2[6] = 1.0f;
+    r2[4] = r2[5] = r2[7] = 0.0f;
+
+    r3[0] = m->_41;
+    r3[1] = m->_42;
+    r3[2] = m->_43;
+    r3[3] = m->_44;
+    r3[7] = 1.0f;
+    r3[4] = r3[5] = r3[6] = 0.0f;
+
+    /* Choose pivot - or die. */
+    if (fabsf(r3[0]) > fabsf(r2[0]))
+        swap_rows(&r3, &r2);
+    if (fabsf(r2[0]) > fabsf(r1[0]))
+        swap_rows(&r2, &r1);
+    if (fabsf(r1[0]) > fabsf(r0[0]))
+        swap_rows(&r1, &r0);
+    if (r0[0] == 0.0f)
+        return FALSE;
+
+    /* Eliminate first variable. */
+    m1 = r1[0] / r0[0]; m2 = r2[0] / r0[0]; m3 = r3[0] / r0[0];
+    s = r0[1]; r1[1] -= m1 * s; r2[1] -= m2 * s; r3[1] -= m3 * s;
+    s = r0[2]; r1[2] -= m1 * s; r2[2] -= m2 * s; r3[2] -= m3 * s;
+    s = r0[3]; r1[3] -= m1 * s; r2[3] -= m2 * s; r3[3] -= m3 * s;
+    s = r0[4];
+    if (s != 0.0f)
+    {
+        r1[4] -= m1 * s;
+        r2[4] -= m2 * s;
+        r3[4] -= m3 * s;
+    }
+    s = r0[5];
+    if (s != 0.0f)
+    {
+        r1[5] -= m1 * s;
+        r2[5] -= m2 * s;
+        r3[5] -= m3 * s;
+    }
+    s = r0[6];
+    if (s != 0.0f)
+    {
+        r1[6] -= m1 * s;
+        r2[6] -= m2 * s;
+        r3[6] -= m3 * s;
+    }
+    s = r0[7];
+    if (s != 0.0f)
+    {
+        r1[7] -= m1 * s;
+        r2[7] -= m2 * s;
+        r3[7] -= m3 * s;
+    }
+
+    /* Choose pivot - or die. */
+    if (fabsf(r3[1]) > fabsf(r2[1]))
+        swap_rows(&r3, &r2);
+    if (fabsf(r2[1]) > fabsf(r1[1]))
+        swap_rows(&r2, &r1);
+    if (r1[1] == 0.0f)
+        return FALSE;
+
+    /* Eliminate second variable. */
+    m2 = r2[1] / r1[1]; m3 = r3[1] / r1[1];
+    r2[2] -= m2 * r1[2]; r3[2] -= m3 * r1[2];
+    r2[3] -= m2 * r1[3]; r3[3] -= m3 * r1[3];
+    s = r1[4];
+    if (s != 0.0f)
+    {
+        r2[4] -= m2 * s;
+        r3[4] -= m3 * s;
+    }
+    s = r1[5];
+    if (s != 0.0f)
+    {
+        r2[5] -= m2 * s;
+        r3[5] -= m3 * s;
+    }
+    s = r1[6];
+    if (s != 0.0f)
+    {
+        r2[6] -= m2 * s;
+        r3[6] -= m3 * s;
+    }
+    s = r1[7];
+    if (s != 0.0f)
+    {
+        r2[7] -= m2 * s;
+        r3[7] -= m3 * s;
+    }
+
+    /* Choose pivot - or die. */
+    if (fabsf(r3[2]) > fabsf(r2[2]))
+        swap_rows(&r3, &r2);
+    if (r2[2] == 0.0f)
+        return FALSE;
+
+    /* Eliminate third variable. */
+    m3 = r3[2] / r2[2];
+    r3[3] -= m3 * r2[3];
+    r3[4] -= m3 * r2[4];
+    r3[5] -= m3 * r2[5];
+    r3[6] -= m3 * r2[6];
+    r3[7] -= m3 * r2[7];
+
+    /* Last check. */
+    if (r3[3] == 0.0f)
+        return FALSE;
+
+    /* Back substitute row 3. */
+    s = 1.0f / r3[3];
+    r3[4] *= s;
+    r3[5] *= s;
+    r3[6] *= s;
+    r3[7] *= s;
+
+    /* Back substitute row 2. */
+    m2 = r2[3];
+    s = 1.0f / r2[2];
+    r2[4] = s * (r2[4] - r3[4] * m2);
+    r2[5] = s * (r2[5] - r3[5] * m2);
+    r2[6] = s * (r2[6] - r3[6] * m2);
+    r2[7] = s * (r2[7] - r3[7] * m2);
+    m1 = r1[3];
+    r1[4] -= r3[4] * m1;
+    r1[5] -= r3[5] * m1;
+    r1[6] -= r3[6] * m1;
+    r1[7] -= r3[7] * m1;
+    m0 = r0[3];
+    r0[4] -= r3[4] * m0;
+    r0[5] -= r3[5] * m0;
+    r0[6] -= r3[6] * m0;
+    r0[7] -= r3[7] * m0;
+
+    /* Back substitute row 1. */
+    m1 = r1[2];
+    s = 1.0f / r1[1];
+    r1[4] = s * (r1[4] - r2[4] * m1);
+    r1[5] = s * (r1[5] - r2[5] * m1);
+    r1[6] = s * (r1[6] - r2[6] * m1);
+    r1[7] = s * (r1[7] - r2[7] * m1);
+    m0 = r0[2];
+    r0[4] -= r2[4] * m0;
+    r0[5] -= r2[5] * m0;
+    r0[6] -= r2[6] * m0;
+    r0[7] -= r2[7] * m0;
+
+    /* Back substitute row 0. */
+    m0 = r0[1];
+    s = 1.0f / r0[0];
+    r0[4] = s * (r0[4] - r1[4] * m0);
+    r0[5] = s * (r0[5] - r1[5] * m0);
+    r0[6] = s * (r0[6] - r1[6] * m0);
+    r0[7] = s * (r0[7] - r1[7] * m0);
+
+    out->_11 = r0[4];
+    out->_12 = r0[5];
+    out->_13 = r0[6];
+    out->_14 = r0[7];
+    out->_21 = r1[4];
+    out->_22 = r1[5];
+    out->_23 = r1[6];
+    out->_24 = r1[7];
+    out->_31 = r2[4];
+    out->_32 = r2[5];
+    out->_33 = r2[6];
+    out->_34 = r2[7];
+    out->_41 = r3[4];
+    out->_42 = r3[5];
+    out->_43 = r3[6];
+    out->_44 = r3[7];
+
+    return TRUE;
+}
+
+/* Taken and adapted from Mesa. */
+static BOOL invert_matrix_3d(struct wined3d_matrix *out, const struct wined3d_matrix *in)
+{
+    float pos, neg, t, det;
+    struct wined3d_matrix temp;
+
+    /* Calculate the determinant of upper left 3x3 submatrix and
+     * determine if the matrix is singular. */
+    pos = neg = 0.0f;
+    t =  in->_11 * in->_22 * in->_33;
+    if (t >= 0.0f)
+        pos += t;
+    else
+        neg += t;
+
+    t =  in->_21 * in->_32 * in->_13;
+    if (t >= 0.0f)
+        pos += t;
+    else
+        neg += t;
+    t =  in->_31 * in->_12 * in->_23;
+    if (t >= 0.0f)
+        pos += t;
+    else
+        neg += t;
+
+    t = -in->_31 * in->_22 * in->_13;
+    if (t >= 0.0f)
+        pos += t;
+    else
+        neg += t;
+    t = -in->_21 * in->_12 * in->_33;
+    if (t >= 0.0f)
+        pos += t;
+    else
+        neg += t;
+
+    t = -in->_11 * in->_32 * in->_23;
+    if (t >= 0.0f)
+        pos += t;
+    else
+        neg += t;
+
+    det = pos + neg;
+
+    if (fabsf(det) < 1e-25f)
+        return FALSE;
+
+    det = 1.0f / det;
+    temp._11 =  (in->_22 * in->_33 - in->_32 * in->_23) * det;
+    temp._12 = -(in->_12 * in->_33 - in->_32 * in->_13) * det;
+    temp._13 =  (in->_12 * in->_23 - in->_22 * in->_13) * det;
+    temp._21 = -(in->_21 * in->_33 - in->_31 * in->_23) * det;
+    temp._22 =  (in->_11 * in->_33 - in->_31 * in->_13) * det;
+    temp._23 = -(in->_11 * in->_23 - in->_21 * in->_13) * det;
+    temp._31 =  (in->_21 * in->_32 - in->_31 * in->_22) * det;
+    temp._32 = -(in->_11 * in->_32 - in->_31 * in->_12) * det;
+    temp._33 =  (in->_11 * in->_22 - in->_21 * in->_12) * det;
+
+    *out = temp;
+    return TRUE;
+}
+
+void compute_normal_matrix(float *normal_matrix, BOOL legacy_lighting,
+        const struct wined3d_matrix *modelview)
+{
+    struct wined3d_matrix mv;
+    unsigned int i, j;
+
+    mv = *modelview;
+    if (legacy_lighting)
+        invert_matrix_3d(&mv, &mv);
+    else
+        invert_matrix(&mv, &mv);
+    /* Tests show that singular modelview matrices are used unchanged as normal
+     * matrices on D3D3 and older. There seems to be no clearly consistent
+     * behavior on newer D3D versions so always follow older ddraw behavior. */
+    for (i = 0; i < 3; ++i)
+        for (j = 0; j < 3; ++j)
+            normal_matrix[i * 3 + j] = (&mv._11)[j * 4 + i];
+}
diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h
index d59d466cb3..42a70891d0 100644
--- a/dlls/wined3d/wined3d_private.h
+++ b/dlls/wined3d/wined3d_private.h
@@ -4860,6 +4860,11 @@ static inline enum wined3d_material_color_source validate_material_colour_source
     return WINED3D_MCS_MATERIAL;
 }
 
+BOOL invert_matrix(struct wined3d_matrix *out, const struct wined3d_matrix *m) DECLSPEC_HIDDEN;
+
+void compute_normal_matrix(float *normal_matrix, BOOL legacy_lighting,
+        const struct wined3d_matrix *modelview) DECLSPEC_HIDDEN;
+
 /* The WNDCLASS-Name for the fake window which we use to retrieve the GL capabilities */
 #define WINED3D_OPENGL_WINDOW_CLASS_NAME "WineD3D_OpenGL"
 
-- 
2.21.0

[PATCH 05/11] wined3d: Factor out compute_normal_matrix() function.

[Henri Verbeet]

Signed-off-by: Henri Verbeet <hverbeet(a)codeweavers.com>

[PATCH 06/11] wined3d: Move multiply_vector_matrix() to wined3d_private.h.

[Paul Gofman]

Signed-off-by: Paul Gofman <gofmanp(a)gmail.com>
---
 dlls/wined3d/glsl_shader.c     | 13 -------------
 dlls/wined3d/wined3d_private.h | 13 +++++++++++++
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/dlls/wined3d/glsl_shader.c b/dlls/wined3d/glsl_shader.c
index 5684e187b1..6fe2b3fc88 100644
--- a/dlls/wined3d/glsl_shader.c
+++ b/dlls/wined3d/glsl_shader.c
@@ -1374,19 +1374,6 @@ static void shader_glsl_ffp_vertex_lightambient_uniform(const struct wined3d_con
     checkGLcall("glUniform3fv");
 }
 
-static void multiply_vector_matrix(struct wined3d_vec4 *dest, const struct wined3d_vec4 *src1,
-        const struct wined3d_matrix *src2)
-{
-    struct wined3d_vec4 temp;
-
-    temp.x = (src1->x * src2->_11) + (src1->y * src2->_21) + (src1->z * src2->_31) + (src1->w * src2->_41);
-    temp.y = (src1->x * src2->_12) + (src1->y * src2->_22) + (src1->z * src2->_32) + (src1->w * src2->_42);
-    temp.z = (src1->x * src2->_13) + (src1->y * src2->_23) + (src1->z * src2->_33) + (src1->w * src2->_43);
-    temp.w = (src1->x * src2->_14) + (src1->y * src2->_24) + (src1->z * src2->_34) + (src1->w * src2->_44);
-
-    *dest = temp;
-}
-
 static void shader_glsl_ffp_vertex_light_uniform(const struct wined3d_context *context,
         const struct wined3d_state *state, unsigned int light, const struct wined3d_light_info *light_info,
         struct glsl_shader_prog_link *prog)
diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h
index 42a70891d0..9616f31513 100644
--- a/dlls/wined3d/wined3d_private.h
+++ b/dlls/wined3d/wined3d_private.h
@@ -4860,6 +4860,19 @@ static inline enum wined3d_material_color_source validate_material_colour_source
     return WINED3D_MCS_MATERIAL;
 }
 
+static inline void multiply_vector_matrix(struct wined3d_vec4 *dest, const struct wined3d_vec4 *src1,
+        const struct wined3d_matrix *src2)
+{
+    struct wined3d_vec4 temp;
+
+    temp.x = (src1->x * src2->_11) + (src1->y * src2->_21) + (src1->z * src2->_31) + (src1->w * src2->_41);
+    temp.y = (src1->x * src2->_12) + (src1->y * src2->_22) + (src1->z * src2->_32) + (src1->w * src2->_42);
+    temp.z = (src1->x * src2->_13) + (src1->y * src2->_23) + (src1->z * src2->_33) + (src1->w * src2->_43);
+    temp.w = (src1->x * src2->_14) + (src1->y * src2->_24) + (src1->z * src2->_34) + (src1->w * src2->_44);
+
+    *dest = temp;
+}
+
 BOOL invert_matrix(struct wined3d_matrix *out, const struct wined3d_matrix *m) DECLSPEC_HIDDEN;
 
 void compute_normal_matrix(float *normal_matrix, BOOL legacy_lighting,
-- 
2.21.0

[PATCH 07/11] wined3d: Implement lighting with directional lights in process_vertices_strided().

[Paul Gofman]

Signed-off-by: Paul Gofman <gofmanp(a)gmail.com>
---
 dlls/ddraw/tests/ddraw1.c      |   4 +-
 dlls/wined3d/device.c          | 296 +++++++++++++++++++++++++++++++--
 dlls/wined3d/wined3d_private.h |   1 +
 3 files changed, 284 insertions(+), 17 deletions(-)

diff --git a/dlls/ddraw/tests/ddraw1.c b/dlls/ddraw/tests/ddraw1.c
index 7dda6eba20..9525b2dc0c 100644
--- a/dlls/ddraw/tests/ddraw1.c
+++ b/dlls/ddraw/tests/ddraw1.c
@@ -5894,7 +5894,7 @@ static void test_material(void)
         ok(SUCCEEDED(hr), "Failed to end scene, hr %#x.\n", hr);
         color = get_surface_color(rt, 320, 240);
         if (test_data[i].material)
-            todo_wine ok(compare_color(color, test_data[i].expected_color, 1)
+            ok(compare_color(color, test_data[i].expected_color, 1)
                     /* The Windows 8 testbot appears to return undefined results. */
                     || broken(TRUE),
                     "Got unexpected color 0x%08x, test %u.\n", color, i);
@@ -6273,7 +6273,7 @@ static void test_lighting(void)
         ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
 
         color = get_surface_color(rt, 320, 240);
-        todo_wine ok(color == tests[i].expected, "%s has color 0x%08x.\n", tests[i].message, color);
+        ok(color == tests[i].expected, "%s has color 0x%08x.\n", tests[i].message, color);
     }
 
     IDirect3DExecuteBuffer_Release(execute_buffer);
diff --git a/dlls/wined3d/device.c b/dlls/wined3d/device.c
index 938c420f5b..2f1ff4f5b5 100644
--- a/dlls/wined3d/device.c
+++ b/dlls/wined3d/device.c
@@ -3107,6 +3107,11 @@ static void color_from_mcs(struct wined3d_color *color, enum wined3d_material_co
     wined3d_color_from_d3dcolor(color, element ? *(const DWORD *)(element->data.addr + index * element->stride) : 0);
 }
 
+static float dot_vec3(const struct wined3d_vec3 *src1, const struct wined3d_vec3 *src2)
+{
+    return src1->x * src2->x + src1->y * src2->y + src1->z * src2->z;
+}
+
 static float clamp(float value, float min_value, float max_value)
 {
     return value < min_value ? min_value : value > max_value ? max_value : value;
@@ -3121,17 +3126,212 @@ static void clamp_vec(float *dst, const float *src, unsigned int count,
         dst[i] = clamp(src[i], min_value, max_value);
 }
 
+static void normalize_vec3(struct wined3d_vec3 *dest, const struct wined3d_vec3 *src)
+{
+    float rnorm = 1.0f / sqrtf(src->x * src->x + src->y * src->y + src->z * src->z);
+
+    *dest = *src;
+    if (isfinite(rnorm))
+    {
+        dest->x *= rnorm;
+        dest->y *= rnorm;
+        dest->z *= rnorm;
+    }
+}
+
+static void madd_vec(float *dst, const float *src, unsigned int count, float c)
+{
+    unsigned int i;
+
+    for (i = 0; i < count; ++i)
+        dst[i] += src[i] * c;
+}
+
+static void multiply_vector3_matrix(struct wined3d_vec3 *dest, const struct wined3d_vec3 *src1,
+        const float *src2)
+{
+    struct wined3d_vec3 temp;
+
+    temp.x = (src1->x * src2[0]) + (src1->y * src2[3]) + (src1->z * src2[3 * 2]);
+    temp.y = (src1->x * src2[1]) + (src1->y * src2[3 + 1]) + (src1->z * src2[3 * 2 + 1]);
+    temp.z = (src1->x * src2[2]) + (src1->y * src2[3 + 2]) + (src1->z * src2[3 * 2 + 2]);
+
+    *dest = temp;
+}
+
+struct light_transformed
+{
+    struct wined3d_color diffuse, specular, ambient;
+    struct wined3d_vec4 position;
+    struct wined3d_vec3 direction;
+    float range, falloff, c_att, l_att, q_att, cos_htheta, cos_hphi;
+};
+
+struct lights_settings
+{
+    struct light_transformed lights[WINED3D_MAX_SOFTWARE_ACTIVE_LIGHTS];
+    struct wined3d_color ambient_light;
+    struct wined3d_matrix modelview_matrix;
+    float normal_matrix[3 * 3];
+
+    DWORD point_light_count          : 4;
+    DWORD spot_light_count           : 4;
+    DWORD directional_light_count    : 4;
+    DWORD parallel_point_light_count : 4;
+    DWORD legacy_lighting            : 1;
+    DWORD normalize                  : 1;
+    DWORD localviewer                : 1;
+    DWORD padding                    : 13;
+};
+
+static void init_transformed_lights(struct lights_settings *ls, const struct wined3d_state *state,
+        BOOL legacy_lighting)
+{
+    const struct wined3d_light_info *lights[WINED3D_MAX_SOFTWARE_ACTIVE_LIGHTS];
+    const struct wined3d_light_info *light_info;
+    struct light_transformed *light;
+    unsigned int lights_count;
+    struct wined3d_vec4 vec4;
+    unsigned int i, index;
+
+    memset(ls, 0, sizeof(*ls));
+
+    wined3d_color_from_d3dcolor(&ls->ambient_light, state->render_states[WINED3D_RS_AMBIENT]);
+    ls->legacy_lighting = !!legacy_lighting;
+    ls->normalize = !!state->render_states[WINED3D_RS_NORMALIZENORMALS];
+    ls->localviewer = !!state->render_states[WINED3D_RS_LOCALVIEWER];
+
+    multiply_matrix(&ls->modelview_matrix, &state->transforms[WINED3D_TS_VIEW],
+            &state->transforms[WINED3D_TS_WORLD_MATRIX(0)]);
+    compute_normal_matrix(ls->normal_matrix, legacy_lighting, &ls->modelview_matrix);
+
+    index = 0;
+    for (i = 0; i < LIGHTMAP_SIZE && index < WINED3D_MAX_SOFTWARE_ACTIVE_LIGHTS; ++i)
+    {
+        LIST_FOR_EACH_ENTRY(light_info, &state->light_state.light_map[i], struct wined3d_light_info, entry)
+        {
+            if (!light_info->enabled)
+                continue;
+
+            switch (light_info->OriginalParms.type)
+            {
+                case WINED3D_LIGHT_DIRECTIONAL:
+                    ++ls->directional_light_count;
+                    break;
+                default:
+                    FIXME("Unhandled light type %#x.\n", light_info->OriginalParms.type);
+                    continue;
+            }
+            lights[index] = light_info;
+            if (++index == WINED3D_MAX_SOFTWARE_ACTIVE_LIGHTS)
+                break;
+        }
+    }
+
+    lights_count = index;
+    index = 0;
+    for (i = 0; i < lights_count; ++i)
+    {
+        light_info = lights[i];
+        if (light_info->OriginalParms.type != WINED3D_LIGHT_DIRECTIONAL)
+            continue;
+
+        light = &ls->lights[index];
+        multiply_vector_matrix(&vec4, &light_info->direction, &state->transforms[WINED3D_TS_VIEW]);
+        normalize_vec3(&light->direction, (const struct wined3d_vec3 *)&vec4);
+
+        light->diffuse = light_info->OriginalParms.diffuse;
+        light->ambient = light_info->OriginalParms.ambient;
+        light->specular = light_info->OriginalParms.specular;
+        ++index;
+    }
+}
+
+static void update_light_diffuse_specular(struct wined3d_color *diffuse, struct wined3d_color *specular,
+        const struct wined3d_vec3 *dir, float att, float material_shininess,
+        const struct wined3d_vec3 *normal_transformed,
+        const struct wined3d_vec3 *position_transformed_normalized,
+        const struct light_transformed *light, const struct lights_settings *ls)
+{
+    struct wined3d_vec3 vec3;
+    float t, c;
+
+    c = clamp(dot_vec3(dir, normal_transformed), 0.0f, 1.0f);
+    madd_vec(&diffuse->r, &light->diffuse.r, 3, c * att);
+
+    if (ls->localviewer)
+    {
+        vec3.x = dir->x - position_transformed_normalized->x;
+        vec3.y = dir->y - position_transformed_normalized->y;
+        vec3.z = dir->z - position_transformed_normalized->z;
+    }
+    else
+    {
+        vec3.x = dir->x;
+        vec3.y = dir->y;
+        vec3.z = dir->z - 1.0f;
+    }
+    normalize_vec3(&vec3, &vec3);
+    t = dot_vec3(normal_transformed, &vec3);
+    if (t > 0.0f && (!ls->legacy_lighting || material_shininess > 0.0f)
+            && dot_vec3(dir, normal_transformed) > 0.0f)
+        madd_vec(&specular->r, &light->specular.r, 4, att * powf(t, material_shininess));
+}
+
+static void compute_light(struct wined3d_color *ambient, struct wined3d_color *diffuse,
+        struct wined3d_color *specular, const struct lights_settings *ls, const struct wined3d_vec3 *normal,
+        const struct wined3d_vec4 *position, float material_shininess)
+{
+    struct wined3d_vec3 position_transformed_normalized;
+    struct wined3d_vec3 normal_transformed = {0.0f};
+    struct wined3d_vec4 position_transformed;
+    const struct light_transformed *light;
+    unsigned int i, index;
+    float rcp_w;
+
+    multiply_vector_matrix(&position_transformed, position, &ls->modelview_matrix);
+    rcp_w = 1.0f / position_transformed.w;
+    position_transformed.x *= rcp_w;
+    position_transformed.y *= rcp_w;
+    position_transformed.z *= rcp_w;
+    position_transformed.w = 1.0f;
+    normalize_vec3(&position_transformed_normalized, (const struct wined3d_vec3 *)&position_transformed);
+
+    if (normal)
+    {
+        multiply_vector3_matrix(&normal_transformed, normal, ls->normal_matrix);
+        if (ls->normalize)
+            normalize_vec3(&normal_transformed, &normal_transformed);
+    }
+
+    diffuse->r = diffuse->g = diffuse->b = diffuse->a = 0.0f;
+    *specular = *diffuse;
+    *ambient = ls->ambient_light;
+
+    index = 0;
+    for (i = 0; i < ls->directional_light_count; ++i, ++index)
+    {
+        light = &ls->lights[index];
+
+        madd_vec(&ambient->r, &light->ambient.r, 3, 1.0f);
+        if (normal)
+            update_light_diffuse_specular(diffuse, specular, &light->direction, 1.0f, material_shininess,
+                    &normal_transformed, &position_transformed_normalized, light, ls);
+    }
+}
+
 /* Context activation is done by the caller. */
 #define copy_and_next(dest, src, size) memcpy(dest, src, size); dest += (size)
 static HRESULT process_vertices_strided(const struct wined3d_device *device, DWORD dwDestIndex, DWORD dwCount,
         const struct wined3d_stream_info *stream_info, struct wined3d_buffer *dest, DWORD flags, DWORD dst_fvf)
 {
-    enum wined3d_material_color_source diffuse_source, specular_source;
+    enum wined3d_material_color_source ambient_source, diffuse_source, emissive_source, specular_source;
     struct wined3d_matrix mat, proj_mat, view_mat, world_mat;
     const struct wined3d_state *state = &device->state;
     struct wined3d_map_desc map_desc;
     struct wined3d_box box = {0};
     struct wined3d_viewport vp;
+    struct lights_settings ls;
     unsigned int vertex_size;
     BOOL doClip, lighting;
     DWORD numTextures;
@@ -3139,11 +3339,6 @@ static HRESULT process_vertices_strided(const struct wined3d_device *device, DWO
     BYTE *dest_ptr;
     HRESULT hr;
 
-    if (stream_info->use_map & (1u << WINED3D_FFP_NORMAL))
-    {
-        WARN(" lighting state not saved yet... Some strange stuff may happen !\n");
-    }
-
     if (!(stream_info->use_map & (1u << WINED3D_FFP_POSITION)))
     {
         ERR("Source has no position mask.\n");
@@ -3221,13 +3416,19 @@ static HRESULT process_vertices_strided(const struct wined3d_device *device, DWO
         {
             diffuse_source = validate_material_colour_source(stream_info->use_map,
                     state->render_states[WINED3D_RS_DIFFUSEMATERIALSOURCE]);
+            ambient_source = validate_material_colour_source(stream_info->use_map,
+                    state->render_states[WINED3D_RS_AMBIENTMATERIALSOURCE]);
+            emissive_source = validate_material_colour_source(stream_info->use_map,
+                    state->render_states[WINED3D_RS_EMISSIVEMATERIALSOURCE]);
             specular_source = validate_material_colour_source(stream_info->use_map,
                     state->render_states[WINED3D_RS_SPECULARMATERIALSOURCE]);
         }
         else
         {
-            diffuse_source = specular_source = WINED3D_MCS_MATERIAL;
+            ambient_source = diffuse_source = emissive_source = specular_source = WINED3D_MCS_MATERIAL;
         }
+        init_transformed_lights(&ls, state, device->adapter->d3d_info.wined3d_creation_flags
+                & WINED3D_LEGACY_FFP_LIGHTING);
     }
     else
     {
@@ -3236,13 +3437,15 @@ static HRESULT process_vertices_strided(const struct wined3d_device *device, DWO
     }
 
     for (i = 0; i < dwCount; i+= 1) {
+        struct wined3d_color ambient, diffuse, specular;
+        const struct wined3d_stream_info_element *position_element
+                = &stream_info->elements[WINED3D_FFP_POSITION];
+        const float *p = (const float *)(position_element->data.addr + i * position_element->stride);
         unsigned int tex_index;
 
         if ( ((dst_fvf & WINED3DFVF_POSITION_MASK) == WINED3DFVF_XYZ ) ||
              ((dst_fvf & WINED3DFVF_POSITION_MASK) == WINED3DFVF_XYZRHW ) ) {
             /* The position first */
-            const struct wined3d_stream_info_element *element = &stream_info->elements[WINED3D_FFP_POSITION];
-            const float *p = (const float *)(element->data.addr + i * element->stride);
             float x, y, z, rhw;
             TRACE("In: ( %06.2f %06.2f %06.2f )\n", p[0], p[1], p[2]);
 
@@ -3350,27 +3553,90 @@ static HRESULT process_vertices_strided(const struct wined3d_device *device, DWO
             copy_and_next(dest_ptr, normal, 3 * sizeof(float));
         }
 
+        if (lighting)
+        {
+            const struct wined3d_stream_info_element *element;
+            struct wined3d_vec4 position;
+            struct wined3d_vec3 *normal;
+
+            position.x = p[0];
+            position.y = p[1];
+            position.z = p[2];
+            position.w = 1.0f;
+
+            if (stream_info->use_map & (1u << WINED3D_FFP_NORMAL))
+            {
+                element = &stream_info->elements[WINED3D_FFP_NORMAL];
+                normal = (struct wined3d_vec3 *)(element->data.addr + i * element->stride);
+            }
+            else
+            {
+                normal = NULL;
+            }
+            compute_light(&ambient, &diffuse, &specular, &ls, normal, &position,
+                    state->render_states[WINED3D_RS_SPECULARENABLE] ? state->material.power : 0.0f);
+        }
+
         if (dst_fvf & WINED3DFVF_DIFFUSE)
         {
-            struct wined3d_color material_diffuse;
+            struct wined3d_color material_diffuse, material_ambient, material_emissive, diffuse_color;
 
             color_from_mcs(&material_diffuse, diffuse_source, &state->material.diffuse, i, stream_info);
 
-            clamp_vec(&material_diffuse.r, &material_diffuse.r, 4, 0.0f, 1.0f);
-            *((DWORD *)dest_ptr) = d3dcolor_from_wined3d_color(&material_diffuse);
+            if (lighting)
+            {
+                color_from_mcs(&material_ambient, ambient_source, &state->material.ambient, i, stream_info);
+                color_from_mcs(&material_emissive, emissive_source, &state->material.emissive, i, stream_info);
+
+                diffuse_color.a = material_diffuse.a;
+                diffuse_color.r = ambient.r * material_ambient.r
+                        + diffuse.r * material_diffuse.r + material_emissive.r;
+                diffuse_color.g = ambient.g * material_ambient.g
+                        + diffuse.g * material_diffuse.g + material_emissive.g;
+                diffuse_color.b = ambient.b * material_ambient.b
+                        + diffuse.b * material_diffuse.b + material_emissive.b;
+            }
+            else
+            {
+                diffuse_color = material_diffuse;
+            }
+            clamp_vec(&diffuse_color.r, &diffuse_color.r, 4, 0.0f, 1.0f);
+            *((DWORD *)dest_ptr) = d3dcolor_from_wined3d_color(&diffuse_color);
             dest_ptr += sizeof(DWORD);
         }
 
         if (dst_fvf & WINED3DFVF_SPECULAR)
         {
-            struct wined3d_color material_specular;
+            struct wined3d_color material_specular, specular_color;
 
             memset(&material_specular, 0, sizeof(material_specular));
             color_from_mcs(&material_specular, specular_source, state->render_states[WINED3D_RS_SPECULARENABLE]
                     ? &state->material.specular : &material_specular, i, stream_info);
 
-            clamp_vec(&material_specular.r, &material_specular.r, 4, 0.0f, 1.0f);
-            *((DWORD *)dest_ptr) = d3dcolor_from_wined3d_color(&material_specular);
+            if (lighting)
+            {
+                specular_color.a = specular.a * material_specular.a;
+                specular_color.r = specular.r * material_specular.r;
+                specular_color.g = specular.g * material_specular.g;
+                specular_color.b = specular.b * material_specular.b;
+                if (state->render_states[WINED3D_RS_FOGENABLE])
+                {
+                    static BOOL warned;
+
+                    if (!warned)
+                    {
+                        FIXME("Fog factor is not implemented.\n");
+                        warned = TRUE;
+                    }
+                    specular_color.a = 1.0f;
+                }
+            }
+            else
+            {
+                specular_color = material_specular;
+            }
+            clamp_vec(&specular_color.r, &specular_color.r, 4, 0.0f, 1.0f);
+            *((DWORD *)dest_ptr) = d3dcolor_from_wined3d_color(&specular_color);
             dest_ptr += sizeof(DWORD);
         }
 
diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h
index 9616f31513..06c085e053 100644
--- a/dlls/wined3d/wined3d_private.h
+++ b/dlls/wined3d/wined3d_private.h
@@ -270,6 +270,7 @@ static inline enum complex_fixup get_complex_fixup(struct color_fixup_desc fixup
 #define WINED3D_MAX_VERTEX_SAMPLERS 4
 #define WINED3D_MAX_COMBINED_SAMPLERS (WINED3D_MAX_FRAGMENT_SAMPLERS + WINED3D_MAX_VERTEX_SAMPLERS)
 #define WINED3D_MAX_ACTIVE_LIGHTS   8
+#define WINED3D_MAX_SOFTWARE_ACTIVE_LIGHTS 32
 #define WINED3D_MAX_CLIP_DISTANCES  8
 #define MAX_CONSTANT_BUFFERS        15
 #define MAX_SAMPLER_OBJECTS         16
-- 
2.21.0

[PATCH 07/11] wined3d: Implement lighting with directional lights in process_vertices_strided().

[Marvin]

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=52287

Your paranoid android.


=== wvistau64_zh_CN (32 bit report) ===

ddraw:
ddraw1.c:2763: Test failed: Expected message 0x46, but didn't receive it.
ddraw1.c:2765: Test failed: Expected screen size 1024x768, got 0x0.
ddraw1.c:2771: Test failed: Expected (0,0)-(1024,768), got (-8,-8)-(1032,745).
ddraw1.c:2801: Test failed: Expected (0,0)-(1024,768), got (-8,-8)-(1032,745).
ddraw1.c:2808: Test failed: Expected (0,0)-(1024,768), got (-8,-8)-(1032,745).
ddraw1.c:2834: Test failed: Expected (0,0)-(1024,768), got (-8,-8)-(1032,745).
ddraw1.c:2857: Test failed: Expected (0,0)-(1024,768), got (-8,-8)-(1032,745).
ddraw1.c:2886: Test failed: Expected (0,0)-(1024,768), got (-8,-8)-(1032,745).
ddraw1.c:2912: Test failed: Expected (0,0)-(1024,768), got (-8,-8)-(1032,745).
ddraw1.c:2932: Test failed: Expected (0,0)-(1024,768), got (-8,-8)-(1032,745).
ddraw1.c:2968: Test failed: Expected (0,0)-(1024,768), got (-8,-8)-(1032,745).
ddraw1.c:2978: Test failed: Expected (0,0)-(1024,768), got (-8,-8)-(1032,745).
ddraw1.c:3004: Test failed: Expected (0,0)-(1024,768), got (-8,-8)-(1032,745).
ddraw1.c:3027: Test failed: Expected (0,0)-(1024,768), got (-8,-8)-(1032,745).
ddraw1.c:3049: Test failed: Expected (0,0)-(1024,768), got (-8,-8)-(1032,745).
ddraw1.c:3075: Test failed: Expected (0,0)-(1024,768), got (-8,-8)-(1032,745).
ddraw1.c:3095: Test failed: Expected (0,0)-(1024,768), got (-8,-8)-(1032,745).
ddraw1.c:3132: Test failed: Expected (0,0)-(1024,768), got (-8,-8)-(1032,745).

=== debian9 (build log) ===

error: patch failed: dlls/ddraw/tests/ddraw1.c:6273
Task: Patch failed to apply

=== debian9 (build log) ===

error: patch failed: dlls/ddraw/tests/ddraw1.c:6273
Task: Patch failed to apply

[PATCH 07/11] wined3d: Implement lighting with directional lights in process_vertices_strided().

[Henri Verbeet]

On Wed, 15 May 2019 at 19:40, Paul Gofman <gofmanp(a)gmail.com> wrote:
> +static float dot_vec3(const struct wined3d_vec3 *src1, const struct wined3d_vec3 *src2)
> +{
> +    return src1->x * src2->x + src1->y * src2->y + src1->z * src2->z;
> +}
> We'd typically call that "wined3d_vec3_dot()".
> +static void normalize_vec3(struct wined3d_vec3 *dest, const struct wined3d_vec3 *src)
> +{
> +    float rnorm = 1.0f / sqrtf(src->x * src->x + src->y * src->y + src->z * src->z);
> "sqrtf(wined3d_vec3_dot(src, src))", right?
> +static void madd_vec(float *dst, const float *src, unsigned int count, float c)
> +{
> +    unsigned int i;
> +
> +    for (i = 0; i < count; ++i)
> +        dst[i] += src[i] * c;
> +}
> +
> +static void multiply_vector3_matrix(struct wined3d_vec3 *dest, const struct wined3d_vec3 *src1,
> +        const float *src2)
> +{
> +    struct wined3d_vec3 temp;
> +
> +    temp.x = (src1->x * src2[0]) + (src1->y * src2[3]) + (src1->z * src2[3 * 2]);
> +    temp.y = (src1->x * src2[1]) + (src1->y * src2[3 + 1]) + (src1->z * src2[3 * 2 + 1]);
> +    temp.z = (src1->x * src2[2]) + (src1->y * src2[3 + 2]) + (src1->z * src2[3 * 2 + 2]);
> +
> +    *dest = temp;
> +}
> I think we'd prefer proper matrix and vector structures here.

[PATCH 08/11] ddraw/tests: Backport test_specular_lighting() for ddraw1.

[Paul Gofman]

Signed-off-by: Paul Gofman <gofmanp(a)gmail.com>
---
 dlls/ddraw/tests/ddraw1.c | 285 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 285 insertions(+)

diff --git a/dlls/ddraw/tests/ddraw1.c b/dlls/ddraw/tests/ddraw1.c
index 9525b2dc0c..cb8b09e4ec 100644
--- a/dlls/ddraw/tests/ddraw1.c
+++ b/dlls/ddraw/tests/ddraw1.c
@@ -415,6 +415,27 @@ static void emit_tquad_tlist(void **ptr, WORD base_idx)
     *ptr = tri;
 }
 
+static void emit_tri_indices(void **ptr, WORD *indices, unsigned int primitive_count)
+{
+    D3DINSTRUCTION *inst = *ptr;
+    D3DTRIANGLE *tri = (D3DTRIANGLE *)(inst + 1);
+    unsigned int i;
+
+    inst->bOpcode = D3DOP_TRIANGLE;
+    inst->bSize = sizeof(*tri);
+    inst->wCount = primitive_count;
+
+    for (i = 0; i < primitive_count; ++i)
+    {
+        U1(*tri).v1 = indices[i * 3];
+        U2(*tri).v2 = indices[i * 3 + 1];
+        U3(*tri).v3 = indices[i * 3 + 2];
+        tri->wFlags = D3DTRIFLAG_START;
+        ++tri;
+    }
+    *ptr = tri;
+}
+
 static void emit_texture_load(void **ptr, D3DTEXTUREHANDLE dst_texture,
         D3DTEXTUREHANDLE src_texture)
 {
@@ -679,6 +700,22 @@ static IDirect3DMaterial *create_emissive_material(IDirect3DDevice *device, floa
     return create_material(device, &mat);
 }
 
+static IDirect3DMaterial *create_specular_material(IDirect3DDevice *device,
+        float r, float g, float b, float a, float power)
+{
+    D3DMATERIAL mat;
+
+    memset(&mat, 0, sizeof(mat));
+    mat.dwSize = sizeof(mat);
+    U1(U2(mat).specular).r = r;
+    U2(U2(mat).specular).g = g;
+    U3(U2(mat).specular).b = b;
+    U4(U2(mat).specular).a = a;
+    U4(mat).power = power;
+
+    return create_material(device, &mat);
+}
+
 static void destroy_material(IDirect3DMaterial *material)
 {
     IDirect3DMaterial_Release(material);
@@ -6294,6 +6331,253 @@ static void test_lighting(void)
     DestroyWindow(window);
 }
 
+static void test_specular_lighting(void)
+{
+    static const unsigned int vertices_side = 5;
+    const unsigned int indices_count = (vertices_side - 1) * (vertices_side - 1) * 2 * 3;
+    const unsigned int vertex_count = vertices_side * vertices_side;
+    static D3DRECT clear_rect = {{0}, {0}, {640}, {480}};
+    static D3DMATRIX mat =
+    {
+        1.0f, 0.0f, 0.0f, 0.0f,
+        0.0f, 1.0f, 0.0f, 0.0f,
+        0.0f, 0.0f, 1.0f, 0.0f,
+        0.0f, 0.0f, 0.0f, 1.0f,
+    };
+    /* Use of D3DLIGHT2 instead of D3DLIGHT is intentional. Using D3DLIGHT
+       without dwFlags looks broken on Windows 7: directional light behaves
+       as if _LOCALVIEWER state is on, specular lights do not work at all and
+       always output zero colors. */
+    static D3DLIGHT2 directional =
+    {
+        sizeof(D3DLIGHT2),
+        D3DLIGHT_DIRECTIONAL,
+        {{1.0f}, {1.0f}, {1.0f}, {0.0f}},
+        {{0.0f}, {0.0f}, {0.0f}},
+        {{0.0f}, {0.0f}, {1.0f}},
+    };
+    static const struct expected_color
+    {
+        unsigned int x, y;
+        D3DCOLOR color;
+    }
+    expected_directional_local[] =
+    {
+        {160, 120, 0x003c3c3c},
+        {320, 120, 0x00717171},
+        {480, 120, 0x003c3c3c},
+        {160, 240, 0x00717171},
+        {320, 240, 0x00ffffff},
+        {480, 240, 0x00717171},
+        {160, 360, 0x003c3c3c},
+        {320, 360, 0x00717171},
+        {480, 360, 0x003c3c3c},
+    },
+    expected_zero[] =
+    {
+        {160, 120, 0x00000000},
+        {320, 120, 0x00000000},
+        {480, 120, 0x00000000},
+        {160, 240, 0x00000000},
+        {320, 240, 0x00000000},
+        {480, 240, 0x00000000},
+        {160, 360, 0x00000000},
+        {320, 360, 0x00000000},
+        {480, 360, 0x00000000},
+    };
+    static const struct
+    {
+        D3DLIGHT2 *light;
+        float specular_power;
+        const struct expected_color *expected;
+        unsigned int expected_count;
+    }
+    tests[] =
+    {
+        {&directional, 30.0f, expected_directional_local, ARRAY_SIZE(expected_directional_local)},
+        {&directional, 0.0f, expected_zero, ARRAY_SIZE(expected_zero)},
+    };
+
+    D3DMATRIXHANDLE world_handle, view_handle, proj_handle;
+    IDirect3DMaterial *material, *background_material;
+    IDirect3DExecuteBuffer *execute_buffer;
+    D3DEXECUTEBUFFERDESC exec_desc;
+    D3DMATERIALHANDLE mat_handle;
+    IDirect3DViewport *viewport;
+    unsigned int i, j, x, y;
+    IDirect3DDevice *device;
+    IDirectDrawSurface *rt;
+    IDirect3DLight *light;
+    IDirectDraw*ddraw;
+    UINT inst_length;
+    D3DVERTEX *quad;
+    IDirect3D *d3d;
+    D3DCOLOR color;
+    ULONG refcount;
+    WORD *indices;
+    HWND window;
+    HRESULT hr;
+    void *ptr;
+
+    window = create_window();
+    ddraw = create_ddraw();
+    ok(!!ddraw, "Failed to create a ddraw object.\n");
+    if (!(device = create_device(ddraw, window, DDSCL_NORMAL)))
+    {
+        skip("Failed to create a 3D device, skipping test.\n");
+        IDirectDraw_Release(ddraw);
+        DestroyWindow(window);
+        return;
+    }
+
+    quad = HeapAlloc(GetProcessHeap(), 0, vertex_count * sizeof(*quad));
+    indices = HeapAlloc(GetProcessHeap(), 0, indices_count * sizeof(*indices));
+    for (i = 0, y = 0; y < vertices_side; ++y)
+    {
+        for (x = 0; x < vertices_side; ++x)
+        {
+            U1(quad[i]).x = x * 2.0f / (vertices_side - 1) - 1.0f;
+            U2(quad[i]).y = y * 2.0f / (vertices_side - 1) - 1.0f;
+            U3(quad[i]).z = 1.0f;
+            U4(quad[i]).nx = 0.0f;
+            U5(quad[i]).ny = 0.0f;
+            U6(quad[i]).nz = -1.0f;
+            U7(quad[i]).tu = 0.0f;
+            U8(quad[i++]).tv = 0.0f;
+        }
+    }
+    for (i = 0, y = 0; y < (vertices_side - 1); ++y)
+    {
+        for (x = 0; x < (vertices_side - 1); ++x)
+        {
+            indices[i++] = y * vertices_side + x + 1;
+            indices[i++] = y * vertices_side + x;
+            indices[i++] = (y + 1) * vertices_side + x;
+            indices[i++] = y * vertices_side + x + 1;
+            indices[i++] = (y + 1) * vertices_side + x;
+            indices[i++] = (y + 1) * vertices_side + x + 1;
+        }
+    }
+
+    hr = IDirect3DDevice_GetDirect3D(device, &d3d);
+    ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
+
+    hr = IDirect3DDevice_QueryInterface(device, &IID_IDirectDrawSurface, (void **)&rt);
+    ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
+
+    viewport = create_viewport(device, 0, 0, 640, 480);
+    background_material = create_diffuse_material(device, 1.0f, 0.0f, 0.0f, 1.0f);
+    viewport_set_background(device, viewport, background_material);
+
+    hr = IDirect3DViewport_Clear(viewport, 1, &clear_rect, D3DCLEAR_TARGET);
+    ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
+
+    hr = IDirect3DDevice_CreateMatrix(device, &world_handle);
+    ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
+    hr = IDirect3DDevice_SetMatrix(device, world_handle, &mat);
+    ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
+    hr = IDirect3DDevice_CreateMatrix(device, &view_handle);
+    ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
+    hr = IDirect3DDevice_SetMatrix(device, view_handle, &mat);
+    ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
+    hr = IDirect3DDevice_CreateMatrix(device, &proj_handle);
+    ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
+    hr = IDirect3DDevice_SetMatrix(device, proj_handle, &mat);
+    ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
+
+
+    hr = IDirect3D_CreateLight(d3d, &light, NULL);
+    ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
+    hr = IDirect3DViewport_AddLight(viewport, light);
+    ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
+
+    memset(&exec_desc, 0, sizeof(exec_desc));
+    exec_desc.dwSize = sizeof(exec_desc);
+    exec_desc.dwFlags = D3DDEB_BUFSIZE | D3DDEB_CAPS;
+    exec_desc.dwBufferSize = 10240;
+    exec_desc.dwCaps = D3DDEBCAPS_SYSTEMMEMORY;
+
+    hr = IDirect3DDevice_CreateExecuteBuffer(device, &exec_desc, &execute_buffer, NULL);
+    ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
+
+    for (i = 0; i < ARRAY_SIZE(tests); ++i)
+    {
+        tests[i].light->dwFlags = D3DLIGHT_ACTIVE;
+        material = create_specular_material(device, 1.0f, 1.0f, 1.0f, 1.0f, tests[i].specular_power);
+        hr = IDirect3DMaterial_GetHandle(material, device, &mat_handle);
+        ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
+
+        hr = IDirect3DLight_SetLight(light, (D3DLIGHT *)tests[i].light);
+        ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
+
+        hr = IDirect3DViewport_Clear(viewport, 1, &clear_rect, D3DCLEAR_TARGET);
+        ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
+
+        hr = IDirect3DDevice_BeginScene(device);
+        ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
+
+        hr = IDirect3DExecuteBuffer_Lock(execute_buffer, &exec_desc);
+        ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
+
+        memcpy(exec_desc.lpData, quad, sizeof(*quad) * vertex_count);
+        ptr = ((BYTE *)exec_desc.lpData) + sizeof(*quad) * vertex_count;
+
+        emit_set_ls(&ptr, D3DLIGHTSTATE_MATERIAL, mat_handle);
+        emit_set_ts(&ptr, D3DTRANSFORMSTATE_WORLD, world_handle);
+        emit_set_ts(&ptr, D3DTRANSFORMSTATE_VIEW, view_handle);
+        emit_set_ts(&ptr, D3DTRANSFORMSTATE_PROJECTION, proj_handle);
+        emit_set_rs(&ptr, D3DRENDERSTATE_ZENABLE, FALSE);
+        emit_set_rs(&ptr, D3DRENDERSTATE_SPECULARENABLE, TRUE);
+
+        emit_process_vertices(&ptr, D3DPROCESSVERTICES_TRANSFORMLIGHT, 0, vertex_count);
+        emit_tri_indices(&ptr, indices, indices_count / 3);
+        emit_end(&ptr);
+        inst_length = (BYTE *)ptr - (BYTE *)exec_desc.lpData;
+        ok(inst_length <= exec_desc.dwBufferSize, "Execute buffer overflow, size %u.\n", inst_length);
+        inst_length -= sizeof(*quad) * vertex_count;
+
+        hr = IDirect3DExecuteBuffer_Unlock(execute_buffer);
+        ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
+
+        set_execute_data(execute_buffer, vertex_count, sizeof(*quad) * vertex_count, inst_length);
+        hr = IDirect3DDevice_Execute(device, execute_buffer, viewport, D3DEXECUTE_CLIPPED);
+        ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
+
+        hr = IDirect3DDevice_EndScene(device);
+        ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
+
+        for (j = 0; j < tests[i].expected_count; ++j)
+        {
+            color = get_surface_color(rt, tests[i].expected[j].x, tests[i].expected[j].y);
+            ok(compare_color(color, tests[i].expected[j].color, 1),
+                    "Expected color 0x%08x at location (%u, %u), got 0x%08x, case %u.\n",
+                    tests[i].expected[j].color, tests[i].expected[j].x,
+                    tests[i].expected[j].y, color, i);
+        }
+        destroy_material(material);
+    }
+
+    IDirect3DExecuteBuffer_Release(execute_buffer);
+    IDirect3DDevice_DeleteMatrix(device, world_handle);
+    IDirect3DDevice_DeleteMatrix(device, view_handle);
+    IDirect3DDevice_DeleteMatrix(device, proj_handle);
+
+    hr = IDirect3DViewport_DeleteLight(viewport, light);
+    ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
+    IDirect3DLight_Release(light);
+    destroy_material(background_material);
+    destroy_viewport(device, viewport);
+    IDirectDrawSurface_Release(rt);
+    refcount = IDirect3DDevice_Release(device);
+    ok(!refcount, "Device has %u references left.\n", refcount);
+    IDirect3D_Release(d3d);
+    refcount = IDirectDraw_Release(ddraw);
+    ok(!refcount, "Ddraw object has %u references left.\n", refcount);
+    DestroyWindow(window);
+    HeapFree(GetProcessHeap(), 0, indices);
+    HeapFree(GetProcessHeap(), 0, quad);
+}
+
 static void test_palette_gdi(void)
 {
     IDirectDrawSurface *surface, *primary;
@@ -12552,6 +12836,7 @@ START_TEST(ddraw1)
     test_p8_blit();
     test_material();
     test_lighting();
+    test_specular_lighting();
     test_palette_gdi();
     test_palette_alpha();
     test_lost_device();
-- 
2.21.0

[PATCH 08/11] ddraw/tests: Backport test_specular_lighting() for ddraw1.

[Marvin]

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=52288

Your paranoid android.


=== w8 (32 bit report) ===

ddraw:
ddraw1.c:6552: Test failed: Expected color 0x003c3c3c at location (160, 120), got 0x00ff0000, case 0.
ddraw1.c:6552: Test failed: Expected color 0x00717171 at location (320, 120), got 0x00ff0000, case 0.
ddraw1.c:6552: Test failed: Expected color 0x003c3c3c at location (480, 120), got 0x00ff0000, case 0.
ddraw1.c:6552: Test failed: Expected color 0x00717171 at location (160, 240), got 0x00ff0000, case 0.
ddraw1.c:6552: Test failed: Expected color 0x00ffffff at location (320, 240), got 0x00ff0000, case 0.
ddraw1.c:6552: Test failed: Expected color 0x00717171 at location (480, 240), got 0x00ff0000, case 0.
ddraw1.c:6552: Test failed: Expected color 0x003c3c3c at location (160, 360), got 0x00ff0000, case 0.
ddraw1.c:6552: Test failed: Expected color 0x00717171 at location (320, 360), got 0x00ff0000, case 0.
ddraw1.c:6552: Test failed: Expected color 0x003c3c3c at location (480, 360), got 0x00ff0000, case 0.
ddraw1.c:6552: Test failed: Expected color 0x00000000 at location (160, 120), got 0x00ff0000, case 1.
ddraw1.c:6552: Test failed: Expected color 0x00000000 at location (320, 120), got 0x00ff0000, case 1.
ddraw1.c:6552: Test failed: Expected color 0x00000000 at location (480, 120), got 0x00ff0000, case 1.
ddraw1.c:6552: Test failed: Expected color 0x00000000 at location (160, 240), got 0x00ff0000, case 1.
ddraw1.c:6552: Test failed: Expected color 0x00000000 at location (320, 240), got 0x00ff0000, case 1.
ddraw1.c:6552: Test failed: Expected color 0x00000000 at location (480, 240), got 0x00ff0000, case 1.
ddraw1.c:6552: Test failed: Expected color 0x00000000 at location (160, 360), got 0x00ff0000, case 1.
ddraw1.c:6552: Test failed: Expected color 0x00000000 at location (320, 360), got 0x00ff0000, case 1.
ddraw1.c:6552: Test failed: Expected color 0x00000000 at location (480, 360), got 0x00ff0000, case 1.

=== w8adm (32 bit report) ===

ddraw:
ddraw1.c:6552: Test failed: Expected color 0x003c3c3c at location (160, 120), got 0x00ff0000, case 0.
ddraw1.c:6552: Test failed: Expected color 0x00717171 at location (320, 120), got 0x00ff0000, case 0.
ddraw1.c:6552: Test failed: Expected color 0x003c3c3c at location (480, 120), got 0x00ff0000, case 0.
ddraw1.c:6552: Test failed: Expected color 0x00717171 at location (160, 240), got 0x00ff0000, case 0.
ddraw1.c:6552: Test failed: Expected color 0x00ffffff at location (320, 240), got 0x00ff0000, case 0.
ddraw1.c:6552: Test failed: Expected color 0x00717171 at location (480, 240), got 0x00ff0000, case 0.
ddraw1.c:6552: Test failed: Expected color 0x003c3c3c at location (160, 360), got 0x00ff0000, case 0.
ddraw1.c:6552: Test failed: Expected color 0x00717171 at location (320, 360), got 0x00ff0000, case 0.
ddraw1.c:6552: Test failed: Expected color 0x003c3c3c at location (480, 360), got 0x00ff0000, case 0.
ddraw1.c:6552: Test failed: Expected color 0x00000000 at location (160, 120), got 0x00ff0000, case 1.
ddraw1.c:6552: Test failed: Expected color 0x00000000 at location (320, 120), got 0x00ff0000, case 1.
ddraw1.c:6552: Test failed: Expected color 0x00000000 at location (480, 120), got 0x00ff0000, case 1.
ddraw1.c:6552: Test failed: Expected color 0x00000000 at location (160, 240), got 0x00ff0000, case 1.
ddraw1.c:6552: Test failed: Expected color 0x00000000 at location (320, 240), got 0x00ff0000, case 1.
ddraw1.c:6552: Test failed: Expected color 0x00000000 at location (480, 240), got 0x00ff0000, case 1.
ddraw1.c:6552: Test failed: Expected color 0x00000000 at location (160, 360), got 0x00ff0000, case 1.
ddraw1.c:6552: Test failed: Expected color 0x00000000 at location (320, 360), got 0x00ff0000, case 1.
ddraw1.c:6552: Test failed: Expected color 0x00000000 at location (480, 360), got 0x00ff0000, case 1.
ddraw1.c:7421: Test failed: Got unexpected color 0x00000000.
ddraw1.c:7425: Test failed: Got unexpected color 0x00000000.
ddraw1.c:7488: Test failed: Got unexpected color 0x00000000.
ddraw1.c:7490: Test failed: Got unexpected color 0x00000000.

=== w864 (32 bit report) ===

ddraw:
ddraw1.c:6552: Test failed: Expected color 0x003c3c3c at location (160, 120), got 0x00ff0000, case 0.
ddraw1.c:6552: Test failed: Expected color 0x00717171 at location (320, 120), got 0x00ff0000, case 0.
ddraw1.c:6552: Test failed: Expected color 0x003c3c3c at location (480, 120), got 0x00ff0000, case 0.
ddraw1.c:6552: Test failed: Expected color 0x00717171 at location (160, 240), got 0x00ff0000, case 0.
ddraw1.c:6552: Test failed: Expected color 0x00ffffff at location (320, 240), got 0x00ff0000, case 0.
ddraw1.c:6552: Test failed: Expected color 0x00717171 at location (480, 240), got 0x00ff0000, case 0.
ddraw1.c:6552: Test failed: Expected color 0x003c3c3c at location (160, 360), got 0x00ff0000, case 0.
ddraw1.c:6552: Test failed: Expected color 0x00717171 at location (320, 360), got 0x00ff0000, case 0.
ddraw1.c:6552: Test failed: Expected color 0x003c3c3c at location (480, 360), got 0x00ff0000, case 0.
ddraw1.c:6552: Test failed: Expected color 0x00000000 at location (160, 120), got 0x00ff0000, case 1.
ddraw1.c:6552: Test failed: Expected color 0x00000000 at location (320, 120), got 0x00ff0000, case 1.
ddraw1.c:6552: Test failed: Expected color 0x00000000 at location (480, 120), got 0x00ff0000, case 1.
ddraw1.c:6552: Test failed: Expected color 0x00000000 at location (160, 240), got 0x00ff0000, case 1.
ddraw1.c:6552: Test failed: Expected color 0x00000000 at location (320, 240), got 0x00ff0000, case 1.
ddraw1.c:6552: Test failed: Expected color 0x00000000 at location (480, 240), got 0x00ff0000, case 1.
ddraw1.c:6552: Test failed: Expected color 0x00000000 at location (160, 360), got 0x00ff0000, case 1.
ddraw1.c:6552: Test failed: Expected color 0x00000000 at location (320, 360), got 0x00ff0000, case 1.
ddraw1.c:6552: Test failed: Expected color 0x00000000 at location (480, 360), got 0x00ff0000, case 1.

=== w1064v1809 (32 bit report) ===

ddraw:
ddraw1.c:6552: Test failed: Expected color 0x003c3c3c at location (160, 120), got 0x00ff0000, case 0.
ddraw1.c:6552: Test failed: Expected color 0x00717171 at location (320, 120), got 0x00ff0000, case 0.
ddraw1.c:6552: Test failed: Expected color 0x003c3c3c at location (480, 120), got 0x00ff0000, case 0.
ddraw1.c:6552: Test failed: Expected color 0x00717171 at location (160, 240), got 0x00ff0000, case 0.
ddraw1.c:6552: Test failed: Expected color 0x00ffffff at location (320, 240), got 0x00ff0000, case 0.
ddraw1.c:6552: Test failed: Expected color 0x00717171 at location (480, 240), got 0x00ff0000, case 0.
ddraw1.c:6552: Test failed: Expected color 0x003c3c3c at location (160, 360), got 0x00ff0000, case 0.
ddraw1.c:6552: Test failed: Expected color 0x00717171 at location (320, 360), got 0x00ff0000, case 0.
ddraw1.c:6552: Test failed: Expected color 0x003c3c3c at location (480, 360), got 0x00ff0000, case 0.
ddraw1.c:6552: Test failed: Expected color 0x00000000 at location (160, 120), got 0x00ff0000, case 1.
ddraw1.c:6552: Test failed: Expected color 0x00000000 at location (320, 120), got 0x00ff0000, case 1.
ddraw1.c:6552: Test failed: Expected color 0x00000000 at location (480, 120), got 0x00ff0000, case 1.
ddraw1.c:6552: Test failed: Expected color 0x00000000 at location (160, 240), got 0x00ff0000, case 1.
ddraw1.c:6552: Test failed: Expected color 0x00000000 at location (320, 240), got 0x00ff0000, case 1.
ddraw1.c:6552: Test failed: Expected color 0x00000000 at location (480, 240), got 0x00ff0000, case 1.
ddraw1.c:6552: Test failed: Expected color 0x00000000 at location (160, 360), got 0x00ff0000, case 1.
ddraw1.c:6552: Test failed: Expected color 0x00000000 at location (320, 360), got 0x00ff0000, case 1.
ddraw1.c:6552: Test failed: Expected color 0x00000000 at location (480, 360), got 0x00ff0000, case 1.

=== debian9 (build log) ===

error: patch failed: dlls/ddraw/tests/ddraw1.c:6273
Task: Patch failed to apply

=== debian9 (build log) ===

error: patch failed: dlls/ddraw/tests/ddraw1.c:6273
Task: Patch failed to apply

[PATCH 08/11] ddraw/tests: Backport test_specular_lighting() for ddraw1.

[Matteo Bruni]

Thanks for the test. I've had a similar one in my queue for a long
time but never got around to finishing it, mostly because I couldn't
understand the results I got.

On Wed, May 15, 2019 at 5:10 PM Paul Gofman <gofmanp(a)gmail.com> wrote:
> +    /* Use of D3DLIGHT2 instead of D3DLIGHT is intentional. Using D3DLIGHT
> +       without dwFlags looks broken on Windows 7: directional light behaves
> +       as if _LOCALVIEWER state is on, specular lights do not work at all and
> +       always output zero colors. */

Oh, interesting. BTW, did you actually mean "specular lights" here or
something else?
FWIW it looks like I used D3DLIGHT and somehow got a fully white quad
out of the directional light instead. Not sure where I tested it but
it was most likely Windows 7 Nvidia. Still, it might explain at least
some of the weird results I got.

Actually, let me attach my patch. In particular, see the large comment
block in there. I don't know how much of that is actually true and how
that's affected by D3DLIGHT vs D3DLIGHT2.

[PATCH 08/11] ddraw/tests: Backport test_specular_lighting() for ddraw1.

[Paul Gofman]

On 5/16/19 22:28, Matteo Bruni wrote:
> Oh, interesting. BTW, did you actually mean "specular lights" here or
> something else?
> FWIW it looks like I used D3DLIGHT and somehow got a fully white quad
> out of the directional light instead. Not sure where I tested it but
> it was most likely Windows 7 Nvidia. Still, it might explain at least
> some of the weird results I got.

Sorry, that's typo, actually 2 of them :(. First of all I meant point 
and spot lights, not "specular". I was getting zeroes when using them 
whatever I tried, I tried a lot of states and coordinate tweaking. It 
was not a background color, it did draw the triangles.

Regarding directional light, I was getting fully white too initially 
when I was using the test values from ddraw2+ tests. But later after 
tweaking some values (setting color values to 0.1 instead of 1.0) I 
found that it actually can output something else, and I matched that 
something else to the lighting computation with _LOCALVIEWER set to 
*off* (not "on" as my buggy comment). I initially suspected that ddraw1 
assumes _LOCALVIEWER should be off, but later testing with point lights 
and using D3DLIGHT2 structure made directional lights in a supposed way, 
same as ddraw2.

Now I suppose that it is likely some bug in Windows ddraw1, e. g. maybe 
it is using the flags which are not in D3DLIGHT and gets mad from some 
random values (e. g., there is a documented flag not to set specular 
lighting with the light, or not to enable it at all).

[Marvin]

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=52281

Your paranoid android.


=== debian9 (32 bit report) ===

ddraw:
ddraw2.c:2792: Test failed: Expected message 0x46, but didn't receive it.
ddraw2.c:2794: Test failed: Expected screen size 1920x1200, got 0x0.
ddraw2.c:2799: Test failed: Expected (0,0)-(1920,1200), got (0,0)-(1920,1080).

[Lukáš Krejčí]

Hello,

just letting you know that your patch series claims to have 11 patches
but only 8 of them made it to the mailing list.

[Paul Gofman]

Yes, I am sorry for that. I intended to send 8 patches to reasonably 
limit the series, but erroneously generated 11 and thus the bogus "11". 
When I realized that they we sent aleady, and I thought that while it is 
annoying it is yet not a great issue so it does not worth it spamming 8 
patches again right away before I get any comments.

There is also a test failure on patch 8 on Testbot with WARP driver, 
this need to be fixed probably by adding broken WARP driver case, but 
since it is the last patch of 8 I thought it makes more sense to wait 
for any comments before sending the series again.

On 5/16/19 12:19, Lukáš Krejčí wrote:
> Hello,
>
> just letting you know that your patch series claims to have 11 patches
> but only 8 of them made it to the mailing list.

[Henri Verbeet]

Signed-off-by: Henri Verbeet <hverbeet(a)codeweavers.com>

[Henri Verbeet]

On Wed, 15 May 2019 at 19:37, Paul Gofman <gofmanp(a)gmail.com> wrote:
> @@ -712,9 +712,13 @@ static HRESULT WINAPI d3d_device1_Execute(IDirect3DDevice *iface,
>      if(!buffer)
>          return DDERR_INVALIDPARAMS;
>
> +    if (FAILED(hr = IDirect3DDevice3_SetCurrentViewport
> +            (&device->IDirect3DDevice3_iface, &viewport_impl->IDirect3DViewport3_iface)))
> +        return hr;
> +
> This is not an entirely new issue, but does remind me about the issue
> of how executing an execute buffer affects the device state. That's
> not an issue for devices created as version 1 devices, since there's
> no way to query the current state by the application. However, it's
> also possible to QI the version 1 interface from a version 2 or 3
> device.

[Paul Gofman]

On 5/16/19 21:01, Henri Verbeet wrote:
> On Wed, 15 May 2019 at 19:37, Paul Gofman <gofmanp(a)gmail.com> wrote:
> > @@ -712,9 +712,13 @@ static HRESULT WINAPI d3d_device1_Execute(IDirect3DDevice *iface,
> >       if(!buffer)
> >           return DDERR_INVALIDPARAMS;
> >
> > +    if (FAILED(hr = IDirect3DDevice3_SetCurrentViewport
> > +            (&device->IDirect3DDevice3_iface, &viewport_impl->IDirect3DViewport3_iface)))
> > +        return hr;
> > +
> > This is not an entirely new issue, but does remind me about the issue
> > of how executing an execute buffer affects the device state. That's
> > not an issue for devices created as version 1 devices, since there's
> > no way to query the current state by the application. However, it's
> > also possible to QI the version 1 interface from a version 2 or 3
> > device.

I don't know whether ExecuteBuffer should preserve the previous state or 
not, I didn't test that yet. But I should note that before this patch it 
was not preserving states as well, but it would also not deactivate the 
lights from previous viewport potentially set in the earlier call to 
ExecuteBuffer or some other way.

[Henri Verbeet]

On Thu, 16 May 2019 at 23:37, Paul Gofman <gofmanp(a)gmail.com> wrote:
> On 5/16/19 21:01, Henri Verbeet wrote:
> > On Wed, 15 May 2019 at 19:37, Paul Gofman <gofmanp(a)gmail.com> wrote:
> > > @@ -712,9 +712,13 @@ static HRESULT WINAPI d3d_device1_Execute(IDirect3DDevice *iface,
> > >       if(!buffer)
> > >           return DDERR_INVALIDPARAMS;
> > >
> > > +    if (FAILED(hr = IDirect3DDevice3_SetCurrentViewport
> > > +            (&device->IDirect3DDevice3_iface, &viewport_impl->IDirect3DViewport3_iface)))
> > > +        return hr;
> > > +
> > > This is not an entirely new issue, but does remind me about the issue
> > > of how executing an execute buffer affects the device state. That's
> > > not an issue for devices created as version 1 devices, since there's
> > > no way to query the current state by the application. However, it's
> > > also possible to QI the version 1 interface from a version 2 or 3
> > > device.
>
> I don't know whether ExecuteBuffer should preserve the previous state or
> not, I didn't test that yet. But I should note that before this patch it
> was not preserving states as well, but it would also not deactivate the
> lights from previous viewport potentially set in the earlier call to
> ExecuteBuffer or some other way.

Yeah, it doesn't directly impact this patch, but I did want to raise the issue.