[OpenJDK Rasterizer] RFR: Marlin renderer #2

[Jim Graham]

Hi Laurent,

On 6/9/2015 2:25 PM, Laurent Bourgès wrote:
>             CollinearSimplifier - why the switch from an enum to numeric
>             constants?  Also, why not use a switch statement instead of
>             a cascade of if tests?
>
>
>         Fixed: I now use a integer-based switch.
>
>
>     Looks good.  I'm still curious why you didn't use an enum - is the
>     code more efficient just dealing with basic numbers rather than enum
>     instances?
>
>
> I did not compare their performance but I prefer using switch(int) vs
> switch(enum) as I would expect it faster. I could try and evaluate the
> performance impact.

switch (enum) uses a lookup table.  All enum values are internally 
assigned an ordinal so it is identical to using a switch on integers 
(other than the fact that it has one method call to get the ordinal of 
the test value, so it's really identical to "switch (foo.getOrdinal())".

> I did understand but there is a subtle difference: 0 is an integer so it
> is handle by the previous case:

Ah, I see it now.  Perhaps it would be worth adding a line to the break 
down comment in case anyone else didn't get it:

     // 0 handled above as an integer
     // sign: 1 for ...
     // add : 0 for ...

> During last week end, I implemented another ceil / floor alternative
> implementations that are definitely faster and exact.
>
> To conclude, do you accept the new floor / ceil alternatives ?

Perhaps we should move them to a follow-on fix?  Just to get the array 
stuff in sooner rather than later...

>     Ah, I see it now.  On the other hand, now you end up with the clunky
>     "Yminus1 + 1" later on.  Since this code is executed once per
>     rendering cycle, I don't see any performance concerns here to
>     require loading the fields into local variables.  Just using the raw
>     fields for the 2 simple calculations may be clearer on the other hand.
>
>
> I wanted to factorize the substraction and avoid repeating it 2 times. I
> agree it is tricky.

Because a nanosecond on an operation that takes several milliseconds is 
worth making the code obscure?  ;)

Also, factoring out the subtraction has a side affect of requiring you 
to insert a new "+1" that didn't use to be there.

I appreciate the attention to detail on some of these calculations, but 
I think there are a lot of opportunities to simplify the algorithms to 
make even bigger impacts.  If a calculation was being factored out of a 
few hundred iterations then it might be worth some level of obscurity in 
the code, but saving 1 instruction per rendering sequence doesn't seem 
worth any amount of disruption to the setup code.

			...jim