[OpenJDK 2D-Dev] [10] Marlin2D upgrade 0.7.5

Jim Graham james.graham at oracle.com
Fri May 5 20:50:00 UTC 2017


Hi Laurent,

A couple of comments that might just be points for clarification.  Minimally it would make sense to include the JBS 
report number on the third item below, but the rest are just notes and suggestions...

On 4/26/17 2:32 PM, Laurent Bourgès wrote:
>     - MarlinProperties - TileSize vs TileWidth.  Is there a reason you haven't created a TileHeight property?  I could
>     see a couple of ways of going here:
>        - TileSize means height and TileWidth is new which is just odd naming
>          In this case, I'd make the default for TileWidth be the value for TileSize
>          otherwise setting tilesize used to set both W&H, but now it only sets H?
>        - TileSize is legacy and new values are TileWidth and TileHeight
>          Both default to TileSize if not specified
>     In either case, I would think that TileWidth should default to TileSize?
>
>
>     Fixed, I adopted the first solution and getTileWidth_Log2() uses getTileSize_Log2() to get the default value (W=H)

I was leaning towards adding W & H and having Size be the old mechanism - for symmetry - but this is fine.

>     - MarlinTileGenerator,MarlinRenderer - all of the methods called on rdrF and rdrD have the same signature.  Why not
>     have MarlinRenderer include those methods and then you just need to store a single MarlinRenderer field and be able
>     to manipulate either type...?
>
>
>     I agree. I tried but benchamrks proved that interface calls method were slower than monomorphic calls so I adopted
> this bimorphic call optimization where only 1 type is really used at runtime.

I'm curious how much difference this made to require this amount of complexity, but there is a solution here if you are 
really worried about performance - use a super-class instead of an interface.  If you can measure overhead for invoking 
an abstract method then there is something wrong with the VM.

>     - Renderer, line 85,114 - maybe add a line saying that is the result of <name> test?  Did we put that test into the
>     repo anywhere?
>
>
>     Added comment: 'TestNonAARasterization: cubics/quads' (not in repo, only in JBS)

I'd add the JBS number "JDK_NNNNNNNN" as well then.

				...jim



More information about the 2d-dev mailing list