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