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

[Jim Graham]

I don't think the constant would add any value and would just cause someone looking over the code to wonder what kind of magical value means NO_SCALE when the value is so obvious.  I think anyone can realize that 1 or 1.0 means "not scaling" with about 2 neurons firing.  The mystery created by hiding it behind a constant would just cause all sorts of thoughts to arise that really have no value or bearing on the work being done...

			...jim

On 3/28/2013 11:34 AM, Denis S. Fokin wrote:
> 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.
>>>>>>
>>>>>
>>>>
>>>>
>>>
>>
>>
>