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.
