[OpenJDK Rasterizer] Fwd: Re: Fwd: RFR: Marlin renderer #3

[Jim Graham]

My "1 minor functional issue" in my last email was actually in error...

On 7/28/15 8:11 PM, Jim Graham wrote:
> Renderer.java, line 461: What happens if x1_cor was already at VPC?  The
> error should be 0, but you end up setting it to ERR_STEP_MAX.

Ignore this.  I see now.  At VPC, any fractional movement forward should 
bump you by another pixel so the error really is ERR_STEP_MAX.

Basically error is "how far we've progressed since the previous VPC 
crossing" and at a VPC crossing you've crossed about as far as you can 
go before you are going to default over to the next pixel crossing.

Instead I'd change the comment on 461 to something like:
     // Crossings bump by a whole pixel just after (to the right of) a VPC.
     // Error is how far since we last bumped the crossing.
And/or:
     // Error is ERR_STEP_MAX right on a VPC where we are about to bump 
1 more pixel
     // and 0 just after VPC where we have just recently bumped

It's a minor nit, but your calculation is still slightly off in that we 
would not necessarily compute a value of 0 right after the VPC because 
(x1_cor - istartx) would be slightly greater than 0.0f.  The calculation 
you are using is correctly placing ERR_STEP_MAX at "on a VPC", but it is 
also placing 0.0 at "on the previous VPC" when the correct placement for 
0.0 is just after the previous VPC.  The error is thus almost always 
slightly larger than it should be (but by a very very tiny amount as in 
1/ERR_STEP_MAX).

It should be:

err(VPC(n-1))         = ERR_STEP_MAX
err(VPC(n-1)+epsilon) = 0
...
err(VPC(n))           = ERR_STEP_MAX

but your calculation is:

err(VPC(n-1))         = 0
err(VPC(n-1)+epsilon) = 1
err(VPC(n))           = ERR_STEP_MAX

This calculation might be ever so slightly more accurate, I'm not sure 
if it is any faster:

long x1_fixed = (long) Math.scalb(x1_cor, 32);
// x1_fixed is now 32.32 representation
x1_fixed -= 1;
// x1_fixed is now reduced by "epsilon"
int x1_whole = ((int) (x1_fixed >> 32));
int x1_fract = (((int) (x1_fixed)) >>> 1);
// x1_whole/fract are now a split 32.31 representation (of x1_fixed - 
epsilon)
x1_whole += 1;
// x1_whole is now ceil(x1_cor)

Or, more compactly:

long x1_fixed_m_eps = ((long) Math.scalb(x1_cor, 32)) - 1;
store(CURX, ((int) (x1_fixed_m_eps >> 32)) + 1);
store(ERRX, ((int) (x1_fixed_m_eps)) >>> 1);

It performs ceil() in fixed point (32.31+1) math as floor(v + 1 - 
epsilon) and ends up with the error/fract term being exactly 
ERR_STEP_MAX on a whole number and the same err/fract term to be 0 
exactly at the next possible fixed point step after a whole number. 
Note that since x1_cor is only single-precision it is unlikely to ever 
see a fract value of "0" since the float doesn't have enough precision 
to represent a number that is "1/ERR_STEP_MAX" greater than an integer 
unless the integer happens to be 0.  But if it could represent that 
number, it would be scaled exactly to an error value of 0.  It would be 
slightly more accurate if the x1_cor value was calculated in double (to 
keep as many bits of fractional value as possible).  A double mantissa 
could maintain 31 bits of sub-pixel precision for coordinates in the 
range of +/- a million or so, though it isn't clear how much value that 
would have since all of the calculations up to this point have been done 
in single precision.

So, I present those equations more in case they might be faster than 
because they may be "more accurate".  Note that there is no call to 
ceil_int() here at all, only a cast to long...

			...jim