<div dir="ltr"><div><div><div>Jim,<br><br></div><div>Here is the updated webrev:<br><a href="http://cr.openjdk.java.net/~lbourges/marlin/Marlin-075.2/">http://cr.openjdk.java.net/~lbourges/marlin/Marlin-075.2/</a><br></div><div><br></div>Changes:<br></div><div>- Added 'TestNonAARasterization (JDK-8170879)' in (D)Stroker classes<br></div><div>- Fixed two comments related to half-open intervals in (D)Renderer classes<br></div><div>- Fixed copyright year to 2017<br></div>- Double-precision Marlin2D enabled by default in RenderingEngine:<br><span style="font-family:monospace,monospace"><br>- final String marlinREClass = "sun.java2d.marlin.MarlinRende<wbr>ringEngine";<br>+ final String marlinREClass = "sun.java2d.marlin.DMarlinRend<wbr>eringEngine";<br></span><br><br></div>My comments below:<br><div><div><div><div><div class="gmail_extra"><br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="gmail-m_3434008031555983311m_3250753773930477956gmail-">
<br>
On 4/26/17 2:32 PM, Laurent Bourgès wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
- MarlinProperties - TileSize vs TileWidth. Is there a reason you haven't created a TileHeight property? I could<br>
see a couple of ways of going here:<br>
- TileSize means height and TileWidth is new which is just odd naming<br>
In this case, I'd make the default for TileWidth be the value for TileSize<br>
otherwise setting tilesize used to set both W&H, but now it only sets H?<br>
- TileSize is legacy and new values are TileWidth and TileHeight<br>
Both default to TileSize if not specified<br>
In either case, I would think that TileWidth should default to TileSize?<br>
<br>
<br>
Fixed, I adopted the first solution and getTileWidth_Log2() uses getTileSize_Log2() to get the default value (W=H)<br>
</blockquote>
<br></span>
I was leaning towards adding W & H and having Size be the old mechanism - for symmetry - but this is fine.<span class="gmail-m_3434008031555983311m_3250753773930477956gmail-"><br></span></blockquote><div><br></div><div>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.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="gmail-m_3434008031555983311m_3250753773930477956gmail-">
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
- MarlinTileGenerator,MarlinRend<wbr>erer - all of the methods called on rdrF and rdrD have the same signature. Why not<br>
have MarlinRenderer include those methods and then you just need to store a single MarlinRenderer field and be able<br>
to manipulate either type...?<br>
<br>
<br>
I agree. I tried but benchamrks proved that interface calls method were slower than monomorphic calls so I adopted<br>
this bimorphic call optimization where only 1 type is really used at runtime.<br>
</blockquote>
<br></span>
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.<span class="gmail-m_3434008031555983311m_3250753773930477956gmail-"><br></span></blockquote><div><br></div><div>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. <br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="gmail-m_3434008031555983311m_3250753773930477956gmail-">
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
- Renderer, line 85,114 - maybe add a line saying that is the result of <name> test? Did we put that test into the<br>
repo anywhere?<br>
<br>
<br>
Added comment: 'TestNonAARasterization: cubics/quads' (not in repo, only in JBS)<br>
</blockquote>
<br></span>
I'd add the JBS number "JDK_NNNNNNNN" as well then.<span class="gmail-m_3434008031555983311m_3250753773930477956gmail-HOEnZb"><font color="#888888"><br></font></span></blockquote><div><br></div><div>Fixed.<br><br></div><div>Cheers,<br></div><div>Laurent<br></div></div>
</div></div></div></div></div></div>