[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-2d-dev
Subject:    Re: [OpenJDK 2D-Dev] [10] Marlin2D upgrade 0.7.5
From:       Jim Graham <james.graham () oracle ! com>
Date:       2017-05-09 20:51:42
Message-ID: eef72fef-dd18-a16e-8716-9f5eacd9b710 () oracle ! com
[Download RAW message or body]

This looks fine, but I've reached out to Phil with a question about changing the \
default and whether we need to file a  request for that.

Is there a JBS bug for this yet?

				...jim

On 5/9/17 3:06 AM, Laurent Bourgès wrote:
> Jim,
> 
> Here is the updated webrev:
> http://cr.openjdk.java.net/~lbourges/marlin/Marlin-075.2/
> 
> Changes:
> - Added 'TestNonAARasterization (JDK-8170879)' in (D)Stroker classes
> - Fixed two comments related to half-open intervals in (D)Renderer classes
> - Fixed copyright year to 2017
> - Double-precision Marlin2D enabled by default in RenderingEngine:
> 
> -            final String marlinREClass = \
> "sun.java2d.marlin.MarlinRenderingEngine"; +            final String marlinREClass \
> = "sun.java2d.marlin.DMarlinRenderingEngine"; 
> 
> My comments below:
> 
> 
> 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. 
> 
> Agreed but we could add that later when we will increase the tile width & height \
> (asymmetric) for performance. Few adjustments remain in java2d pipeline classes.
> 
> 
> 
> - 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. 
> 
> I tested this issue a lot in december and this 'bimorphic' optimization is \
> concluding. I agree having an abstract class would be good but extracting common \
> parts between Renderer & DRenderer is not easy and it may be more difficult to \
> maintain. 
> 
> 
> - 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.
> 
> 
> Fixed.
> 
> Cheers,
> Laurent


[prev in list] [next in list] [prev in thread] [next in thread]