[OpenJDK Rasterizer] Fwd: Re: Fwd: RFR: Marlin renderer #3
Jim Graham
james.graham at oracle.com
Wed Jul 29 03:11:13 UTC 2015
Hi Laurent,
Almost all of this is code comments with only 1 minor functional issue
(Renderer.java, line 461).
NormalizingIterator.NearestPixel should probably be renamed
NearestPixelQuarter to reflect its functionality (and
RenderCtx.nPQPathIterator).
getNormalizing() - why not just return src for mode OFF? That way you
don't even have to test in the calling function, though it does add an
extra method call in the non-normalizing case, it simplifies the code -
and hopefully the method is simple enough to be inlined.
Renderer.java, line 45: fractToInt() is never used.
Renderer.java, line 203: typo in comment -> 96K?
Renderer.java, line 379: Instead, mention that y values are already
shifted by -0.5 in tosubpixy()?
(The "note" comment makes it appear that that is work that is left to
be done, but it is already done)
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.
Renderer.java, line 792: Do you need to copy ERR_STEP_MAX into a local?
It is a statically initialized final int - that should already be a
constant that gets copied into the code.
Renderer.java, line 1350-ish: mention in here somewhere that edgeMinMaxY
values have already been adjusted by -0.5 in tosubpixy(), but
edgeMinMaxX values have not yet been adjusted? (It took me a moment to
realize that.)
CeilAndFloorTests - whoa, hex floating point literals? That's a new one
for me... ;)
I'm guessing that p22 has 1 bit of fractional mantissa and p23 has no
fractional mantissa digits?
...jim
On 7/24/15 6:55 AM, Laurent Bourgès wrote:
> Jim,
> Did you have time to have a look to my last webrev ?
>
> I made some improvements since on the cubic / quad breaking into lines
> to use a proper norm instead of abs (ddx) or abs (ddy).
> I tried the taxicab norm and now I use square = ddx * ddx + ddy * ddy.
>
> Note: the test for quads was flawed:
> Max (ddx, ddy) fails for negative values !
>
> PS: I am now adjusting (lowering) the threshold to achieve better
> quality without having too much performance loss.
>
> I will be on vacation next week, but I may read emails but less often.
>
> Cheers,
> Laurent
>
> Le 21 juil. 2015 23:33, "Laurent Bourgès" <bourges.laurent at gmail.com
> <mailto:bourges.laurent at gmail.com>> a écrit :
>
> Jim,
>
> Here is the first webrev including the new rounding approach:
> http://cr.openjdk.java.net/~lbourges/marlin/marlin-s3.2/
>
> 1. It is based on your fixed-point approach and use FloatMath
> ceil_int / floor_int for perfomance.
> I shift the subpixel y coordinates by - 0.5 in tosubpixy() as mentioned.
>
> 2. I tested many variants (Unsafe / Safe) with many subtle changes
> and I am now satisfied by the code and its performance (only 1%
> slower than previous) but the quality is a lot better and very close
> to ductus.
>
> If you are interested by the Safe variant (int[] arrays) I can
> provide another webrev (2-3% slower) as I now use offsets as long
> variables.
>
> 3. I finally understood the curve and quad AFD approach (reading the
> reference paper) and I reduced the initial step to lg=2 (instead or
> 3 and 4).
> It is of course faster without any quality loss:
> - cubic step is adaptive (based on dx/dy ie pixel distance between
> points): in my tests, it was mainly doubling to reach to count=2 !
> - quad step can only be divided so I selected an higher step to let
> the algo reduce it if needed: in my tests, the initial step (lg=4)
> was very too precise generating lots of small segments ! It improved
> a lot the performance of the first 2 maps (see below)
>
> To conclude, the curve / quad decimation is not optimal as the step
> criteria is only constrained by |dx/dy| so it is appropriate to draw
> pixels (depending on the previous one) and maybe not to estimate the
> error between the segment and the real curve.
>
> That's why it is generating more or less segments independently to
> the distance error:
> I propose to study *later* other algorithms present in Pisces
> Flattener or AGG (adaptive subdivision) that can provide a good
> estimation of the min/max (error):
>
> https://code.google.com/p/pisces-graphics/source/browse/trunk/src/pisces/d/Flattener.java?spec=svn24&r=24
> http://antigrain.com/research/adaptive_bezier/
>
> "DrawingParametric CurvesUsing ChebyshevPolynomials ":
> http://graphicsinterface.org/wp-content/uploads/gi1991-3.pdf
>
> Maybe you have some ideas on Renderer's curve subdivision
> implementation and how to tune it better ... or propose another
> algorithms ...
>
> >>>> - do you know if the breakCurveAndAddLines (quad or cubic) really takes
> >>>> into account the supersampling scale to generate only segments needed
> >>>> and no more ?
> >>>
> >>>
> >>> I don't remember. I'd have to read the code and figure it out.
> >>
> >>
> >> Thanks, it seems there are some thresholds BND... but I am unable to
> >> find out what it is related to ?
> >
> >
> > I'd have to research that as well. I briefly understood them when I reviewed the code and I was able to fine-tune them once when we had failures in the FX version, but they are essentially a variant of "epsilon" but related to the adaptive subdivision algorithm so I mostly just treated them as tuning parameters - an accuracy vs. time tradeoff.
>
> It seems these values are 32 (quad) and 8 (curve) so it seems
> related to the scaling factor = 8. It would be great to express
> this dependency in the constants.
>
>
> Cheers,
> Laurent
>
> PS: Benchmark results with OpenJDK9 (after/before):
>
>
> New OpenJDK9 tests:
> mardi 21 juillet 2015, 22:53:05 (UTC+0200)
>
> TEST results:
> Test Threads Ops
> Med Pct95 Avg StdDev Min Max TotalOps [ms/op]
> dc_boulder_2013-13-30-06-13-17.ser 1 105
> 99.323 99.710 99.372 0.181 99.148 100.230 105
> dc_boulder_2013-13-30-06-13-17.ser 2 210
> 100.390 100.669 100.407 0.135 100.151 101.080 210
> dc_boulder_2013-13-30-06-13-17.ser 4 420
> 101.404 101.903 101.471 1.100 100.530 121.952 420
> dc_boulder_2013-13-30-06-13-20.ser 1 204
> 50.945 51.125 50.905 0.183 50.400 51.403 204
> dc_boulder_2013-13-30-06-13-20.ser 2 408
> 52.005 52.207 52.005 0.125 51.538 52.419 408
> dc_boulder_2013-13-30-06-13-20.ser 4 816
> 52.615 52.979 52.658 0.766 51.925 72.766 816
> dc_shp_alllayers_2013-00-30-07-00-43.ser 1 254
> 41.029 41.408 41.023 0.240 40.628 41.717 254
> dc_shp_alllayers_2013-00-30-07-00-43.ser 2 508
> 41.609 42.037 41.619 0.243 41.153 42.360 508
> dc_shp_alllayers_2013-00-30-07-00-43.ser 4 1016
> 42.371 42.701 42.418 0.680 41.722 63.057 1016
> dc_shp_alllayers_2013-00-30-07-00-47.ser 1 25
> 784.913 786.240 784.909 0.821 783.040 786.625 25
> dc_shp_alllayers_2013-00-30-07-00-47.ser 2 50
> 787.773 789.291 787.699 1.208 785.225 789.820 50
> dc_shp_alllayers_2013-00-30-07-00-47.ser 4 100
> 787.382 790.768 787.992 3.168 783.223 805.677 100
> dc_spearfish_2013-11-30-06-11-15.ser 1 808
> 13.012 13.247 13.051 0.098 12.992 13.545 808
> dc_spearfish_2013-11-30-06-11-15.ser 2 1616
> 13.046 13.168 13.065 0.079 13.003 13.568 1616
> dc_spearfish_2013-11-30-06-11-15.ser 4 3232
> 13.047 13.244 13.081 0.302 13.002 29.082 3232
> dc_spearfish_2013-11-30-06-11-19.ser 1 1585
> 6.632 6.733 6.648 0.053 6.619 6.979 1585
> dc_spearfish_2013-11-30-06-11-19.ser 2 3170
> 6.644 6.744 6.658 0.053 6.621 6.996 3170
> dc_spearfish_2013-11-30-06-11-19.ser 4 6340
> 6.641 6.754 6.663 0.234 6.619 22.091 6340
> dc_topp:states_2013-11-30-06-11-06.ser 1 841
> 12.556 12.668 12.572 0.060 12.458 12.759 841
> dc_topp:states_2013-11-30-06-11-06.ser 2 1682
> 12.505 12.614 12.523 0.059 12.395 12.749 1682
> dc_topp:states_2013-11-30-06-11-06.ser 4 3364
> 12.528 12.643 12.575 0.821 12.411 45.321 3364
> dc_topp:states_2013-11-30-06-11-07.ser 1 1355
> 7.721 7.781 7.705 0.058 7.568 7.950 1355
> dc_topp:states_2013-11-30-06-11-07.ser 2 2710
> 7.720 7.764 7.701 0.056 7.561 7.876 2710
> dc_topp:states_2013-11-30-06-11-07.ser 4 5420
> 7.729 7.799 7.721 0.237 7.565 24.055 5420
> test_z_625k.ser 1 63
> 165.148 165.499 165.208 0.200 164.967 166.228 63
> test_z_625k.ser 2 126
> 166.309 166.458 166.319 0.075 166.169 166.604 126
> test_z_625k.ser 4 252
> 167.391 168.287 167.599 1.352 166.501 188.421 252
>
> Scores:
> Tests 27 9 9 9
> Threads 4 1 2 4
> Pct95 132.313 131.601 132.328 133.009
>
>
> B/ Previous OpenJDK9 tests:
> vendredi 19 juin 2015, 23:37:44 (UTC+0200)
>
> TEST results:
> Test Threads Ops
> Med Pct95 Avg StdDev Min Max TotalOps [ms/op]
> dc_boulder_2013-13-30-06-13-17.ser 1 94
> 111.198 111.351 111.199 0.101 110.993 111.483 94
> dc_boulder_2013-13-30-06-13-17.ser 2 188
> 112.305 112.532 112.324 0.120 112.086 112.883 188
> dc_boulder_2013-13-30-06-13-17.ser 4 376
> 113.469 113.640 113.439 0.200 112.499 115.244 376
> dc_boulder_2013-13-30-06-13-20.ser 1 187
> 55.177 55.432 55.191 0.143 54.960 55.902 187
> dc_boulder_2013-13-30-06-13-20.ser 2 374
> 56.192 56.443 56.194 0.146 55.860 56.709 374
> dc_boulder_2013-13-30-06-13-20.ser 4 748
> 57.034 57.259 57.046 0.127 56.669 58.219 748
> dc_shp_alllayers_2013-00-30-07-00-43.ser 1 244
> 42.747 43.124 42.780 0.151 42.549 43.374 244
> dc_shp_alllayers_2013-00-30-07-00-43.ser 2 488
> 43.256 43.520 43.278 0.134 42.992 43.929 488
> dc_shp_alllayers_2013-00-30-07-00-43.ser 4 976
> 44.137 44.581 44.228 1.927 42.911 89.464 976
> dc_shp_alllayers_2013-00-30-07-00-47.ser 1 25
> 765.628 766.016 765.629 0.195 765.233 766.170 25
> dc_shp_alllayers_2013-00-30-07-00-47.ser 2 50
> 772.097 772.824 771.872 0.797 769.439 773.263 50
> dc_shp_alllayers_2013-00-30-07-00-47.ser 4 100
> 769.742 770.547 770.285 2.738 768.929 784.274 100
> dc_spearfish_2013-11-30-06-11-15.ser 1 822
> 12.675 12.748 12.685 0.039 12.648 13.445 822
> dc_spearfish_2013-11-30-06-11-15.ser 2 1644
> 12.686 12.746 12.692 0.035 12.651 13.474 1644
> dc_spearfish_2013-11-30-06-11-15.ser 4 3288
> 12.702 12.768 12.710 0.051 12.659 15.153 3288
> dc_spearfish_2013-11-30-06-11-19.ser 1 1604
> 6.545 6.611 6.554 0.027 6.523 6.810 1604
> dc_spearfish_2013-11-30-06-11-19.ser 2 3208
> 6.550 6.613 6.557 0.026 6.527 6.816 3208
> dc_spearfish_2013-11-30-06-11-19.ser 4 6416
> 6.556 6.627 6.564 0.028 6.531 6.967 6416
> dc_topp:states_2013-11-30-06-11-06.ser 1 876
> 12.004 12.083 11.995 0.064 11.861 12.583 876
> dc_topp:states_2013-11-30-06-11-06.ser 2 1752
> 12.005 12.082 11.995 0.061 11.837 12.173 1752
> dc_topp:states_2013-11-30-06-11-06.ser 4 3504
> 11.992 12.085 11.997 0.078 11.852 13.553 3504
> dc_topp:states_2013-11-30-06-11-07.ser 1 1446
> 7.266 7.343 7.252 0.066 7.120 7.903 1446
> dc_topp:states_2013-11-30-06-11-07.ser 2 2892
> 7.277 7.350 7.264 0.062 7.120 7.444 2892
> dc_topp:states_2013-11-30-06-11-07.ser 4 5784
> 7.276 7.347 7.263 0.069 7.116 9.590 5784
> test_z_625k.ser 1 62
> 166.409 166.693 166.430 0.158 166.098 166.865 62
> test_z_625k.ser 2 124
> 167.600 167.801 167.610 0.107 167.412 167.906 124
> test_z_625k.ser 4 248
> 168.970 169.217 168.979 0.188 168.510 170.056 248
>
> Scores:
> Tests 27 9 9 9
> Threads 4 1 2 4
> Pct95 132.125 131.267 132.435 132.675
>
More information about the graphics-rasterizer-dev
mailing list