Hi Sergey,

Did you check for possible performance hits for the new transform save/compare 
code in BufferedContext?  That is significantly more work per validate().  
Also, isn't this case only used in the case where the caller specified a null 
xform in the argument list?  Does that happen (or, I guess, how does it happen) 
on a scaled graphics?

Something I didn't notice before - you adjust the max texture sizes in CGLGraphicsConfig by the 
device scale, but you adjust them by "getScaleFactor() * 2".  Why the "* 2"?  
Shouldn't adjusting them by the scale factor itself be enough?  In the unscaled case, are our max 
textures sizes now half of what they used to be?  Also, where is this code used from?  Are there 
cases where an unscaled texture is planned and so the divide by the scale factor is unnecessarily 
conservative (as opposed to cases like back buffer allocation where the texture may be @2x...)

You removed a call to flushOnScreenGraphics in LWWindowPeer.  Was that just 
being paranoid before and really isn't needed?

It's already done and the code is written, but in retrospect, I'm not sure how 
much the optimizations in getTransform/setTransform are worth compared to the 
easy readability of having the transforms done linearly in a LIFO manner, such 
as:

setTransform:
        transform.setToIdentity();
        if (cX|cY != 0) { transform.translate(cX, cY); }
        if (dS != 1) transform.scale(dS, dS); }
        transform.concatenate(Tx);
getTransform:
        tx = new AT(transform);
        if (dS != 1) { tx.scale(invDs, invDs); }
        if (cX|cY != 0) { tx.translate(-cX, -cY); }
        return tx;

(I don't think it should be changed, but I'm throwing it out there as food for 
thought as it may be easier to maintain in the future, but it could also change 
the performance as I'm just speculating from how I remember the optimizations 
work...)

                        ...jim

On 4/8/2013 6:10 AM, Sergey Bylokhov wrote:
Hi, Phil, JIm.
New version of the fix:
http://cr.openjdk.java.net/~serb/8000629/webrev.08
  - Assumption that we should scale all native surfaces was wrong. In case of 
managed Bufferedimage we shouldn't scale native surface, because they are 
copies 1-1.(CGLSurfaceData.java:60).
  - BufferedContext is not always updates current Paint, because it checks 
transX/transY from sg2d, which is not maintained in the scale mode.
Test: FillTexturePaint.java

Also I found a problem in the sg2d.getTransform(), where invScale does not 
apply to constrainX/Y.
Test: TransformSetGet.java

On 4/5/13 10:00 PM, Phil Race wrote:
SFAIK you haven't fixed the problem with TexturePaint I pointed out last week.
That needs to be fixed before you push.

-phil.

On 4/5/2013 9:54 AM, Sergey Bylokhov wrote:
Hi, Jim.
I assume, that you haven't additional comments? Can I consider you as the 
second reviewer?
Note that I plan push it to awt-dev because, my other fixes depend on it.

On 3/26/13 7:33 PM, Sergey Bylokhov wrote:
Hello,
Please review the fix for jdk 8.
Change adds initial support of hidpi(mostly on 2d side). In the fix scale was 
added to the surface data/CGraphicsDevice /CGLLayer. This scale factor maps 
virtual coordinates to physical pixels.
This change doesn't add support of hidpi to aqua l&f and doesn't add support of 
dynamic change of scale factor.

Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8000629
Webrev can be found at: http://cr.openjdk.java.net/~serb/8000629/webrev.06
--
Best regards, Sergey.


--
Best regards, Sergey.



--
Best regards, Sergey.

Reply via email to