[OpenJDK 2D-Dev] <AWT Dev> [8] Request for review: 8000629 [macosx] Blurry rendering with Java 7 on Retina display

[Sergey Bylokhov]

Hi, Jim.
During additional testing I found some artifacts in some animation, when 
the rendering to the scaled VolatileImge is different from the non 
scaled variant. Can you please take a look to the CR:8011764.

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?  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.
>>


-- 
Best regards, Sergey.