On 11/6/13 5:19 AM, Alexander Scherbatiy wrote:
On 11/6/2013 5:39 AM, Jim Graham wrote:
Why is getScaledInstance() being consulted here? It seems a misuse of
that method.
We need to introduce a new API that allows developers to return
scaled images for HiDPI displays.
Then we should introduce new API.
Scaled images are just new images N times bigger than the original
one and created by designers.
No, they are different images for the same content that are designed to
have more or less detail for a given resolution from the start.
The good way is to allow a user to override a method like
getScaledImage() which has either scaledFactor or width and height
parameters and returns necessary scaled images.
If that is a new API with new semantics, then I agree.
But the getScaledImage(width, height) method looks similar to the
getScaledInstance(width, height, hints). It also can confuse a user
which method should be overridden to return
the scaled version of an image.
A better name would help. I don't think width/height are really the
best parameters for such a method since you want an image that can be
substituted and the original image has already established an aspect
ratio. A single resolution would have been preferable.
The current fix is based on the CCC request 8011059 where it has
been approved to use the getScaledInstance(width, height, hints) method
for this purposes.
Note that in addition to the issues I raise wherein that method was
never intended to support this new behavior, and the fact that a hint on
G2D would cause that method on *all* images to be executed whether or
not that image is one of the images that a developer needed the
resolution support, this method also suffers from having a set of
arguments that are poorly applicable to this operation. The fact that
the API request had to mention which type of pixel replication algorithm
would be used to mean "we really actually don't want pixel replication
in the first place" is a pretty good indicator that this API is not up
to the task.
The method was designed to return a rescaled version of the same
pixels that you would get if you examined the raw pixels. You are
overriding it to return a different image. That does not fit in with
the original intent of that method as I created it.
It is also causing implementation headaches (read reflection) in the
SG2D code to try to use it that way.
The Image.SCALE_DEFAULT hint is used to retrieve user scaled
images in the CCC request 8011059.
That was a wrong idea. There are intersections with the original
meaning with the SCALE_DEFAULT hint.
There should be no scaling hints in the first place for an alternate
resolution source.
Also there are performance regression and exceptions like OOM or
IOBE if the getScaledInstance(width, height, hints) is not overridden
and it is invoked for each image.
And there are 10 years of potential overrides that developers could have
created that could be even worse than the default implementation from a
performance perspective. "Whether or not the method is overridden" is a
bad heuristic from the get go and it is much, much more inappropriate to
apply to a legacy method that has been in the wild for over a decade.
The better way could be to use a new Image hint designed only to
retrieve scaled images provided by a user.
I have created the CCC request 8027655 for this.
Once the request will be approved it will be possible to get rid
of the reflection usage.
The suggestion to add a new hint to a decade old API is unwise.
Additionally, the hint requested "SCALE_CUSTOM" could mean anything and
is documented so loosely that resolution swapping is only one of an
infinite number of responses an application could make. The spec of
that hint also violates the contract of the getScaledInstance() method
which states that the returned instance will be of the requested size,
but you are planning on returning an image that can have a different size.
Allowing developers to return a substitute image for a different screen
resolution is a good goal, but none of these API hacks are really suited
to the task and they are attempting to leverage a very dated legacy API
for a purpose that it was expressly designed not to do in the first place...
...jim
The @2x mechanism should be based on different API. I guess it would
have to be internal-only for 8.0 and could be exposed to allow
developers to call it and possibly to be a provider for it in JDK9...
...jim
On 10/31/13 9:19 AM, Alexander Scherbatiy wrote:
Could you review the updated fix:
http://cr.openjdk.java.net/~alexsch/8011059/webrev.04/
The reflection is used to skip the Image.getScaledInstance() method
call if it is not overridden by a user.
On 10/29/2013 11:08 PM, Sergey Bylokhov wrote:
Hi, Alexander.
The fix looks fine to me in general. But there is at least one issue.
I build you fix and test it:
- Consuming of cpu increased by 500 times Java2Demo on images tab.
- FPS is dropped from 220(jdk8)/35(jdk7u40) to 15 in guimark2. Note
that jdk6 has the same FPS(15) on my system.
The main problem is that the Image.SCALE_DEFAULT hint is used to
retrieve a scaled image from Image.getScaledInstance() method.
It always uses the ReplicateScaleFilter for images which
getScaledInstance() method has not been overridden.
The ReplicateScaleFilter creates a lot of arrays and consumes the
CPU during the image parsing.
The better fix would be to introduce the new Image.SCALE_CUSTOM hint
which could be used to get a scaled image and does not use filters by
default.
But it should be a separated bug with a new CCC request.
Thanks,
Alexandr.
On 29.10.2013 20:45, Alexander Scherbatiy wrote:
Could you review the updated fix:
http://cr.openjdk.java.net/~alexsch/8011059/webrev.03
On 10/28/2013 2:33 PM, Artem Ananiev wrote:
Hi, Alexander,
a few comments:
1. SunGraphics2D.java:3076 - should isHiDPIImage() be used here?
The isHiDPIImage() method is used to check that the
drawHiDPIImage should be called like:
if (isHiDPIImage(img)) {
return drawHiDPIImage(...);
}
2. I'm not sure that the proposed getScaledImageName()
implementation in ScalableToolkitImage works perfectly for URLs like
this:
http://www.exampmle.com/dir/image
In this case it will try to find 2x image here:
http://www.exam...@2x.com/dir/image
which doesn't look correct.
Fixed. Only path part of a URL is converted to path2x.
3. RenderingHints spec references Retina or non-Retina displays,
which should be removed.
Fixed.
- devScale is used instead of transform parsing in the
drawHiDPIImage() method just to not have performance regression more
than 2 times on HiDPI displays
- LWCToolkit.ScalableToolkitImage is made public for the fix
8024926 [macosx] AquaIcon HiDPI support.
Thanks,
Alexandr.
Thanks,
Artem
On 10/25/2013 5:18 PM, Alexander Scherbatiy wrote:
Could you review the updated fix:
http://cr.openjdk.java.net/~alexsch/8011059/webrev.02/
- Scaled image width and height are transformed according to the
AffineTransform type.
- ToolkitImage subclass is used to hold @2x image instance.
Thanks,
Alexandr.
On 10/23/2013 7:24 PM, Alexander Scherbatiy wrote:
Could you review the updated fix:
http://cr.openjdk.java.net/~alexsch/8011059/webrev.01/
The JCK failures has been resolved:
- Some tests tries to draw an image with Integer.MAX_VALUE
width
or height. Passing large values to image.getScaledImage(width,
height,
hints).
leads that an Image filter is not able to create necessary
arrays. The fix uses the original image if width or height are
equal
to Integer.MAX_VALUE.
- Using Image.SCALE_DEFAULT hint for the getScaledImage(width,
height, hints) method to get the high resolution image interferes
with
JCK tests that expect that the scaled image by certain
algorithm is returned. This is fixed by invoking the
super.getScaledImage(width, height, hints)
method in ToolkitImage in case if a high resolution image is
not set.
Thanks,
Alexandr.
On 10/22/2013 1:31 PM, Alexander Scherbatiy wrote:
Hello,
Could you review the fix:
bug: https://bugs.openjdk.java.net/browse/JDK-8011059
webrev: http://cr.openjdk.java.net/~alexsch/8011059/webrev.00
The IMAGE_SCALING rendering hint is added to the RenderingHints
class.
Enabling the image scaling rendering hint forces the
SunGraphics2D
to use getScaledInstance(width, height, hints) method
from Image class with SCALE_DEFAULT hint.
By default the image scaling rendering hint is enabled on HiDPI
display and disabled for standard displays.
User can override the getScaledInstance(width, height, hints)
method and return necessary high resolution image
according to the given image width and height.
For example:
---------------------
final Image highResolutionImage =
new BufferedImage(2 * WIDTH, 2 * HEIGHT,
BufferedImage.TYPE_INT_RGB);
Image image = new BufferedImage(WIDTH, HEIGHT,
BufferedImage.TYPE_INT_RGB) {
@Override
public Image getScaledInstance(int width, int height,
int
hints) {
if ((hints & Image.SCALE_DEFAULT) != 0) {
return (width <= WIDTH && height <= HEIGHT)
? this : highResolutionImage;
}
return super.getScaledInstance(width, height,
hints);
}
};
---------------------
The LWCToolkit and ToolkitImage classes are patched to
automatically get provided im...@2x.ext images on MacOSX.
There are no significant changes in the Java2D demo to make it
look
perfect on Retina displays.
It needs only to put necessary images with the @2x postfix and
they
will be automatically drawn.
Thanks,
Alexandr.
