[PATCH 2/2] d3drm: Implement IDirect3DRMViewport*::Clear.
Stefan Dösinger
stefandoesinger at gmail.com
Sat Aug 6 17:36:05 CDT 2016
On 2016-08-06 16:29, Aaryaman Vasishta wrote:
> Signed-off-by: Aaryaman Vasishta <jem456.vasishta at gmail.com>
> ---
> dlls/d3drm/tests/d3drm.c | 640 +++++++++++++++++++++++++++++++++++++++++++++++
> dlls/d3drm/viewport.c | 71 +++++-
> 2 files changed, 704 insertions(+), 7 deletions(-)
>
> diff --git a/dlls/d3drm/tests/d3drm.c b/dlls/d3drm/tests/d3drm.c
> index c4020e4..b9a83a0 100644
> --- a/dlls/d3drm/tests/d3drm.c
> +++ b/dlls/d3drm/tests/d3drm.c
> @@ -5852,6 +5852,644 @@ static void test_viewport_qi(void)
> IDirect3DRM_Release(d3drm1);
> }
>
> +static D3DCOLOR get_surface_color(IDirectDrawSurface *surface, UINT x, UINT y)
> +{
> + RECT rect = { x, y, x + 1, y + 1 };
> + DDSURFACEDESC surface_desc;
> + D3DCOLOR color;
> + HRESULT hr;
> +
> + memset(&surface_desc, 0, sizeof(surface_desc));
> + surface_desc.dwSize = sizeof(surface_desc);
> +
> + hr = IDirectDrawSurface_Lock(surface, &rect, &surface_desc, DDLOCK_READONLY | DDLOCK_WAIT, NULL);
> + ok(SUCCEEDED(hr), "Failed to lock surface, hr %#x.\n", hr);
> + if (FAILED(hr))
> + return 0xdeadbeef;
> +
> + color = *((DWORD *)surface_desc.lpSurface) & 0x00ffffff;
> +
> + hr = IDirectDrawSurface_Unlock(surface, NULL);
> + ok(SUCCEEDED(hr), "Failed to unlock surface, hr %#x.\n", hr);
> +
> + return color;
> +}
> +
> +static BOOL compare_color(D3DCOLOR c1, D3DCOLOR c2, BYTE max_diff)
> +{
> + if ((c1 & 0xff) - (c2 & 0xff) > max_diff) return FALSE;
> + c1 >>= 8; c2 >>= 8;
> + if ((c1 & 0xff) - (c2 & 0xff) > max_diff) return FALSE;
> + c1 >>= 8; c2 >>= 8;
> + if ((c1 & 0xff) - (c2 & 0xff) > max_diff) return FALSE;
> + c1 >>= 8; c2 >>= 8;
> + if ((c1 & 0xff) - (c2 & 0xff) > max_diff) return FALSE;
> + return TRUE;
> +}
> +
> +static void clear_depth_surface(IDirectDrawSurface *surface, DWORD value)
> +{
> + HRESULT hr;
> + DDBLTFX fx;
> +
> + memset(&fx, 0, sizeof(fx));
> + fx.dwSize = sizeof(fx);
> + U5(fx).dwFillDepth = value;
> +
> + hr = IDirectDrawSurface_Blt(surface, NULL, NULL, NULL, DDBLT_DEPTHFILL | DDBLT_WAIT, &fx);
> + ok(SUCCEEDED(hr), "Got unexpected hr %#x.\n", hr);
> +}
> +
> +static void set_execute_data(IDirect3DExecuteBuffer *execute_buffer, UINT vertex_count, UINT offset, UINT len)
> +{
> + D3DEXECUTEDATA exec_data;
> + HRESULT hr;
> +
> + memset(&exec_data, 0, sizeof(exec_data));
> + exec_data.dwSize = sizeof(exec_data);
> + exec_data.dwVertexCount = vertex_count;
> + exec_data.dwInstructionOffset = offset;
> + exec_data.dwInstructionLength = len;
> + hr = IDirect3DExecuteBuffer_SetExecuteData(execute_buffer, &exec_data);
> + ok(SUCCEEDED(hr), "Failed to set execute data, hr %#x.\n", hr);
> +}
> +
> +static void emit_set_ts(void **ptr, D3DTRANSFORMSTATETYPE state, DWORD value)
> +{
> + D3DINSTRUCTION *inst = *ptr;
> + D3DSTATE *ts = (D3DSTATE *)(inst + 1);
> +
> + inst->bOpcode = D3DOP_STATETRANSFORM;
> + inst->bSize = sizeof(*ts);
> + inst->wCount = 1;
> +
> + U1(*ts).dtstTransformStateType = state;
> + U2(*ts).dwArg[0] = value;
> +
> + *ptr = ts + 1;
> +}
> +
> +static void emit_set_rs(void **ptr, D3DRENDERSTATETYPE state, DWORD value)
> +{
> + D3DINSTRUCTION *inst = *ptr;
> + D3DSTATE *rs = (D3DSTATE *)(inst + 1);
> +
> + inst->bOpcode = D3DOP_STATERENDER;
> + inst->bSize = sizeof(*rs);
> + inst->wCount = 1;
> +
> + U1(*rs).drstRenderStateType = state;
> + U2(*rs).dwArg[0] = value;
> +
> + *ptr = rs + 1;
> +}
> +
> +static void emit_process_vertices(void **ptr, DWORD flags, WORD base_idx, DWORD vertex_count)
> +{
> + D3DINSTRUCTION *inst = *ptr;
> + D3DPROCESSVERTICES *pv = (D3DPROCESSVERTICES *)(inst + 1);
> +
> + inst->bOpcode = D3DOP_PROCESSVERTICES;
> + inst->bSize = sizeof(*pv);
> + inst->wCount = 1;
> +
> + pv->dwFlags = flags;
> + pv->wStart = base_idx;
> + pv->wDest = 0;
> + pv->dwCount = vertex_count;
> + pv->dwReserved = 0;
> +
> + *ptr = pv + 1;
> +}
> +
> +static void emit_tquad(void **ptr, WORD base_idx)
> +{
> + D3DINSTRUCTION *inst = *ptr;
> + D3DTRIANGLE *tri = (D3DTRIANGLE *)(inst + 1);
> +
> + inst->bOpcode = D3DOP_TRIANGLE;
> + inst->bSize = sizeof(*tri);
> + inst->wCount = 2;
> +
> + U1(*tri).v1 = base_idx;
> + U2(*tri).v2 = base_idx + 1;
> + U3(*tri).v3 = base_idx + 2;
> + tri->wFlags = D3DTRIFLAG_START;
> + ++tri;
> +
> + U1(*tri).v1 = base_idx + 2;
> + U2(*tri).v2 = base_idx + 1;
> + U3(*tri).v3 = base_idx + 3;
> + tri->wFlags = D3DTRIFLAG_ODD;
> + ++tri;
> +
> + *ptr = tri;
> +}
> +
> +static void emit_end(void **ptr)
> +{
> + D3DINSTRUCTION *inst = *ptr;
> +
> + inst->bOpcode = D3DOP_EXIT;
> + inst->bSize = 0;
> + inst->wCount = 0;
> +
> + *ptr = inst + 1;
> +}
> +
> +static void d3d_draw_quad1(IDirect3DDevice *device, IDirect3DViewport *viewport)
> +{
> + IDirect3DExecuteBuffer *execute_buffer;
> + D3DEXECUTEBUFFERDESC exec_desc;
> + HRESULT hr;
> + void *ptr;
> + UINT inst_length;
> + D3DMATRIXHANDLE world_handle, view_handle, proj_handle;
> + 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,
> + };
> + static const D3DLVERTEX quad_strip[] =
> + {
> + {{-1.0f}, {-1.0f}, {0.80f}, 0, {0xffbada55}},
> + {{-1.0f}, { 1.0f}, {0.80f}, 0, {0xffbada55}},
> + {{ 1.0f}, {-1.0f}, {0.80f}, 0, {0xffbada55}},
> + {{ 1.0f}, { 1.0f}, {0.80f}, 0, {0xffbada55}},
> + };
You could make this
{{-1.0f}, {-1.0f}, {0.0f}, 0, {0xffbada55}},
{{-1.0f}, { 1.0f}, {0.0f}, 0, {0xffbada55}},
{{ 1.0f}, {-1.0f}, {1.0f}, 0, {0xffbada55}},
{{ 1.0f}, { 1.0f}, {1.0f}, 0, {0xffbada55}},
Then your currently discarded draws will draw to ca x = 250. That way you can make sure the draw is really discarded by the depth test and not some other reason.
> + /* Check what happens if we release the depth surface that d3drm created, and clear the viewport */
> ...
> + hr = IDirect3DRMViewport_Clear(viewport1);
> + ok(SUCCEEDED(hr), "Cannot clear viewport (hr = %#x).\n", hr);
It's worth checking the color surface here to see if the clear was completely ignored or if it realized that there's no depth surface and it only cleared color. Also clear with a different color than what's left behind by the previous test.
> ...
> + d3d_draw_quad1(d3d_device1, d3d_viewport);
> +
> + ret_color = get_surface_color(surface, 100, 200);
> + todo_wine ok(compare_color(ret_color, 0x000000ff, 1), "Got unexpected color 0x%08x.\n", ret_color);
My reading of this is that the depth surface wasn't cleared (everything else would be very surprising). If the color surface was indeed cleared this shows that native at some place expects your ugly deed of removing the depth surface. If the color is not cleared it and the clear call is completely ignored I'd say something is broken somewhere and we shouldn't care too much about this corner case.
> + hr = IDirect3DRM_CreateViewport(d3drm1, device1, dummy_frame1, 0, 0, rc.right,
> + rc.bottom, &viewport1);
> + ok(SUCCEEDED(hr), "Cannot get IDirect3DRMViewport interface (hr = %#x)\n", hr);
> +
> +
> + /* Clear uses the scene/root frame's background color. */
Two newlines. Is that intentional?
> + hr = IDirect3DRMFrame_SetSceneBackgroundRGB(frame1, 1.0f, 1.0f, 1.0f);
> + ok(SUCCEEDED(hr), "Cannot set scene background RGB (hr = %#x)\n", hr);
> + ret_color = IDirect3DRMFrame_GetSceneBackground(frame1);
> + ok(ret_color == 0xffffffff, "Expected scene color returned == 0xffffffff, got %#x.\n", ret_color);
> + hr = IDirect3DRMFrame_SetSceneBackgroundRGB(dummy_frame1, 0.0f, 1.0f, 0.0f);
> + ok(SUCCEEDED(hr), "Cannot set scene background RGB (hr = %#x)\n", hr);
> + ret_color = IDirect3DRMFrame_GetSceneBackground(dummy_frame1);
> + ok(ret_color == 0xff00ff00, "Expected scene color returned == 0xff00ff00, got %#x.\n", ret_color);
> + hr = IDirect3DRMFrame_SetSceneBackgroundRGB(dummy_frame1, 0.0f, 0.0f, 1.0f);
> + ok(SUCCEEDED(hr), "Cannot set scene background RGB (hr = %#x)\n", hr);
> + ret_color = IDirect3DRMFrame_GetSceneBackground(dummy_frame1);
> + ok(ret_color == 0xff0000ff, "Expected scene color returned == 0xff0000ff, got %#x.\n", ret_color);
Can't the "where does the color come from" test be merged into the clear test at the start?
> +static void draw_quad2(IDirect3DDevice2 *device, IDirect3DViewport *viewport)
> +{
Do you need to draw with IDirect3DDevice2 directly here? What about QI'ing IDirect3DDevice and calling d3d_draw_quad1?
> diff --git a/dlls/d3drm/viewport.c b/dlls/d3drm/viewport.c
> index cb57fe0..371aae3 100644
> --- a/dlls/d3drm/viewport.c
> +++ b/dlls/d3drm/viewport.c
> @@ -39,6 +39,42 @@ static inline struct d3drm_viewport *impl_from_IDirect3DRMViewport2(IDirect3DRMV
> return CONTAINING_RECORD(iface, struct d3drm_viewport, IDirect3DRMViewport2_iface);
> }
>
> +static inline void d3drm_normalize_d3d_color(D3DCOLORVALUE *color_value, D3DCOLOR color)
> +{
> + color_value->r = RGBA_GETRED(color) / 255.0f;
> + color_value->g = RGBA_GETGREEN(color) / 255.0f;
> + color_value->b = RGBA_GETBLUE(color) / 255.0f;
> + color_value->a = RGBA_GETALPHA(color) / 255.0f;
> +}
> +
> +static HRESULT d3drm_update_background_material(struct d3drm_viewport *viewport)
> +{
> + IDirect3DRMFrame *root_frame;
> + D3DCOLOR color;
> + D3DMATERIAL mat;
> + D3DMATERIALHANDLE hmat;
> + HRESULT hr;
> +
> + if (FAILED(hr = IDirect3DRMFrame_GetScene(viewport->camera, &root_frame)))
> + return hr;
> + color = IDirect3DRMFrame_GetSceneBackground(root_frame);
You are leaking root_frame here.
> + if (FAILED(hr = IDirect3DMaterial_GetHandle(viewport->material, viewport->device->device, &hmat)))
> + return hr;
> +
> + if (FAILED(hr = IDirect3DViewport_SetBackground(viewport->d3d_viewport, hmat)))
> + return hr;
You can do these two calls during viewport creation, you don't have to repeat them every time.
> @@ -344,10 +380,7 @@ static HRESULT WINAPI d3drm_viewport2_Init(IDirect3DRMViewport2 *iface, IDirect3
>
> memset(&mat, 0, sizeof(mat));
> mat.dwSize = sizeof(mat);
> - mat.diffuse.r = RGBA_GETRED(color) / 255.0f;
> - mat.diffuse.g = RGBA_GETGREEN(color) / 255.0f;
> - mat.diffuse.b = RGBA_GETBLUE(color) / 255.0f;
> - mat.diffuse.a = RGBA_GETALPHA(color) / 255.0f;
> + d3drm_normalize_d3d_color(&mat.diffuse, color);
>
> if (FAILED(hr = IDirect3DMaterial_SetMaterial(material, &mat)))
> goto cleanup;
You can remove the IDirect3DMaterial::SetMaterial call from Init because you have to do it every time clear is called anyway.
Keep the GetHandle / SetBackground call in Init though to link the material with the viewport.
> static HRESULT WINAPI d3drm_viewport2_Clear(IDirect3DRMViewport2 *iface, DWORD flags)
> ...
> + if (flags & D3DRMCLEAR_ZBUFFER)
> + clear_flags |= D3DCLEAR_ZBUFFER;
I guess somewhere here or in the IDirect3DViewport::Clear implementation in ddraw you'll have to handle your missing depth buffer case.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20160806/5fbf8d43/attachment.sig>
More information about the wine-devel
mailing list