[PATCH] d3dx9/tests: Add test for out of bound array selector in effect.
Paul Gofman
gofmanp at gmail.com
Tue Mar 7 13:57:37 CST 2017
Hi Matteo,
> It seems preferable to keep the detailed ok() calls from the old test
> instead of a generic ok() message on the return value of
> test_effect_preshader_compare_shader(). I guess you want to avoid to
> have ambiguous ok() calls in this helper. Until we get something like
> the with_context() thing from
> https://source.winehq.org/patches/data/131137 just passing a string
> with the test name to the helper function and using that in the ok()
> messages should do the trick.
The problem I had with it is actually when ok(...) with the call to this
function as the condition fails within the main test function body, I
get the line of failure inside this function (and not of the actual
ok()), which is confusing. I didn't check, it may be related to compiler
optimization/inlining somehow and may be not reproducible with default
build flags, but should not I avoid that? Maybe I would better trace
failure messages passing a main test context string?
>> + hr = effect->lpVtbl->BeginPass(effect, 1);
>> + todo_wine ok(hr == E_FAIL, "Got result %#x.\n", hr);
>> + if (SUCCEEDED(hr))
>> + effect->lpVtbl->EndPass(effect);
>> +
>> + hr = effect->lpVtbl->BeginPass(effect, 1);
>> + ok(hr == D3D_OK, "Got result %#x.\n", hr);
> That reminds me, do we have tests for two BeginPass() calls without an
> EndPass() in between? Might be interesting to see what happens both
> with the same or different pass index.
I could not find two consequent BeginPass() tests, I can add some as a
separate case, but do you think it is related here to the out of bounds
behaviour?
BTW if I add EndPass right after the first (failing) BeginPass here, it
will fail as expected.
What is shown here, is that after setting variable with Set
function, the first call to BeginPass fails, while exactly the same next
one succeeds, which suggests that it is related to the "dirty" parameter
state which is reset within even an unsuccessful call to BeginPass. The
2nd BeginPass selects the last element in the array for both negative
and positive out of bounds index. But there is a special value of -1 for
index, which makes even the first call to BeginPass succeed and select
first array element. At the same time, CommitChanges never fails, while
the state is not updated.
When you say what happens with the same or different pass index, do
you mean a test where the same variable causes out of bounds behaviour
in 2 different passes, and check if BeginPass() will succeed for another
pass after failing for the first one? This will require blob
modification, as currently 2 passes have array selectors, but different
variables are used there. Maybe add a completely different test blob in
this case not to overload & constantly update all the same big one?
> Other somewhat interesting
> cases might be BeginPass() outside of Begin() / End() and multiple
> Begin() / End() / EndPass() calls in a row. It should probably be in a
> separate patch though.
The motivation under this test was that I started preparing my patches
for dirty parameter flags update and usage in CommitChanges, and while
extending the test I found that very specific behaviour of out of bounds
access processing. So I extracted this part of test into separate one to
split the thing. I actually suggest not to cover this todo's in the
initial dirty parameters update implementation leaving the out of bounds
processing in some consistent state, i. e., always clamp to range or
always skip the state (as it is done now) without failing BeginPass().
>
> BTW, I think it's somewhat idiomatic to call it "out of bounds" even
> when technically there is a single bound involved.
>
I will rename it.
More information about the wine-devel
mailing list