Re: [v2 6/6] d3dx9: implement state save and restore in effect.

[Matteo Bruni]

2016-03-01 18:10 GMT+01:00 Paul Gofman :
> Signed-off-by: Paul Gofman
>
> dlls/d3dx9_36/effect.c       | 66 ++++++++++++++++++++++++++++++++------------
>  dlls/d3dx9_36/tests/effect.c |  8 +++---
>  2 files changed, 53 insertions(+), 21 deletions(-)
>
> diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c
> index 8a359d0..a7611fe 100644
> --- a/dlls/d3dx9_36/effect.c
> +++ b/dlls/d3dx9_36/effect.c
> @@ -162,6 +162,8 @@ struct d3dx_technique
>  struct d3dx_parameter *annotations;
>  struct d3dx_pass *passes;
> -
> - struct IDirect3DStateBlock9 *saved_state;
>
> };
>
> struct param_table
> @@ -642,6 +644,12 @@ static void free_technique(struct d3dx_technique *technique)
>      if (!technique)
>          return;
> - if (technique->saved_state)
> - {
> -
>    IDirect3DStateBlock9_Release(technique->saved_state);
> -
>    technique->saved_state = NULL;
> - }
> -

What happens if the effect is released while inside a Begin() / End()
pair? Is the state restored or not?
>  if (technique->annotations)
>  {
>      for (i = 0; i < technique->annotation_count; ++i)
>
> @@ -3467,8 +3475,8 @@ static BOOL WINAPI ID3DXEffectImpl_IsParameterUsed(ID3DXEffect* iface, D3DXHANDL
>
> static HRESULT WINAPI ID3DXEffectImpl_Begin(ID3DXEffect *iface, UINT *passes, DWORD flags)
>  {
> - struct ID3DXEffectImpl *This = impl_from_ID3DXEffect(iface);
> - struct d3dx_technique *technique = This->active_technique;
> -
>
> struct ID3DXEffectImpl *effect = impl_from_ID3DXEffect(iface);
> -
>
> struct d3dx_technique *technique = effect->active_technique;
>
> TRACE("iface %p, passes %p, flags %#x.\n", iface, passes, flags);
>
> @@ -3477,18 +3485,34 @@ static HRESULT WINAPI ID3DXEffectImpl_Begin(ID3DXEffect *iface, UINT *passes, DW
>          if (flags & ~(D3DXFX_DONOTSAVESTATE | D3DXFX_DONOTSAVESAMPLERSTATE | D3DXFX_DONOTSAVESHADERSTATE))
>              WARN("Invalid flags (%#x) specified.\n", flags);
> -
>    if (This->manager || flags & D3DXFX_DONOTSAVESTATE)
> -
>    if (effect->manager || flags & D3DXFX_DONOTSAVESTATE)
>    {
>        TRACE("State capturing disabled.\n");
>    }
>    else
>    {
> -
>        FIXME("State capturing not supported, yet!\n");
> -
>        HRESULT hr;
> -
>        int i;
> -
> -
>        if (!technique->saved_state)
> -
>        {
> -
>            hr = IDirect3DDevice9_BeginStateBlock(effect->device);
> -
>            if (FAILED(hr))
> -
>                ERR("BeginStateBlock failed: %#x.\n",hr);
> -
>            for (i = 0; i < technique->pass_count; i++)
> -
>                d3dx9_apply_pass_states(effect, &technique->passes[i]);
> -
>            hr = IDirect3DDevice9_EndStateBlock(effect->device, &technique->saved_state);
> -
>            if (FAILED(hr))
> -
>                ERR("EndStateBlock failed: %#x.\n",hr);
> -
>        }
> -
>        hr = IDirect3DStateBlock9_Capture(technique->saved_state);
> -
>        if (FAILED(hr))
> -
>            ERR("StateBlock Capture failed: %#x.\n",hr);
>    }
>
>    *passes = technique->pass_count;
> -
>    This->started = TRUE;
> -
>    This->flags = flags;
> -
>    effect->started = TRUE;
> -
>    effect->flags = flags;
>
>    return D3D_OK;
>
> }
>
> @@ -3518,17 +3542,15 @@ static HRESULT WINAPI ID3DXEffectImpl_BeginPass(ID3DXEffect *iface, UINT pass)
>
> static HRESULT WINAPI ID3DXEffectImpl_CommitChanges(ID3DXEffect* iface)
>  {
> - struct ID3DXEffectImpl *This = impl_from_ID3DXEffect(iface);
> -
> - FIXME("(%p)->(): stub\n", This);
> - struct ID3DXEffectImpl *effect = impl_from_ID3DXEffect(iface);

Implementing CommitChanges should be a separate patch. Also please
turn the FIXME into a TRACE instead of dropping it entirely.
> - if (!This->active_pass)
> - if (!effect->active_pass)
> {
>     WARN("Called without an active pass.\n");
>     return D3D_OK;
> }
> -
> - return E_NOTIMPL;
> - /* TODO: apply only changed states */
> - return d3dx9_apply_pass_states(effect, effect->active_pass);
>
> }
>
> static HRESULT WINAPI ID3DXEffectImpl_EndPass(ID3DXEffect *iface)
> @@ -3550,23 +3572,33 @@ static HRESULT WINAPI ID3DXEffectImpl_EndPass(ID3DXEffect *iface)
>
> static HRESULT WINAPI ID3DXEffectImpl_End(ID3DXEffect *iface)
>  {
> - struct ID3DXEffectImpl *This = impl_from_ID3DXEffect(iface);
> -
>
> struct ID3DXEffectImpl *effect = impl_from_ID3DXEffect(iface);
> -
>
> struct d3dx_technique *technique = effect->active_technique;
>
> TRACE("iface %p.\n", iface);
> - if (!This->started)
> - if (!effect->started)
>     return D3D_OK;
> - if (This->manager || This->flags & D3DXFX_DONOTSAVESTATE)
> - if (effect->manager || effect->flags & D3DXFX_DONOTSAVESTATE)
> {
>     TRACE("State restoring disabled.\n");
> }
> else
> {
> -
>    FIXME("State restoring not supported, yet!\n");
> -
>    HRESULT hr;
> -
> -
>    if (technique && technique->saved_state)
> -
>    {
> -
>        hr = IDirect3DStateBlock9_Apply(technique->saved_state);
> -
>        if (FAILED(hr))
> -
>            ERR("State block apply failed: %#x.\n",hr);
> -
>    }
> -
>    else
> -
>        ERR("No saved state.");

Missing '\n'.
>  }
>
> - This->started = FALSE;
> -
>
> effect->started = FALSE;
>
> return D3D_OK;
>
> }
> diff --git a/dlls/d3dx9_36/tests/effect.c b/dlls/d3dx9_36/tests/effect.c
> index a052bce..3d0e01c 100644
> --- a/dlls/d3dx9_36/tests/effect.c
> +++ b/dlls/d3dx9_36/tests/effect.c
> @@ -2850,7 +2850,7 @@ static void test_effect_states(IDirect3DDevice9 *device)
>  hr = IDirect3DDevice9_GetRenderState(device, D3DRS_BLENDOP, &value);
>  ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK).\n", hr);
> - todo_wine ok(value == 1, "Got result %u, expected %u.\n", value, 1);
> - ok(value == 1, "Got result %u, expected %u.\n", value, 1);
> hr = IDirect3DDevice9_GetRenderState(device, D3DRS_ALPHAFUNC, &value);
> ok(value == 2, "Got result %u, expected %u.\n", value, 2);
>
> @@ -2862,7 +2862,7 @@ static void test_effect_states(IDirect3DDevice9 *device)
>      ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK).\n", hr);
>  hr = IDirect3DDevice9_GetLightEnable(device, 2, &bval);
> - todo_wine ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK).\n", hr);
> - ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK).\n", hr);
> if (hr == D3D_OK)
>     ok(!bval,"Got result %u, expected 0.", bval);
>
> @@ -2940,12 +2940,12 @@ static void test_effect_states(IDirect3DDevice9 *device)
>  hr = IDirect3DDevice9_GetTransform(device, D3DTS_WORLDMATRIX(1), &mat);
>  ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK).\n", hr);
> - todo_wine ok(!memcmp(mat.m, test_mat.m, sizeof(mat)), "World matrix not restored.\n");
> -
>
> ok(!memcmp(mat.m, test_mat.m, sizeof(mat)), "World matrix not restored.\n");
>
> hr = IDirect3DDevice9_GetLightEnable(device, 2, &bval);
> ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK).\n", hr);
> if (hr == D3D_OK)
> -
>    todo_wine ok(!bval,"Got result %u, expected 0.\n", bval);
> -
>    ok(!bval,"Got result %u, expected 0.\n", bval);
>
> if (effect)
>        effect->lpVtbl->Release(effect);
>
> --
> 2.5.0

[v2 6/6] d3dx9: implement state save and restore in effect.

[Paul Gofman]

On 03/02/2016 07:14 PM, Matteo Bruni wrote:
> What happens if the effect is released while inside a Begin() / End()
> pair? Is the state restored or not?

I will add such check to the same test case and will restore state on
effect release if test shows that this is required.