September 2017 - Wine-Devel

Re: [v3 3/3] d3dx9: Return E_FAIL from ValidateTechnique() for techniques with unsupported shaders.

by Marvin

Hi, While running your changed tests on Windows, 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=32937 Your paranoid android. === build (build) === Patch failed to apply

Sept. 1, 2017

Re: [v3 3/3] d3dx9: Return E_FAIL from ValidateTechnique() for techniques with unsupported shaders.

by Paul Gofman

Please ignore this patch for now (along with the previous version). I have discovered that there is still the case when object_id can be nonzero, but the object is never created (not initialized array of shaders). So we should keep the creation_failed flag probably. I will send the test for null shaders and the new patch version in the next series. On 09/01/2017 01:56 PM, Paul Gofman wrote: > Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> > --- > v3: > - use object_id != 0 as the indication of non-null shader. > --- > dlls/d3dx9_36/effect.c | 45 +++++++++++++++++++++++++++++++++++++++++--- > dlls/d3dx9_36/tests/effect.c | 3 --- > 2 files changed, 42 insertions(+), 6 deletions(-) > > diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c > index 2cd0c22f64..ef772d99b5 100644 > --- a/dlls/d3dx9_36/effect.c > +++ b/dlls/d3dx9_36/effect.c > @@ -3941,11 +3941,50 @@ static D3DXHANDLE WINAPI ID3DXEffectImpl_GetCurrentTechnique(ID3DXEffect *iface) > > static HRESULT WINAPI ID3DXEffectImpl_ValidateTechnique(ID3DXEffect* iface, D3DXHANDLE technique) > { > - struct ID3DXEffectImpl *This = impl_from_ID3DXEffect(iface); > + struct ID3DXEffectImpl *effect = impl_from_ID3DXEffect(iface); > + struct d3dx9_base_effect *base = &effect->base_effect; > + struct d3dx_technique *tech = get_valid_technique(base, technique); > + HRESULT ret = D3D_OK; > + unsigned int i, j; > > - FIXME("(%p)->(%p): stub\n", This, technique); > + FIXME("iface %p, technique %p semi-stub.\n", iface, technique); > > - return D3D_OK; > + if (!tech) > + { > + ret = D3DERR_INVALIDCALL; > + goto done; > + } > + for (i = 0; i < tech->pass_count; ++i) > + { > + struct d3dx_pass *pass = &tech->passes[i]; > + > + for (j = 0; j < pass->state_count; ++j) > + { > + struct d3dx_state *state = &pass->states[j]; > + > + if (state_table[state->operation].class == SC_VERTEXSHADER > + || state_table[state->operation].class == SC_PIXELSHADER) > + { > + struct d3dx_parameter *param; > + void *param_value; > + BOOL param_dirty; > + HRESULT hr; > + > + if (FAILED(hr = d3dx9_get_param_value_ptr(pass, &pass->states[j], &param_value, &param, > + FALSE, &param_dirty))) > + return hr; > + > + if (param->object_id && !*(void **)param->data) > + { > + ret = E_FAIL; > + goto done; > + } > + } > + } > + } > +done: > + TRACE("Returning %#x.\n", ret); > + return ret; > } > > static HRESULT WINAPI ID3DXEffectImpl_FindNextValidTechnique(ID3DXEffect* iface, D3DXHANDLE technique, D3DXHANDLE* next_technique) > diff --git a/dlls/d3dx9_36/tests/effect.c b/dlls/d3dx9_36/tests/effect.c > index bb26fe4e51..7260411c39 100644 > --- a/dlls/d3dx9_36/tests/effect.c > +++ b/dlls/d3dx9_36/tests/effect.c > @@ -7090,10 +7090,8 @@ static void test_effect_unsupported_shader(void) > ok(hr == D3D_OK, "Got result %#x.\n", hr); > > hr = effect->lpVtbl->ValidateTechnique(effect, "missing_technique"); > - todo_wine > ok(hr == D3DERR_INVALIDCALL, "Got result %#x.\n", hr); > hr = effect->lpVtbl->ValidateTechnique(effect, "tech0"); > - todo_wine > ok(hr == E_FAIL, "Got result %#x.\n", hr); > > hr = effect->lpVtbl->ValidateTechnique(effect, "tech1"); > @@ -7101,7 +7099,6 @@ static void test_effect_unsupported_shader(void) > effect->lpVtbl->SetInt(effect, "i", 1); > ok(hr == D3D_OK, "Got result %#x.\n", hr); > hr = effect->lpVtbl->ValidateTechnique(effect, "tech1"); > - todo_wine > ok(hr == E_FAIL, "Got result %#x.\n", hr); > effect->lpVtbl->SetInt(effect, "i", 0); > hr = effect->lpVtbl->ValidateTechnique(effect, "tech1");

