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

[Denis S. Fokin]

Hi Sergey,

The fix looks ok. But I have two ideas.

1.)  We use everywhere 1 as a measure of scale.Would not it be better to 
use a constant at least for integer values? Actually, as far as we 
dealing with 1.0 we should not have any issues with precision. On the 
other hand, the widening primitive conversion can lead to a performance 
impact. But at least for integers we could introduce a constant.

CGLLayer.java
48     private int scale = 1;


SurfaceData.java
1068         return 1;

SunGraphics2D.java
262         if (devScale != 1) {
1639         final double invScale = 1.0 / devScale;

SurfaceManager.java
300             return 1;


Region.java
143         if (sv == 1.0) {
381         if ((sx == 1.0 && sy == 1.0) || (this == WHOLE_REGION)) {


CGraphicsDevice.m
328     __block jdouble ret = 1.0f;

The name for constant is a tricky question. Maybe NOT_SCALED or 
something like that.

2.)

I would really like to have a comment here
457         return getMaxTextureSize() / (getDevice().getScaleFactor() * 2);

why not

CGLGraphicsConfig.java
457         return (getMaxTextureSize() / (getDevice().getScaleFactor()) 
- 1);

at least a minor comment.


Thank you,
    Denis.




On 3/28/2013 3:14 PM, Sergey Bylokhov wrote:
> On 3/28/13 1:04 PM, Denis S. Fokin wrote:
>> Hi Sergey,
>>
>>    actually, the round function has a little bit more complicated
>> implementation because of 6430675.
> Here is the new version:
> http://cr.openjdk.java.net/~serb/8000629/webrev.07
>>
>> Thank you,
>>    Denis.
>>
>> On 3/27/2013 7:15 PM, Sergey Bylokhov wrote:
>>> On 3/27/13 4:12 PM, Denis S. Fokin wrote:
>>>> Hi Sergey,
>>>>
>>>> why we do not use Math.round() here?
>>>> Region.java:
>>>> 153         return (int) Math.floor(newv + 0.5);
>>> Just because it one additional call.
>>>>
>>>> Thank you,
>>>>    Denis.
>>>>
>>>>
>>>> On 3/26/2013 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.
>>>>>
>>>>
>>>
>>>
>>
>
>