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
