2015-06-25 23:45 GMT+02:00 David Adam david.adam.cnrs@gmail.com:
Hi,
I added a test with 65000 sides. All the test pass with a native d3dx9_36 dll.
You should have mentioned that the new test doesn't pass with the previous implementation, which is what makes this patch really convincing. So this patch is okay to me.
Also notice that this in practice doesn't save any multiplication in the loop, at least with my version of GCC: the multiplication 2.0f * D3DX_PI was precalculated at compile time. A better patch title mentioning D3DXCreatePolygon would be nice.
The tests generally have some room for improvement. As a followup it would be nice to try and improve them, maybe by replacing the comparison function with one which allows for a relative error and makes it possible to choose a potentially different error value for each test (to be able to lower those as much as possible).
I would like to point you to some other place in Wine where the float comparison is done right but, on the spot, I only found relative_error + expect_color macros from d3dx9_36/tests/math.c which are really ugly. FWIW https://randomascii.wordpress.com/2012/02/25/comparing-floating-point-number... seems to be a good read on the subject.