<AWT Dev> [OpenJDK 2D-Dev] [8] Review request for 8011059 [macosx] Make JDK demos look perfect on retina displays
Sergey Bylokhov
Sergey.Bylokhov at oracle.com
Wed Nov 6 17:20:32 PST 2013
On 07.11.2013 4:09, Jim Graham wrote:
> On 11/6/13 3:09 PM, Sergey Bylokhov wrote:
>> Hi , Jim.
>> On 07.11.2013 1:40, Jim Graham wrote:
>>> 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.
>> Well during draw of the image we should request appropriate scale image
>> from the user, who possible know better how its image can be scaled.
>> What can be better, than a method which "Creates a scaled version of
>> this image."?
>
> That method "returns a scaled version of *this* image". It
> specifically refers to recombining the pixels of the image you would
> normally get into a new image that, by default, renders at the
> requested size.
>
> The method has parameters to control the pixel replication/elimination
> *algorithm*.
Custom hint can be renamed to the BEST_EVER_QUALITY_AS_IMAGE_CANhint and
write in doc that it is nop by default..
>
> This is a pixel manipulation method pure and simple. It is also not
> anywhere documented that it is used by drawImage(), in fact it is
> documented that the developer call this method and then the result is
> for the developer to use when they call drawImage().
>
> drawImage() should *NOT* be using this method on its own, and the
> method should not be involved in choosing alternate designs. It is
> for manually resampling an image into a new permanent image.
Why not? We can document that. It is something related to
|RenderingHints.KEY_INTERPOLATION| which was added as an alternate of
image hints. But this new hints do not provide possibility to customize
scaling by the user.
> But it can be used independently, if the graphics is scaled in one
>> direction. And to allow the user to decide how to scale the picture
>> depending on how it will be expanded. And there is no direct
>> relationship with retina and its scale factor.
>
> Currently we only have one case of HiDPI displays (retina) and they
> have the same pixel scaling in both directions. It is true that a
> more complete API would allow for having different resolutions in X
> and Y, though I'm not sure that will ever be a case needed in practice.
>
>> We can move deeper and request here the actual size of the image
>> including transform of the graphics (<=TRANSFORM_TRANSLATESCALE). So the
>> user can apply IMAGE_SCALING hint to the transformed sg2d and all images
>> will have a chance to use custom scale instead of some scaling from the
>> sg2d.
>
> I see no reason for this. We are trying to solve image fidelity for
> retina displays here, (and hopefully whatever Windows comes up with
> for similar purposes). The graphics object already has plenty of
> information at its disposal to determine how to scale images.
>
> I also don't think we want to take the transform into consideration.
> We are either "on a hi res display" or we are not on such a display
> and we should use the variant of the image that is appropriate for
> that display, even if they are scaled up or down by 2x or more - we
> don't want an image to suddenly change its look just because they are
> animating its size (though we could offer a hint in the future that
> might indicate that they would like us to do that, I don't think it
> should be the standard or default behavior and I am not sure anyone
> would ever actually want it in the first place as it creates a visual
> discontinuity in what should be a smooth seamless process).
>
>>> 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.
>> If user overrides this method and provides some specific scale algorithm
>> is not what we want from the beginning?
>
> No, that method is provided for developers to call to get "scaled
> versions" of an image. If they've provided some custom code - of
> unknown performance capability - in their libraries and apps then they
> have probably not expected this to ever be used in any automatic sense
> by drawImage().
>
>> "Whether or not the method is overridden" come in to play because
>> default scale operation in SG2D and the results of default
>> implementation of getScaledInstance is quite similar, but performance is
>> extremely different.
>> So it can be rephrased as "Whether or not the default scale algorithm is
>> use", so in the fix we use default from the S2G2 not Image.
> Fact: getScaledInstance() *MUST ALWAYS* return an image that is the
> requested WxH. It has no other option. you can't override that *API
> REQUIREMENT* in the documentation of a hint. Read the documentation
> for it: "A new Image object is returned which will render the image at
> the specified width and height by default."
Yes, I see. The image should be "new" and it does not say it should be
the same object. And content of this "new" image depends from the hint
which was passed to the method.
> In other words, if you call g.drawImage(thenewscaledinstance, x, y)
> (i.e. the version with no w,h parameters in the drawImage() signature)
> then it will draw at the size requested when "thenewscaledinstance"
> was created.
>
>> In general an idea of the fix is: The image know better how it could be
>> scaled
>> - It could use some pixel replicate scale filters
>> - it could use different files images
>> - it could redraw itself from scratch if it use vectors graphics.
>> The only parameters it needed is width and height.
>
> You are not understanding that it is a new scaled version of *THIS*
> image. The pixels from the original image must be used. This is about
> pixel replication algorithms, not about redesigning a representation.
The user know what pixels contains in its image, and he know how to
scale its better.
>
>> The CUSTOM hint is used for two reasons
>> - to use default scale from graphics and to skip default in image
>> - to explicitly document new usage of getScaledInstance().
>>
>> And additionally about compatibility you can compare this version of the
>> fix and version of jdk7(where demos was rewritten). If I understand it
>> correctly nothing should be changed in demos in the current version of
>> the fix(except new x2 images.)
>> http://closedjdk.us.oracle.com/jdk7u/jdk7u-dev/jdk/src/closed/rev/335cba03699d
>>
>
> I am not suggesting that we change the demos. I am suggesting that we
> handle this internally for now with an option to provide a developer
> to inspect and/or intercept the process when we can create new API.
>
> In other words:
>
> package sun.awt.image;
>
> public interface MultiResImage {
> public Image getResolutionVariant(float resolution);
> }
>
> public class MacImage extends Image implements MultiResImage {
> public Image getResolutionVariant(float resolution) {
> if (resolution >= 2f && ImageAt2x != null) {
> return ImageAt2x;
> }
> return this;
> }
> }
>
> SG2D.drawImage() {
> if (img instanceof MultiResImage && dest.pixelscale != 1) {
> img = ((MultiResImage)
> img).getResolutionVariant(dest.pixelscale);
> }
> }
In this example there is a problem. For example we have 2
BufferedImages/ToolkiImagest A and B; Both wants be scaled perfectly.
- Image A draws to the image B
- Image B draws to the window.
When window is moving from the screen x1 to the screen x2 and back. How
to handle this situation?
In this case getResolutionVariant() can return
Related discussion:
http://mail.openjdk.java.net/pipermail/macosx-port-dev/2013-April/005580.html
http://mail.openjdk.java.net/pipermail/macosx-port-dev/2013-April/005581.html
>
> ...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.example@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 image at 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.
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>
>>
>>
--
Best regards, Sergey.
More information about the macosx-port-dev
mailing list