[quake3-bugzilla] [Bug 3639] BoxOnPlaneSide patch

[bugzilla-daemon at icculus.org]

http://bugzilla.icculus.org/show_bug.cgi?id=3639

--- Comment #12 from Daniel Gibson <metalcaedes at gmail.com> 2009-09-19 00:10:45 EDT ---
I forgot one case: the default case in that switch statement. I don't know if
its important (the comments suggest it was just added to shut the compiler up).
However, if the new implementation should behave *exactly* like the old one,
there has to be made a minor change:
Add "if(p->type < 8)" just before the for()-loop.
This made the implementation slightly slower (73-74ms instead of 67-69ms), but
it's still faster than the ASM version (76ms) or the old C version (83-84ms).

As Patrick said, this is no groundbreaking improvement.
But the new code is shorter than the old C code, slightly faster than the old
ASM code, about 1.13 times as fast as the old C code (good for non-i386) and
you could get rid of >700lines of assembly code.
So I'd suggest to accept the patch, possibly with my suggested fix for the
default case (that probably shouldn't happen).

Cheers,
- Daniel

--- Comment #13 from Daniel Gibson <metalcaedes at gmail.com> 2009-09-19 11:38:28 EDT ---
(In reply to comment #12)
> I forgot one case: the default case in that switch statement. I don't know if
> its important (the comments suggest it was just added to shut the compiler up).
> However, if the new implementation should behave *exactly* like the old one,
> there has to be made a minor change:
> Add "if(p->type < 8)" just before the for()-loop.
> This made the implementation slightly slower (73-74ms instead of 67-69ms), but
> it's still faster than the ASM version (76ms) or the old C version (83-84ms).


It should have been "if(p->signbits < 8)" before the for()-loop of course..
shouldn't have tested that idea at 6am :-/
Of course I've also benched that version and it even seems like it's a bit
faster than the wrong version (~70ms, instead of 73-74ms with "if(p->type
<8)").

I also altered the Benchmarking Code again to check correctness. It runs both
implementations and compares the results (see attachement).

Cheers,
- Daniel

-- 
Configure bugmail: http://bugzilla.icculus.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.