[1/2] gdi32: Added PolyDraw tests to tests/path.c [try2]

Misha Koshelev mk144210 at bcm.edu
Fri Jun 29 19:56:33 CDT 2007


On Fri, 2007-06-29 at 14:49 -0700, Evan Stade wrote:
> Thanks a lot for your help.
No prob.

> 
> Your shorter diff may make the tests conform, but it incorrectly
> assigns lastmove.x = dc->CursPosX.  This only makes it conform to the
> tests because the tests happen to have a MoveToEx as the last path
> point before the PolyDraw is called.  
You should include this in your tests, e.g., by calling LineTo after the
MoveToEx and before the first PolyDraw. This immediately makes it clear
why you need to have separate path and non-path cases. (I personally
think about conformance tests like this - you worked hard to figure out
how the function works on native and implemented your own version, but
someone might commit a patch sometime in the future to fix something
else that screws up all your hard work; that's why you need to put this
knowledge into conformance tests, so once that feature is implemented in
wine it will never break again).

> Also, your diff doesn't
> correctly check the point types. The test if( lpbTypes[i] ==
> PT_MOVETO) (which was there before your changes) is wrong because it
> returns false for PT_MOVETO | PT_LINETO, when it should return true
> (according to my testing which was not a part of the test I
> submitted).  Also, your diff does not MoveToEx to orig_pos at the
> second "return FALSE".  This is incorrect but again does not show up
> in the tests.
Again, these are things that should ideally be a part of your
conformance test. That would make the rationale for your extensive
changes in your second patch clearer and would improve the likelihood of
the patches going in.

> 
> So while a small and simple diff can make these specific tests
> conform, it is not correct.  However I still don't understand how this
> influences whether the test patch got accepted.  Your explanation of
> why the fix patch didn't get accepted on the other hand does make
> sense.
> 
I am not sure why the test patch was not accepted (unless maybe
Alexandre wants both patches ready to go before committing either, as if
you make any changes to the second one you might find you need to add
something to the tests... don't know), but having the whole patchset
complete and good to go certainly wouldn't hurt either patch.

Misha




More information about the wine-devel mailing list