Sept. 1, 2017

Re: [v2 3/3] d3dx9: Return E_FAIL from ValidateTechnique() for techniques with unsupported shaders.

by Paul Gofman

On 09/01/2017 02:37 AM, Matteo Bruni wrote: > 2017-08-31 20:56 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > >> + if (!param->object_id || base->objects[param->object_id].creation_failed) > It should be possible to replace this with a check for !*(void **)param->data. > Unless you mean to use the flag for something else later on? > > I explored how can I make a null shader in effect, and found that it is possible by just assigning 'null' to VertexShader in fx file. The important thing is, object gets id 0 which we now treat as an error. So I suggest to: - change this line to 'if (param->object_id && !*(void **)param->data)', don't add creation_failed flag, as it is now clear how to properly distinguish null shader from an unsupported one; - after that add a separate test with null shader (concerning GetPassDesc and ValidateTechnique) as the next patch and fix d3dx9_base_effect_get_pass_desc() which also thinks object_id == 0 is a problem.

Sept. 1, 2017

Re: [PATCH] d2d1: Partially implement StrokeContainsPoint() for rectangle geometry

by Nikolay Sivov

On 01.09.2017 1:25, Henri Verbeet wrote: > On 31 August 2017 at 14:00, Nikolay Sivov <nsivov(a)codeweavers.com> wrote: >> - if (transform) >> - { >> - if (!d2d_matrix_invert(&g_i, transform)) >> - return D2DERR_UNSUPPORTED_OPERATION; >> - d2d_point_transform(&point, &g_i, point.x, point.y); >> - } >> + if (FAILED(hr = geometry_inv_transform_point(transform, &point))) >> + return hr; > That's fine in principle, but please do it in a separate commit, and > please follow the existing conventions. E.g.: > > if (transform && FAILED(hr = d2d_point_transform_inverse(&point, > transform, point.x, point.y))) > return hr; Sure. > >> + stroke_width /= 2.0f; >> + >> + rect = geometry->u.rectangle.rect; >> + rect.left -= stroke_width; >> + rect.right += stroke_width; >> + rect.top -= stroke_width; >> + rect.bottom += stroke_width; >> + >> + dx = max(fabsf((rect.right + rect.left) / 2.0f - point.x) - (rect.right - rect.left) / 2.0f, 0.0f); >> + dy = max(fabsf((rect.bottom + rect.top) / 2.0f - point.y) - (rect.bottom - rect.top) / 2.0f, 0.0f); >> + >> + if ((*contains = tolerance * tolerance > (dx * dx + dy * dy))) >> + { >> + rect = geometry->u.rectangle.rect; >> + stroke_width += tolerance; >> + rect.left += stroke_width; >> + rect.right -= stroke_width; >> + rect.top += stroke_width; >> + rect.bottom -= stroke_width; >> + >> + *contains = rect.left >= point.x || rect.right <= point.x || rect.top >= point.y || rect.bottom <= point.y; >> + } > I think that looks a little more complicated than it needs to be. How about: > > D2D1_POINT_2F d, s; > ... > s.x = rect.right - rect.left; > s.y = rect.bottom - rect.top; > d.x = fabsf((rect.right + rect.left) * 0.5f - point.x); > d.y = fabsf((rect.bottom + rect.top) * 0.5f - point.y); > > if (d.x < (s.x - stroke_width) * 0.5f - tolerance && d.y < (s.y - > stroke_width) * 0.5f - tolerance) > { > *contains = FALSE; > return S_OK; > } > > d.x = max(d.x - (s.x + stroke_width) * 0.5f, 0.0f); > d.y = max(d.y - (s.y + stroke_width) * 0.5f, 0.0f); > *contains = (d.x * d.x + d.y * d.y) < tolerance * tolerance; Ok, I'll take a look. > > I notice you don't have any tests with transformations, is that > intentional? I think you'd need to transform the stroke width with the > transformation matrix. > No particular reason, but it's a good point. Also we don't have any tests for non-invertible case apparently, I'll add some.

Sept. 1, 2017