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.
