Re: [OpenJDK 2D-Dev] [11] Upgrade to Marlin renderer 0.9.1

[Philip Race]

Hi Laurent,

It took me I at least 5 tries to get all the way through this.
I don't have any substantial comments, just a few clean up questions.


What is this in Curve.java + DCurve.java ?

+        if (false) {
+            dax = 0.0d;
+            day = 0.0d;
+            dbx = 0.0d;
+            dby = 0.0d;
+        }

In Renderer.java, should the commented line be removed ?

+&&  ((Math.abs(ddx) + Math.abs(ddy) * _SCALE_DY)<= _INC_BND
+//&&  (Math.abs(ddx + dddx) + Math.abs(ddy + dddy) * _SCALE_DY)<= _INC_BND

A similar pattern occurs a little later in the same file.

+//                || (Math.abs(ddx + dddx) + Math.abs(ddy + dddy) * 
_SCALE_DY)>= _DEC_BND


+        static final float LEN_TH = MarlinProperties.getSubdividerMinLength();

You really meant to name it LEN_TH and not LENGTH ?


There are a few TODO's I see but they seem to be more about later clean up or 
optimisation
so are probably all OK.

So I am OK to push, and if you clean up any of the above first I don't need to 
see a new webrev.

-phil.



On 3/23/18, 2:03 PM, Laurent Bourgès wrote:

Hi,

Sorry to insist but I would like to get feedback on this Marlin patch soon before going forward on tile-size tuning in java2d accelerated pipelines.

Laurent

2018-03-21 22:56 GMT+01:00 Laurent Bourgès <bourges.laur...@gmail.com <mailto:bourges.laur...@gmail.com>>:

    Hi,

    Here is the updated webrev:
    http://cr.openjdk.java.net/~lbourges/marlin/marlin-091.1/
    <http://cr.openjdk.java.net/%7Elbourges/marlin/marlin-091.1/>

    Changes in MarlinProperties only:
    - getTileSize_Log2() & getTileWidth_Log2(); 32x32 tiles ie default
    = 5 (log2)

    I hope it is good for now as tile settings are the same as in jdk9/10.

    Regards,
    Laurent


    2018-03-21 21:44 GMT+01:00 Laurent Bourgès
    <bourges.laur...@gmail.com <mailto:bourges.laur...@gmail.com>>:

        Sergey,

        Le mer. 14 mars 2018 à 17:14, Sergey Bylokhov
        <sergey.bylok...@oracle.com
        <mailto:sergey.bylok...@oracle.com>> a écrit :

            On 13/03/2018 17:04, Sergey Bylokhov wrote:

                I have started to look to this review, will run some
                closed tests and send a feedback soon.


            No issues found so far, +1.


        Thanks for your vote.
        I need another approval I suppose ?

        I will prepare another review asap reverting only tile size
        changes as using large tiles has performance drop on d3d & ogl
        that needs more work. It can be done later in follow-up issues.

        Phil do you agree the proposed plan ?

        Laurent