[OpenJDK 2D-Dev] [8] Request for review: 8000629 [macosx] Blurry rendering with Java 7 on Retina display (original) (raw)

Sergey Bylokhov Sergey.Bylokhov at oracle.com
Tue Apr 9 02:54:28 PDT 2013


Hi, Jim. On 4/9/13 1:19 AM, Jim Graham wrote:

Hi Sergey,

Did you check for possible performance hits for the new transform save/compare code in BufferedContext? I am not sure how it can be improved, because BufferedPaints.setPaint() depends from the current transform of the sg2d.transform. And it should be called, when sg2d.transform is changed. That is significantly more work per validate(). I guess in most common case only aftm.equals() was added. 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? Yes this happens if xform is null, so only code under if (xform == null) was changed. 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"? This is related to 7160609. The problem was in fact, that we create huge texture during window initialization. In the fix I change this logic to use GL_MAX_TEXTURE_SIZE/2. Separate CR:8010999 was created to track it. You removed a call to flushOnScreenGraphics in LWWindowPeer. Was that just being paranoid before and really isn't needed? Now it is called not only when the window is resized, but when the surface is replaced. It was moved to replaceSurfaceData. 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...) When it was written, I was impressed by a code in the 2D. ...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/viewbug.do?bugid=8000629 Webrev can be found at: http://cr.openjdk.java.net/~serb/8000629/webrev.06 -- Best regards, Sergey.

-- Best regards, Sergey. -- Best regards, Sergey.

-- Best regards, Sergey.

-------------- next part -------------- An HTML attachment was scrubbed... URL: http://mail.openjdk.java.net/pipermail/awt-dev/attachments/20130409/ad85b34d/attachment.html



More information about the awt-dev mailing list