[OpenJDK Rasterizer] RFR: Marlin renderer #3

[Jim Graham]

Hi Laurent,

On 6/25/15 12:50 PM, Laurent Bourgès wrote:
> Marlin use cases:
> - ceil:
> 1/ addLine: it serves to check if points are within clip bbox (integer)
> and compute first/ last crossings.
> But later crossings are computed in endRendering from float maths but
> just casted to int : it seems incorrect /inconsistent to me !

It is.  We should have consistent crossing policies in both directions.

> Maybe DDA or floor (val) or ceil (val + 0.5) should be used instead in
> both places ?

I'm not sure how DDA impacts this since it is simply a formula for 
computing the exact location where the curve crosses the scanline. 
Either way we need to somehow determine how to interpret that crossing 
and that's where the ceil()/floor()/cast decision comes in.

non-AA uses ceil([xy] - 0.5) to match the algorithms used to test 
Shape.contains(x,y).

> But it will be slower... so DDA seems the only fast and probably correct
> approach but It may be tricky too to implement properly. Do you know how
> to do it well ?

How does DDA make a difference for ceil/floor/cast?

> How to deal with a NaN coord ? I suppose no line should be renderered
> but it may cause artefacts...

In other parts of the system we skip all segments that have a NaN 
anywhere in them.

sun.dc.DuctusRenderingEngine.feedConsumer has tests to eliminate NaN 
segments (this class is in the public repos even though it is feeding 
the Ductus library).

sun.java2d.loops.ProcessPath.doProcessPath also has similar code to 
eliminate NaN values.

> 2/ Determine edgeBucket used range: correct too.
>
> - Floor: it is used by NormalizingPathIterator to compute x/ adjust
> values = 0.5 - fractional_part (coord).
> Here the possible values may be in the full float domain !
>
> Probably I should use a correct Floor impl as before or propose a new
> method to compute the fractional part directly.
> Fract = coord - floor (coord) = coord % 1f

What do you mean by a correct Floor impl?

I'd have to see how the fract method would be used there to see if it 
makes sense.  One thing I noticed when I read through that code is that 
we duplicate some calculations.  In particular, we have:

coord = coords[N];
adjust = <round calc> - coord;
... then after the if statement:
coords[N] += adjust;

This is equivalent to:

coord = coords[N];
adjust = <round> - coord;
val = coords[N];  // This is the same as coord, right?
coords[N] = val + adjust;  // This is the same as <round>, right?

We should just do it all together as in:

coord = coords[N];
val = <round>;
coords[N] = val;
adjust = val - coord;

It saves a memory load and having to undo a subtract with an add.

> Any idea?
> Again how to deal with NaN / Inf values in this case ?

This is interesting because if we ever encounter a NaN then the adjust 
parameters will tend to carry it forward in all of the calculations and 
so the rest of the path could become all NaN values.  Hopefully the code 
I pointed out above can provide an idea of how to deal with this. 
NormalizingPathIterator could serve both as a normalizer and a NaN 
rejecter.  We'd need some form of NaN rejection for the case where 
STROKE_NORMALIZE is turned off, though.

>> I notice that CHECK_NAN is false, but you get the right result below. Do you also get the right result for the Floor function?
>
> It works well if CHECK_NAN is true so I will enable it back.

We may not care if we reject NaN's at a higher level.

			...jim