Quantcast

Review request for 8029339 Custom MultiResolution image support on HiDPI displays

classic Classic list List threaded Threaded
31 messages Options
12
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Review request for 8029339 Custom MultiResolution image support on HiDPI displays

Alexandr Scherbatiy

   Hi Phil,

   I have prepared two versions of the proposed API:

   I) Resolution variants are added directly to the Image:
    http://cr.openjdk.java.net/~alexsch/8029339/list/webrev.00

   II)  MultiResolutionImage interface is used:
     http://cr.openjdk.java.net/~alexsch/8029339/webrev.05

   It could help to decide which way should be chosen for the the
multi-resolution image support.

   Below are some comments:

   1. High level goal:
      Introduce an API that allows to create and handle an image with
resolution variants.

   2. What is not subject of the provided API
     - Scale naming convention for high-resolution images
     - Providing pixel scale factor for the screen/window

   3. Use cases
    3.1 Loading and drawing high-resolution icons in IntelliJ IDEA
      A high-resolution image is loaded from resources and stored in
JBHiDPIScaledImage class  which is a subclass of the buffered image.
      The high-resolution image is used to create a disabled icon in the
IconLoader.getDisabledIcon(icon) method.
https://github.com/JetBrains/intellij-community/blob/master/platform/util/src/com/intellij/openapi/util/IconLoader.java

    3.2 Loading and drawing high-resolution icons in NetBeans
      NetBeans does not have support for the high-resolution icons loading.
      It loads an icon from the file system using
Toolkit.getDefaultToolkit().getImage(url) method or from resources
      by  ImageReader  and store it in ToolTipImage class which is
subclass of the buffered image.
      ImageUtilities.createDisabledIcon(icon) method creates a disabled
icon by applying  RGBImageFilter to the icon.
http://hg.netbeans.org/main/file/97dcf49eb4a7/openide.util/src/org/openide/util/ImageUtilities.java

    3.3 Loading system icons in JDK 1.8
      JDK requests icons from the native system for system L&Fs and
applies filters for them.
      See for example AquaUtils.generateLightenedImage() method:
http://hg.openjdk.java.net/jdk9/client/jdk/file/e6f48c4fad38/src/java.desktop/macosx/classes/com/apple/laf/AquaUtils.java

   4. HiDPI support for Images on different OSes

     4.1 Mac OS X
       Cocoa API contains NSImage that allows to work with image
representations: add/remove/get all representations.
       It picks up an image with necessary resolution based on the
screen backing store pixel scale factor and applied transforms.
https://developer.apple.com/library/mac/documentation/Cocoa/Reference/ApplicationKit/Classes/NSImage_Class/Reference/Reference.html

     4.2 Linux
       GTK+ 3 API has gtkcssimagescaled lib (it seems that it is not
public/stable)
       that parses the -gtk-scaled css property and draws a GtkCssImage
according to the given scale factor.

       I have not found information about the HiDPI support in Xlib.

     4.3 Windows
       I have only found the tutorial that suggests to select and draw a
bitmap using the queried DPI
       and scale the coordinates for drawing a rectangular frame
http://msdn.microsoft.com/en-us/library/dd464659%28v=vs.85%29.aspx

       Windows also provides the horizontal and vertical DPI of the desktop
http://msdn.microsoft.com/en-us/library/windows/apps/dd371316

   5. Pseudo API
      Below are some ways which illustrates how multi-resolution images
can be created and used.

     5.1 Resolution variants are stored directly in Image class.
     To query a resolution variant it needs to compare the resolution
variant width/height
     with the requested high-resolution size.
     ------------
     public abstract class Image {

         public void addResolutionVariant(Image image) {...}
         public List<Image> getResolutionVariants() {...}
     }
     ------------
     // create a disabled image with resolution variants

     Image disabledImage = getDisabledImage(image);

     for (Image rv : image.getResolutionVariants()) {
         disabledImage.addResolutionVariant(getDisabledImage(rv));
     }
     ------------
     This approach requires that all resolution variants have been
created even not of them are really used.

     5.2  Resolution variants are stored in a separate object that
allows to create them by demand.
     To query a resolution variant it needs to compare the resolution
variant scale factor
     with the requested scale (that can include both screen DPI scale
and applied transforms).
     ------------
     public abstract class Image {

         public static interface ResolutionVariant {
             Image getImage();
             float getScaleFactor();
         }

         public void addResolutionVariant(ResolutionVariant
resolutionVariant) {...}
         public List<ResolutionVariant> getResolutionVariants() {...}
     }
     ------------
     // create a disabled image with resolution variants
     Image disabledImage = getDisabledImage(image);

     for (Image.ResolutionVariant rv : image.getResolutionVariants()) {
         disabledImage.addResolutionVariant(new Image.ResolutionVariant() {

             public Image getImage() {
                 return getDisabledImage(rv.getImage());
             }

             public float getScaleFactor() {
                 return rv.getScaleFactor();
             }
         });
     }
     ------------

     It does not have problem if a predefined set of images is provided
(like image.png and [hidden email] on the file system).
     This does not cover cases where a resolution variant can be created
using the exact requested size (like loading icons from the native system).
     A resolution variant can be queried based on a scale factor and
applied transforms.

     5.3 The provided example allows to create a resolution variant
using the requested high-resolution image size.
     ------------
     public interface MultiResolutionImage {
         Image getResolutionVariant(float width, float height);
     }
     ------------
     // create a multi-resolution image
     Image mrImage = new AbstractMultiResolutionImage() {

             public Image getResolutionVariant(float width, float height) {
                 // create and return a resolution variant with exact
requested width/height size
             }

             protected Image getBaseImage() {
                 return baseImage;
             }
         };
     ------------
     // create a disabled image with resolution variants
     Image disabledImage = null;
     if (image instanceof MultiResolutionImage) {
         final MultiResolutionImage mrImage = (MultiResolutionImage) image;
         disabledImage = new AbstractMultiResolutionImage(){

             public Image getResolutionVariant(float width, float height) {
                 return
getDisabledImage(mrImage.getResolutionVariant(width, height));
             }

             protected Image getBaseImage() {
                 return getDisabledImage(mrImage);
             }
         };
     } else {
         disabledImage = getDisabledImage(image);
     }
     ------------

   Thanks,
   Alexandr.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Review request for 8029339 Custom MultiResolution image support on HiDPI displays

Jim Graham-5
Hi Alexander,

The code that lets someone modify an existing image has an important and
undesirable side effect of making images mutable.  Since we cache images
internally, they are treated as immutable so that if two unrelated
parties ask for an image that comes from a specific URL, we can give
them back the same object (with a bit of code internally to make sure
that each and both of them have permissions to access the specified
URL/filename/source).  So, the first solution that adds a new method
onto Image that lets someone add a resolution variant after the fact is
not feasible.  (Arguably, we could override and sabotage attempts to add
new variants, but that style of providing an API that is implemented in
a common case by blocking code is not very interesting.)

The version that provides an interface allows us to implement automatic
loading of resolution variants internally and expose that information in
a read-only fashion without making a cached Image object (or its
resolution variants) mutable - as long as we only ever provide immutable
collections populated by other immutable images.  It can be combined
with an Image subclass variant that lets a developer specify their own
list of images, and potentially even add more images to that list on the
fly after the image is created (because it is not shared by a hidden
mechanism since they created it directly).  We could probably add that
to BufferedImage since those are not shared, but we'd have to make sure
that shared "Toolkit images" don't subclass from BufferedImage or expose
that through their API - that's probably already true otherwise we'd
have provided a way for someone to scribble on a shared image.

I'll look into the second proposal (the interface variant) in a little
more detail, but I wanted to get this basic comment out there in advance
of more specific feedback...

                        ...jim

On 1/22/2015 6:49 AM, Alexander Scherbatiy wrote:

>
>    Hi Phil,
>
>    I have prepared two versions of the proposed API:
>
>    I) Resolution variants are added directly to the Image:
>     http://cr.openjdk.java.net/~alexsch/8029339/list/webrev.00
>
>    II)  MultiResolutionImage interface is used:
>      http://cr.openjdk.java.net/~alexsch/8029339/webrev.05
>
>    It could help to decide which way should be chosen for the the
> multi-resolution image support.
>
>    Below are some comments:
>
>    1. High level goal:
>       Introduce an API that allows to create and handle an image with
> resolution variants.
>
>    2. What is not subject of the provided API
>      - Scale naming convention for high-resolution images
>      - Providing pixel scale factor for the screen/window
>
>    3. Use cases
>     3.1 Loading and drawing high-resolution icons in IntelliJ IDEA
>       A high-resolution image is loaded from resources and stored in
> JBHiDPIScaledImage class  which is a subclass of the buffered image.
>       The high-resolution image is used to create a disabled icon in the
> IconLoader.getDisabledIcon(icon) method.
> https://github.com/JetBrains/intellij-community/blob/master/platform/util/src/com/intellij/openapi/util/IconLoader.java
>
>
>     3.2 Loading and drawing high-resolution icons in NetBeans
>       NetBeans does not have support for the high-resolution icons loading.
>       It loads an icon from the file system using
> Toolkit.getDefaultToolkit().getImage(url) method or from resources
>       by  ImageReader  and store it in ToolTipImage class which is
> subclass of the buffered image.
>       ImageUtilities.createDisabledIcon(icon) method creates a disabled
> icon by applying  RGBImageFilter to the icon.
> http://hg.netbeans.org/main/file/97dcf49eb4a7/openide.util/src/org/openide/util/ImageUtilities.java
>
>
>     3.3 Loading system icons in JDK 1.8
>       JDK requests icons from the native system for system L&Fs and
> applies filters for them.
>       See for example AquaUtils.generateLightenedImage() method:
> http://hg.openjdk.java.net/jdk9/client/jdk/file/e6f48c4fad38/src/java.desktop/macosx/classes/com/apple/laf/AquaUtils.java
>
>
>    4. HiDPI support for Images on different OSes
>
>      4.1 Mac OS X
>        Cocoa API contains NSImage that allows to work with image
> representations: add/remove/get all representations.
>        It picks up an image with necessary resolution based on the
> screen backing store pixel scale factor and applied transforms.
> https://developer.apple.com/library/mac/documentation/Cocoa/Reference/ApplicationKit/Classes/NSImage_Class/Reference/Reference.html
>
>
>      4.2 Linux
>        GTK+ 3 API has gtkcssimagescaled lib (it seems that it is not
> public/stable)
>        that parses the -gtk-scaled css property and draws a GtkCssImage
> according to the given scale factor.
>
>        I have not found information about the HiDPI support in Xlib.
>
>      4.3 Windows
>        I have only found the tutorial that suggests to select and draw a
> bitmap using the queried DPI
>        and scale the coordinates for drawing a rectangular frame
> http://msdn.microsoft.com/en-us/library/dd464659%28v=vs.85%29.aspx
>
>        Windows also provides the horizontal and vertical DPI of the desktop
> http://msdn.microsoft.com/en-us/library/windows/apps/dd371316
>
>    5. Pseudo API
>       Below are some ways which illustrates how multi-resolution images
> can be created and used.
>
>      5.1 Resolution variants are stored directly in Image class.
>      To query a resolution variant it needs to compare the resolution
> variant width/height
>      with the requested high-resolution size.
>      ------------
>      public abstract class Image {
>
>          public void addResolutionVariant(Image image) {...}
>          public List<Image> getResolutionVariants() {...}
>      }
>      ------------
>      // create a disabled image with resolution variants
>
>      Image disabledImage = getDisabledImage(image);
>
>      for (Image rv : image.getResolutionVariants()) {
>          disabledImage.addResolutionVariant(getDisabledImage(rv));
>      }
>      ------------
>      This approach requires that all resolution variants have been
> created even not of them are really used.
>
>      5.2  Resolution variants are stored in a separate object that
> allows to create them by demand.
>      To query a resolution variant it needs to compare the resolution
> variant scale factor
>      with the requested scale (that can include both screen DPI scale
> and applied transforms).
>      ------------
>      public abstract class Image {
>
>          public static interface ResolutionVariant {
>              Image getImage();
>              float getScaleFactor();
>          }
>
>          public void addResolutionVariant(ResolutionVariant
> resolutionVariant) {...}
>          public List<ResolutionVariant> getResolutionVariants() {...}
>      }
>      ------------
>      // create a disabled image with resolution variants
>      Image disabledImage = getDisabledImage(image);
>
>      for (Image.ResolutionVariant rv : image.getResolutionVariants()) {
>          disabledImage.addResolutionVariant(new Image.ResolutionVariant() {
>
>              public Image getImage() {
>                  return getDisabledImage(rv.getImage());
>              }
>
>              public float getScaleFactor() {
>                  return rv.getScaleFactor();
>              }
>          });
>      }
>      ------------
>
>      It does not have problem if a predefined set of images is provided
> (like image.png and [hidden email] on the file system).
>      This does not cover cases where a resolution variant can be created
> using the exact requested size (like loading icons from the native system).
>      A resolution variant can be queried based on a scale factor and
> applied transforms.
>
>      5.3 The provided example allows to create a resolution variant
> using the requested high-resolution image size.
>      ------------
>      public interface MultiResolutionImage {
>          Image getResolutionVariant(float width, float height);
>      }
>      ------------
>      // create a multi-resolution image
>      Image mrImage = new AbstractMultiResolutionImage() {
>
>              public Image getResolutionVariant(float width, float height) {
>                  // create and return a resolution variant with exact
> requested width/height size
>              }
>
>              protected Image getBaseImage() {
>                  return baseImage;
>              }
>          };
>      ------------
>      // create a disabled image with resolution variants
>      Image disabledImage = null;
>      if (image instanceof MultiResolutionImage) {
>          final MultiResolutionImage mrImage = (MultiResolutionImage) image;
>          disabledImage = new AbstractMultiResolutionImage(){
>
>              public Image getResolutionVariant(float width, float height) {
>                  return
> getDisabledImage(mrImage.getResolutionVariant(width, height));
>              }
>
>              protected Image getBaseImage() {
>                  return getDisabledImage(mrImage);
>              }
>          };
>      } else {
>          disabledImage = getDisabledImage(image);
>      }
>      ------------
>
>    Thanks,
>    Alexandr.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Review request for 8029339 Custom MultiResolution image support on HiDPI displays

Jim Graham-5
In reply to this post by Alexandr Scherbatiy
The second solution looks good.  I'd make it standard practice (perhaps
even mentioned in the documentation) to return unmodifiable lists from
the getVariants() method.  The Collections class provides easy methods
to create these lists, and it sends a clear message to the caller that
the list was provided for them to read, but not write to.  Otherwise
they may add a new image to the list you provided them and wonder why it
wasn't showing up.  Also, an unmodifiable list can be cached and reused
for subsequent calls without having to create a new list every time.

In getResolutionVariant() was there a reason why the base dimensions
were provided as float?  The destination dimensions make sense as float
since they could be the result of a scale, but the source dimensions are
typically getWidth/getHeight() on the base image.  A related question
would be if they are needed at all, since the implementation should
probably already be aware of what the base image is and what its
dimensions are.  I'm guessing they are provided because the
implementation in SG2D already knows them and it makes it easier to
forward the implementation on to a shared (static?) method?

With respect to default implementations, I take it that the BaseMRI is
along the pattern that we see in Swing for Base classes.  Would it be
helpful to provide an implementation (in addition or instead) that
allows a developer to take a bunch of images and quickly make an MRI
without having to override anything?  The implementations of
getBaseImage() and getResolutionVariants() are pretty straightforward
and a fairly reasonable default algorithm can be provided for
getRV(dimensions).  This question is more of an idle question for my own
curiosity than a stumbling block...

                        ...jim

On 1/22/2015 6:49 AM, Alexander Scherbatiy wrote:

>
>    Hi Phil,
>
>    I have prepared two versions of the proposed API:
>
>    I) Resolution variants are added directly to the Image:
>     http://cr.openjdk.java.net/~alexsch/8029339/list/webrev.00
>
>    II)  MultiResolutionImage interface is used:
>      http://cr.openjdk.java.net/~alexsch/8029339/webrev.05
>
>    It could help to decide which way should be chosen for the the
> multi-resolution image support.
>
>    Below are some comments:
>
>    1. High level goal:
>       Introduce an API that allows to create and handle an image with
> resolution variants.
>
>    2. What is not subject of the provided API
>      - Scale naming convention for high-resolution images
>      - Providing pixel scale factor for the screen/window
>
>    3. Use cases
>     3.1 Loading and drawing high-resolution icons in IntelliJ IDEA
>       A high-resolution image is loaded from resources and stored in
> JBHiDPIScaledImage class  which is a subclass of the buffered image.
>       The high-resolution image is used to create a disabled icon in the
> IconLoader.getDisabledIcon(icon) method.
> https://github.com/JetBrains/intellij-community/blob/master/platform/util/src/com/intellij/openapi/util/IconLoader.java
>
>
>     3.2 Loading and drawing high-resolution icons in NetBeans
>       NetBeans does not have support for the high-resolution icons loading.
>       It loads an icon from the file system using
> Toolkit.getDefaultToolkit().getImage(url) method or from resources
>       by  ImageReader  and store it in ToolTipImage class which is
> subclass of the buffered image.
>       ImageUtilities.createDisabledIcon(icon) method creates a disabled
> icon by applying  RGBImageFilter to the icon.
> http://hg.netbeans.org/main/file/97dcf49eb4a7/openide.util/src/org/openide/util/ImageUtilities.java
>
>
>     3.3 Loading system icons in JDK 1.8
>       JDK requests icons from the native system for system L&Fs and
> applies filters for them.
>       See for example AquaUtils.generateLightenedImage() method:
> http://hg.openjdk.java.net/jdk9/client/jdk/file/e6f48c4fad38/src/java.desktop/macosx/classes/com/apple/laf/AquaUtils.java
>
>
>    4. HiDPI support for Images on different OSes
>
>      4.1 Mac OS X
>        Cocoa API contains NSImage that allows to work with image
> representations: add/remove/get all representations.
>        It picks up an image with necessary resolution based on the
> screen backing store pixel scale factor and applied transforms.
> https://developer.apple.com/library/mac/documentation/Cocoa/Reference/ApplicationKit/Classes/NSImage_Class/Reference/Reference.html
>
>
>      4.2 Linux
>        GTK+ 3 API has gtkcssimagescaled lib (it seems that it is not
> public/stable)
>        that parses the -gtk-scaled css property and draws a GtkCssImage
> according to the given scale factor.
>
>        I have not found information about the HiDPI support in Xlib.
>
>      4.3 Windows
>        I have only found the tutorial that suggests to select and draw a
> bitmap using the queried DPI
>        and scale the coordinates for drawing a rectangular frame
> http://msdn.microsoft.com/en-us/library/dd464659%28v=vs.85%29.aspx
>
>        Windows also provides the horizontal and vertical DPI of the desktop
> http://msdn.microsoft.com/en-us/library/windows/apps/dd371316
>
>    5. Pseudo API
>       Below are some ways which illustrates how multi-resolution images
> can be created and used.
>
>      5.1 Resolution variants are stored directly in Image class.
>      To query a resolution variant it needs to compare the resolution
> variant width/height
>      with the requested high-resolution size.
>      ------------
>      public abstract class Image {
>
>          public void addResolutionVariant(Image image) {...}
>          public List<Image> getResolutionVariants() {...}
>      }
>      ------------
>      // create a disabled image with resolution variants
>
>      Image disabledImage = getDisabledImage(image);
>
>      for (Image rv : image.getResolutionVariants()) {
>          disabledImage.addResolutionVariant(getDisabledImage(rv));
>      }
>      ------------
>      This approach requires that all resolution variants have been
> created even not of them are really used.
>
>      5.2  Resolution variants are stored in a separate object that
> allows to create them by demand.
>      To query a resolution variant it needs to compare the resolution
> variant scale factor
>      with the requested scale (that can include both screen DPI scale
> and applied transforms).
>      ------------
>      public abstract class Image {
>
>          public static interface ResolutionVariant {
>              Image getImage();
>              float getScaleFactor();
>          }
>
>          public void addResolutionVariant(ResolutionVariant
> resolutionVariant) {...}
>          public List<ResolutionVariant> getResolutionVariants() {...}
>      }
>      ------------
>      // create a disabled image with resolution variants
>      Image disabledImage = getDisabledImage(image);
>
>      for (Image.ResolutionVariant rv : image.getResolutionVariants()) {
>          disabledImage.addResolutionVariant(new Image.ResolutionVariant() {
>
>              public Image getImage() {
>                  return getDisabledImage(rv.getImage());
>              }
>
>              public float getScaleFactor() {
>                  return rv.getScaleFactor();
>              }
>          });
>      }
>      ------------
>
>      It does not have problem if a predefined set of images is provided
> (like image.png and [hidden email] on the file system).
>      This does not cover cases where a resolution variant can be created
> using the exact requested size (like loading icons from the native system).
>      A resolution variant can be queried based on a scale factor and
> applied transforms.
>
>      5.3 The provided example allows to create a resolution variant
> using the requested high-resolution image size.
>      ------------
>      public interface MultiResolutionImage {
>          Image getResolutionVariant(float width, float height);
>      }
>      ------------
>      // create a multi-resolution image
>      Image mrImage = new AbstractMultiResolutionImage() {
>
>              public Image getResolutionVariant(float width, float height) {
>                  // create and return a resolution variant with exact
> requested width/height size
>              }
>
>              protected Image getBaseImage() {
>                  return baseImage;
>              }
>          };
>      ------------
>      // create a disabled image with resolution variants
>      Image disabledImage = null;
>      if (image instanceof MultiResolutionImage) {
>          final MultiResolutionImage mrImage = (MultiResolutionImage) image;
>          disabledImage = new AbstractMultiResolutionImage(){
>
>              public Image getResolutionVariant(float width, float height) {
>                  return
> getDisabledImage(mrImage.getResolutionVariant(width, height));
>              }
>
>              protected Image getBaseImage() {
>                  return getDisabledImage(mrImage);
>              }
>          };
>      } else {
>          disabledImage = getDisabledImage(image);
>      }
>      ------------
>
>    Thanks,
>    Alexandr.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Review request for 8029339 Custom MultiResolution image support on HiDPI displays

Alexandr Scherbatiy

   Hello,

   Could you review the proposed API based on MultiresolutionImage
interface:
     http://cr.openjdk.java.net/~alexsch/8029339/webrev.06/

  - return unmodifiable list comment is added to the
getResolutionVariants() method javadoc in MultiresolutionImage interface
  - base image size arguments are removed form the
getResolutionVariant(...) method in MultiresolutionImage interface
  - BaseMultiResolutionImage class that allows to create a
multi-resolution image based on resolution variant array is added
  - the test for the BaseMultiResolutionImage is added

  Thanks,
  Alexandr.

On 2/14/2015 3:23 AM, Jim Graham wrote:

> The second solution looks good.  I'd make it standard practice
> (perhaps even mentioned in the documentation) to return unmodifiable
> lists from the getVariants() method.  The Collections class provides
> easy methods to create these lists, and it sends a clear message to
> the caller that the list was provided for them to read, but not write
> to.  Otherwise they may add a new image to the list you provided them
> and wonder why it wasn't showing up.  Also, an unmodifiable list can
> be cached and reused for subsequent calls without having to create a
> new list every time.
>
> In getResolutionVariant() was there a reason why the base dimensions
> were provided as float?  The destination dimensions make sense as
> float since they could be the result of a scale, but the source
> dimensions are typically getWidth/getHeight() on the base image.  A
> related question would be if they are needed at all, since the
> implementation should probably already be aware of what the base image
> is and what its dimensions are.  I'm guessing they are provided
> because the implementation in SG2D already knows them and it makes it
> easier to forward the implementation on to a shared (static?) method?
>
> With respect to default implementations, I take it that the BaseMRI is
> along the pattern that we see in Swing for Base classes.  Would it be
> helpful to provide an implementation (in addition or instead) that
> allows a developer to take a bunch of images and quickly make an MRI
> without having to override anything?  The implementations of
> getBaseImage() and getResolutionVariants() are pretty straightforward
> and a fairly reasonable default algorithm can be provided for
> getRV(dimensions).  This question is more of an idle question for my
> own curiosity than a stumbling block...
>
>             ...jim
>
> On 1/22/2015 6:49 AM, Alexander Scherbatiy wrote:
>>
>>    Hi Phil,
>>
>>    I have prepared two versions of the proposed API:
>>
>>    I) Resolution variants are added directly to the Image:
>>     http://cr.openjdk.java.net/~alexsch/8029339/list/webrev.00
>>
>>    II)  MultiResolutionImage interface is used:
>>      http://cr.openjdk.java.net/~alexsch/8029339/webrev.05
>>
>>    It could help to decide which way should be chosen for the the
>> multi-resolution image support.
>>
>>    Below are some comments:
>>
>>    1. High level goal:
>>       Introduce an API that allows to create and handle an image with
>> resolution variants.
>>
>>    2. What is not subject of the provided API
>>      - Scale naming convention for high-resolution images
>>      - Providing pixel scale factor for the screen/window
>>
>>    3. Use cases
>>     3.1 Loading and drawing high-resolution icons in IntelliJ IDEA
>>       A high-resolution image is loaded from resources and stored in
>> JBHiDPIScaledImage class  which is a subclass of the buffered image.
>>       The high-resolution image is used to create a disabled icon in the
>> IconLoader.getDisabledIcon(icon) method.
>> https://github.com/JetBrains/intellij-community/blob/master/platform/util/src/com/intellij/openapi/util/IconLoader.java 
>>
>>
>>
>>     3.2 Loading and drawing high-resolution icons in NetBeans
>>       NetBeans does not have support for the high-resolution icons
>> loading.
>>       It loads an icon from the file system using
>> Toolkit.getDefaultToolkit().getImage(url) method or from resources
>>       by  ImageReader  and store it in ToolTipImage class which is
>> subclass of the buffered image.
>>       ImageUtilities.createDisabledIcon(icon) method creates a disabled
>> icon by applying  RGBImageFilter to the icon.
>> http://hg.netbeans.org/main/file/97dcf49eb4a7/openide.util/src/org/openide/util/ImageUtilities.java 
>>
>>
>>
>>     3.3 Loading system icons in JDK 1.8
>>       JDK requests icons from the native system for system L&Fs and
>> applies filters for them.
>>       See for example AquaUtils.generateLightenedImage() method:
>> http://hg.openjdk.java.net/jdk9/client/jdk/file/e6f48c4fad38/src/java.desktop/macosx/classes/com/apple/laf/AquaUtils.java 
>>
>>
>>
>>    4. HiDPI support for Images on different OSes
>>
>>      4.1 Mac OS X
>>        Cocoa API contains NSImage that allows to work with image
>> representations: add/remove/get all representations.
>>        It picks up an image with necessary resolution based on the
>> screen backing store pixel scale factor and applied transforms.
>> https://developer.apple.com/library/mac/documentation/Cocoa/Reference/ApplicationKit/Classes/NSImage_Class/Reference/Reference.html 
>>
>>
>>
>>      4.2 Linux
>>        GTK+ 3 API has gtkcssimagescaled lib (it seems that it is not
>> public/stable)
>>        that parses the -gtk-scaled css property and draws a GtkCssImage
>> according to the given scale factor.
>>
>>        I have not found information about the HiDPI support in Xlib.
>>
>>      4.3 Windows
>>        I have only found the tutorial that suggests to select and draw a
>> bitmap using the queried DPI
>>        and scale the coordinates for drawing a rectangular frame
>> http://msdn.microsoft.com/en-us/library/dd464659%28v=vs.85%29.aspx
>>
>>        Windows also provides the horizontal and vertical DPI of the
>> desktop
>> http://msdn.microsoft.com/en-us/library/windows/apps/dd371316
>>
>>    5. Pseudo API
>>       Below are some ways which illustrates how multi-resolution images
>> can be created and used.
>>
>>      5.1 Resolution variants are stored directly in Image class.
>>      To query a resolution variant it needs to compare the resolution
>> variant width/height
>>      with the requested high-resolution size.
>>      ------------
>>      public abstract class Image {
>>
>>          public void addResolutionVariant(Image image) {...}
>>          public List<Image> getResolutionVariants() {...}
>>      }
>>      ------------
>>      // create a disabled image with resolution variants
>>
>>      Image disabledImage = getDisabledImage(image);
>>
>>      for (Image rv : image.getResolutionVariants()) {
>> disabledImage.addResolutionVariant(getDisabledImage(rv));
>>      }
>>      ------------
>>      This approach requires that all resolution variants have been
>> created even not of them are really used.
>>
>>      5.2  Resolution variants are stored in a separate object that
>> allows to create them by demand.
>>      To query a resolution variant it needs to compare the resolution
>> variant scale factor
>>      with the requested scale (that can include both screen DPI scale
>> and applied transforms).
>>      ------------
>>      public abstract class Image {
>>
>>          public static interface ResolutionVariant {
>>              Image getImage();
>>              float getScaleFactor();
>>          }
>>
>>          public void addResolutionVariant(ResolutionVariant
>> resolutionVariant) {...}
>>          public List<ResolutionVariant> getResolutionVariants() {...}
>>      }
>>      ------------
>>      // create a disabled image with resolution variants
>>      Image disabledImage = getDisabledImage(image);
>>
>>      for (Image.ResolutionVariant rv : image.getResolutionVariants()) {
>>          disabledImage.addResolutionVariant(new
>> Image.ResolutionVariant() {
>>
>>              public Image getImage() {
>>                  return getDisabledImage(rv.getImage());
>>              }
>>
>>              public float getScaleFactor() {
>>                  return rv.getScaleFactor();
>>              }
>>          });
>>      }
>>      ------------
>>
>>      It does not have problem if a predefined set of images is provided
>> (like image.png and [hidden email] on the file system).
>>      This does not cover cases where a resolution variant can be created
>> using the exact requested size (like loading icons from the native
>> system).
>>      A resolution variant can be queried based on a scale factor and
>> applied transforms.
>>
>>      5.3 The provided example allows to create a resolution variant
>> using the requested high-resolution image size.
>>      ------------
>>      public interface MultiResolutionImage {
>>          Image getResolutionVariant(float width, float height);
>>      }
>>      ------------
>>      // create a multi-resolution image
>>      Image mrImage = new AbstractMultiResolutionImage() {
>>
>>              public Image getResolutionVariant(float width, float
>> height) {
>>                  // create and return a resolution variant with exact
>> requested width/height size
>>              }
>>
>>              protected Image getBaseImage() {
>>                  return baseImage;
>>              }
>>          };
>>      ------------
>>      // create a disabled image with resolution variants
>>      Image disabledImage = null;
>>      if (image instanceof MultiResolutionImage) {
>>          final MultiResolutionImage mrImage = (MultiResolutionImage)
>> image;
>>          disabledImage = new AbstractMultiResolutionImage(){
>>
>>              public Image getResolutionVariant(float width, float
>> height) {
>>                  return
>> getDisabledImage(mrImage.getResolutionVariant(width, height));
>>              }
>>
>>              protected Image getBaseImage() {
>>                  return getDisabledImage(mrImage);
>>              }
>>          };
>>      } else {
>>          disabledImage = getDisabledImage(image);
>>      }
>>      ------------
>>
>>    Thanks,
>>    Alexandr.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: <AWT Dev> Review request for 8029339 Custom MultiResolution image support on HiDPI displays

Hendrik Schreiber

> On Mar 13, 2015, at 14:34, Alexander Scherbatiy <[hidden email]> wrote:
>
>  Could you review the proposed API based on MultiresolutionImage interface:
>    http://cr.openjdk.java.net/~alexsch/8029339/webrev.06/

Not that it counts much, but to me the proposed API looks good in that it provides us with multiple ways to create custom MultiResolutionImages ourselves. Either via BaseMultiResolutionImage or by implementing the public MultiResolutionImage interface directly.

-hendrik

<slight-rant>
Personally, it irks me a little that we have an MRI, an AbstractMRI, and a BaseMRI (that's how you know it's Java code...). Fewer classes would be nice, but I guess one can't have it all. At least I don't have a great suggestion how to make this easy for developers while at the same time honoring DRY.
</slight-rant>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Review request for 8029339 Custom MultiResolution image support on HiDPI displays

Jim Graham-5
In reply to this post by Alexandr Scherbatiy
Hi Alexander,

MRI.java:

Who is the intended audience for the recommendation in the interface to
cache image variants?  Are we asking the developers who call the methods
on the interface to cache the answers?  That would be unwise because the
list of variants may change over time for some MRIs.  Are we speaking to
the implementer?  If so, then I think that it is unnecessary to remind
implementers of basic implementation strategies like "cache complicated
objects".

How about this wording for the getRV() method? - "Gets a specific image
that is the best variant to represent this logical image at the
indicated size and screen resolution."

Perhaps we should clarify in the doc comments for the dimension
arguments in getRV() that the dimensions are measured in pixels?

line 56 - delete blank line between doc comment and method declaration

Also, it is probably overkill to document that the interface
getVariants() method returns an unmodifiable list.  Responsible
implementations would probably use an unmodifiable list, but I don't
think it should be required by the interface.  We do need to specify
that it isn't required to be modifiable so that a caller doesn't expect
to be able to modify it, but that is a looser spec.  How about "Gets a
readable list of all resolution variants.  Note that many
implementations might return an unmodifiable list."?

AbstractMIR.java:

"provides a default implementation for the MRI" or "provides default
implementations of several methods in the <MRI> interface and the
<Image> class"?  Actually, I'll note that it provides none of the
implementations of the MRI methods so maybe it should be "provides
default implementations of several <Image> methods for classes that want
to implement the <MRI> interface"?

In the doc example - wrap the list to make it unmodifiable

The doc example could be made 2 or 3 lines shorter by simply assuming
the base image is in index 0 (it's just an example so I'm not sure the
flexibility needs to be a part of the example).

getGraphics is allowed by the Image interface to return null.  Actually,
the exact wording is that it can only be called for "offscreen images"
and a MRI is technically not "an offscreen image".  Perhaps we should
return null here since modifying the base image is unlikely to modify
the variants and arguably it would be required by the wording in the
method docs (even if the base image was an offscreen, the MRI produced
from it stops being an offscreen)?

Are all of the empty "@Inherit" comments needed?  I thought inheritance
was the default?

BaseMRI.java:

"This class is [an] array-based implementation of the {AMRI} class"
(missing "an" and "the")

We should probably include the comment about the sorting of the images
in the class comments as well as document that the algorithm is a simple
scan from the beginning for the first image large enough (and default ==
last image).  The algorithm also might not work very well if the images
are not monotonically both wider and taller.  How about adding this to
the class comments:

"The implementation will return the first image variant in the array
that is large enough to satisfy the rendering request.  For best effect
the array of images should be sorted with each image being both wider
and taller than the previous image.  The base image need not be the
first image in the array."

getVariants() - you need to return an unmodifiable list.  asList()
returns a list that "writes back" to the array.  You need to use
Collections.unmodifiableList(Arrays.asList(array)).

RenderingHints.java:

Where do we process this hint?  Don't we need to pass it to the
getVariant() method?

I don't understand the hint values other than the one that always uses
the base image.  Default I guess gives us the ability to use the Mac
algorithm of "next larger size" on Mac and "based on Screen DPI" on
Windows, but the 3rd value "ON" is so vague as to not have any meaning.
  Perhaps we should just delete it, but then do we just always do the
Mac method?  Or do we vaguely have our internal images have a
platform-specific method and the Abstract/Base classes just do what they
want with no control from the user?  In FX we are also still struggling
with this issue and we may likely just do what the Mac does in all
cases, but perhaps AWT needs to "behave like the platform" more?  If we
are going to have actual values, then we need to have them do something,
which means:

- SG2D needs to track the hint just like we do the other hints that
affect our processing
- MRI.getVariant() needs to have the hint as an argument
- BaseMRI should probably do something with that hint
- hint values should include:
     - ..._DEFAULT - implementation gets to decide
     - ..._BASE - always use the base image
     - ..._SIZE_FIT - Mac algorithm of smallest image that is big enough
     - ..._DPI_FIT - choose based on DPI of the screen, ignoring transform

line 978 - missing blank line between fields

SG2D.java:

- The interface says that you will be passing in the "logical DPI" of
the display, but here you are actually passing in the screen's scale factor.

BaseMRITest.java:

- testBaseMRI also passes in a scale rather than a DPI to the MRI method.

- How does the modification test pass when the implementation doesn't
use unmodifiable lists?

MRITest.java:

- also uses scale rather than DPI in several places

                ...jim

On 3/13/15 6:34 AM, Alexander Scherbatiy wrote:

>
>    Hello,
>
>    Could you review the proposed API based on MultiresolutionImage
> interface:
>      http://cr.openjdk.java.net/~alexsch/8029339/webrev.06/
>
>   - return unmodifiable list comment is added to the
> getResolutionVariants() method javadoc in MultiresolutionImage interface
>   - base image size arguments are removed form the
> getResolutionVariant(...) method in MultiresolutionImage interface
>   - BaseMultiResolutionImage class that allows to create a
> multi-resolution image based on resolution variant array is added
>   - the test for the BaseMultiResolutionImage is added
>
>   Thanks,
>   Alexandr.
>
> On 2/14/2015 3:23 AM, Jim Graham wrote:
>> The second solution looks good.  I'd make it standard practice
>> (perhaps even mentioned in the documentation) to return unmodifiable
>> lists from the getVariants() method.  The Collections class provides
>> easy methods to create these lists, and it sends a clear message to
>> the caller that the list was provided for them to read, but not write
>> to.  Otherwise they may add a new image to the list you provided them
>> and wonder why it wasn't showing up.  Also, an unmodifiable list can
>> be cached and reused for subsequent calls without having to create a
>> new list every time.
>>
>> In getResolutionVariant() was there a reason why the base dimensions
>> were provided as float?  The destination dimensions make sense as
>> float since they could be the result of a scale, but the source
>> dimensions are typically getWidth/getHeight() on the base image.  A
>> related question would be if they are needed at all, since the
>> implementation should probably already be aware of what the base image
>> is and what its dimensions are.  I'm guessing they are provided
>> because the implementation in SG2D already knows them and it makes it
>> easier to forward the implementation on to a shared (static?) method?
>>
>> With respect to default implementations, I take it that the BaseMRI is
>> along the pattern that we see in Swing for Base classes.  Would it be
>> helpful to provide an implementation (in addition or instead) that
>> allows a developer to take a bunch of images and quickly make an MRI
>> without having to override anything?  The implementations of
>> getBaseImage() and getResolutionVariants() are pretty straightforward
>> and a fairly reasonable default algorithm can be provided for
>> getRV(dimensions).  This question is more of an idle question for my
>> own curiosity than a stumbling block...
>>
>>             ...jim
>>
>> On 1/22/2015 6:49 AM, Alexander Scherbatiy wrote:
>>>
>>>    Hi Phil,
>>>
>>>    I have prepared two versions of the proposed API:
>>>
>>>    I) Resolution variants are added directly to the Image:
>>>     http://cr.openjdk.java.net/~alexsch/8029339/list/webrev.00
>>>
>>>    II)  MultiResolutionImage interface is used:
>>>      http://cr.openjdk.java.net/~alexsch/8029339/webrev.05
>>>
>>>    It could help to decide which way should be chosen for the the
>>> multi-resolution image support.
>>>
>>>    Below are some comments:
>>>
>>>    1. High level goal:
>>>       Introduce an API that allows to create and handle an image with
>>> resolution variants.
>>>
>>>    2. What is not subject of the provided API
>>>      - Scale naming convention for high-resolution images
>>>      - Providing pixel scale factor for the screen/window
>>>
>>>    3. Use cases
>>>     3.1 Loading and drawing high-resolution icons in IntelliJ IDEA
>>>       A high-resolution image is loaded from resources and stored in
>>> JBHiDPIScaledImage class  which is a subclass of the buffered image.
>>>       The high-resolution image is used to create a disabled icon in the
>>> IconLoader.getDisabledIcon(icon) method.
>>> https://github.com/JetBrains/intellij-community/blob/master/platform/util/src/com/intellij/openapi/util/IconLoader.java
>>>
>>>
>>>
>>>     3.2 Loading and drawing high-resolution icons in NetBeans
>>>       NetBeans does not have support for the high-resolution icons
>>> loading.
>>>       It loads an icon from the file system using
>>> Toolkit.getDefaultToolkit().getImage(url) method or from resources
>>>       by  ImageReader  and store it in ToolTipImage class which is
>>> subclass of the buffered image.
>>>       ImageUtilities.createDisabledIcon(icon) method creates a disabled
>>> icon by applying  RGBImageFilter to the icon.
>>> http://hg.netbeans.org/main/file/97dcf49eb4a7/openide.util/src/org/openide/util/ImageUtilities.java
>>>
>>>
>>>
>>>     3.3 Loading system icons in JDK 1.8
>>>       JDK requests icons from the native system for system L&Fs and
>>> applies filters for them.
>>>       See for example AquaUtils.generateLightenedImage() method:
>>> http://hg.openjdk.java.net/jdk9/client/jdk/file/e6f48c4fad38/src/java.desktop/macosx/classes/com/apple/laf/AquaUtils.java
>>>
>>>
>>>
>>>    4. HiDPI support for Images on different OSes
>>>
>>>      4.1 Mac OS X
>>>        Cocoa API contains NSImage that allows to work with image
>>> representations: add/remove/get all representations.
>>>        It picks up an image with necessary resolution based on the
>>> screen backing store pixel scale factor and applied transforms.
>>> https://developer.apple.com/library/mac/documentation/Cocoa/Reference/ApplicationKit/Classes/NSImage_Class/Reference/Reference.html
>>>
>>>
>>>
>>>      4.2 Linux
>>>        GTK+ 3 API has gtkcssimagescaled lib (it seems that it is not
>>> public/stable)
>>>        that parses the -gtk-scaled css property and draws a GtkCssImage
>>> according to the given scale factor.
>>>
>>>        I have not found information about the HiDPI support in Xlib.
>>>
>>>      4.3 Windows
>>>        I have only found the tutorial that suggests to select and draw a
>>> bitmap using the queried DPI
>>>        and scale the coordinates for drawing a rectangular frame
>>> http://msdn.microsoft.com/en-us/library/dd464659%28v=vs.85%29.aspx
>>>
>>>        Windows also provides the horizontal and vertical DPI of the
>>> desktop
>>> http://msdn.microsoft.com/en-us/library/windows/apps/dd371316
>>>
>>>    5. Pseudo API
>>>       Below are some ways which illustrates how multi-resolution images
>>> can be created and used.
>>>
>>>      5.1 Resolution variants are stored directly in Image class.
>>>      To query a resolution variant it needs to compare the resolution
>>> variant width/height
>>>      with the requested high-resolution size.
>>>      ------------
>>>      public abstract class Image {
>>>
>>>          public void addResolutionVariant(Image image) {...}
>>>          public List<Image> getResolutionVariants() {...}
>>>      }
>>>      ------------
>>>      // create a disabled image with resolution variants
>>>
>>>      Image disabledImage = getDisabledImage(image);
>>>
>>>      for (Image rv : image.getResolutionVariants()) {
>>> disabledImage.addResolutionVariant(getDisabledImage(rv));
>>>      }
>>>      ------------
>>>      This approach requires that all resolution variants have been
>>> created even not of them are really used.
>>>
>>>      5.2  Resolution variants are stored in a separate object that
>>> allows to create them by demand.
>>>      To query a resolution variant it needs to compare the resolution
>>> variant scale factor
>>>      with the requested scale (that can include both screen DPI scale
>>> and applied transforms).
>>>      ------------
>>>      public abstract class Image {
>>>
>>>          public static interface ResolutionVariant {
>>>              Image getImage();
>>>              float getScaleFactor();
>>>          }
>>>
>>>          public void addResolutionVariant(ResolutionVariant
>>> resolutionVariant) {...}
>>>          public List<ResolutionVariant> getResolutionVariants() {...}
>>>      }
>>>      ------------
>>>      // create a disabled image with resolution variants
>>>      Image disabledImage = getDisabledImage(image);
>>>
>>>      for (Image.ResolutionVariant rv : image.getResolutionVariants()) {
>>>          disabledImage.addResolutionVariant(new
>>> Image.ResolutionVariant() {
>>>
>>>              public Image getImage() {
>>>                  return getDisabledImage(rv.getImage());
>>>              }
>>>
>>>              public float getScaleFactor() {
>>>                  return rv.getScaleFactor();
>>>              }
>>>          });
>>>      }
>>>      ------------
>>>
>>>      It does not have problem if a predefined set of images is provided
>>> (like image.png and [hidden email] on the file system).
>>>      This does not cover cases where a resolution variant can be created
>>> using the exact requested size (like loading icons from the native
>>> system).
>>>      A resolution variant can be queried based on a scale factor and
>>> applied transforms.
>>>
>>>      5.3 The provided example allows to create a resolution variant
>>> using the requested high-resolution image size.
>>>      ------------
>>>      public interface MultiResolutionImage {
>>>          Image getResolutionVariant(float width, float height);
>>>      }
>>>      ------------
>>>      // create a multi-resolution image
>>>      Image mrImage = new AbstractMultiResolutionImage() {
>>>
>>>              public Image getResolutionVariant(float width, float
>>> height) {
>>>                  // create and return a resolution variant with exact
>>> requested width/height size
>>>              }
>>>
>>>              protected Image getBaseImage() {
>>>                  return baseImage;
>>>              }
>>>          };
>>>      ------------
>>>      // create a disabled image with resolution variants
>>>      Image disabledImage = null;
>>>      if (image instanceof MultiResolutionImage) {
>>>          final MultiResolutionImage mrImage = (MultiResolutionImage)
>>> image;
>>>          disabledImage = new AbstractMultiResolutionImage(){
>>>
>>>              public Image getResolutionVariant(float width, float
>>> height) {
>>>                  return
>>> getDisabledImage(mrImage.getResolutionVariant(width, height));
>>>              }
>>>
>>>              protected Image getBaseImage() {
>>>                  return getDisabledImage(mrImage);
>>>              }
>>>          };
>>>      } else {
>>>          disabledImage = getDisabledImage(image);
>>>      }
>>>      ------------
>>>
>>>    Thanks,
>>>    Alexandr.
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Review request for 8029339 Custom MultiResolution image support on HiDPI displays

Sergey Bylokhov
Hello, Jim.
27.03.15 0:48, Jim Graham wrote:

> RenderingHints.java:
>
> Where do we process this hint?  Don't we need to pass it to the
> getVariant() method?
>
> I don't understand the hint values other than the one that always uses
> the base image.  Default I guess gives us the ability to use the Mac
> algorithm of "next larger size" on Mac and "based on Screen DPI" on
> Windows, but the 3rd value "ON" is so vague as to not have any
> meaning.  Perhaps we should just delete it, but then do we just always
> do the Mac method?  Or do we vaguely have our internal images have a
> platform-specific method and the Abstract/Base classes just do what
> they want with no control from the user?  In FX we are also still
> struggling with this issue and we may likely just do what the Mac does
> in all cases, but perhaps AWT needs to "behave like the platform"
> more?  If we are going to have actual values, then we need to have
> them do something, which means:
>
> - SG2D needs to track the hint just like we do the other hints that
> affect our processing
> - MRI.getVariant() needs to have the hint as an argument
> - BaseMRI should probably do something with that hint
> - hint values should include:
>     - ..._DEFAULT - implementation gets to decide
>     - ..._BASE - always use the base image
>     - ..._SIZE_FIT - Mac algorithm of smallest image that is big enough
>     - ..._DPI_FIT - choose based on DPI of the screen, ignoring transform
I did not watch this discussion, but I have a question about use of dpi
vs scale. Why we cannot make things simpler and provide only
width+height to the getResolutionVariant()?
Like this:
Image getResolutionVariant(float destImageWidth, float destImageWidth);
All other things can be calculated internally by this method.

We will pass width and height, based on the hints which were set to the
SG2D.
By default this hint will be _FULL_SIZE_FIT like on osx and width/height
will include full transformation(dev+usr),
On windows for Window and VolatileImages this will be _DPI_FIT and
width/height will include dev transformation only.
It is unclear what hint should be default in BufferedImage on windows.
List of the hints should looks like:
     - ..._DEFAULT - implementation gets to decide
     - ..._BASE - always use the base image
     - ..._FULL_SIZE_FIT - Mac algorithm of smallest image that is big
enough
     - ..._DEFAULT_DPI_FIT - choose based on DPI of the screen, ignoring
transform
     - ..._USR_DPI_FIT - choose based on users transform, ignoring
device transform.
Names can be tweaked for sure.
It seems we will cover all possible cases in such solution, no?

> line 978 - missing blank line between fields
>
> SG2D.java:
>
> - The interface says that you will be passing in the "logical DPI" of
> the display, but here you are actually passing in the screen's scale
> factor.
>
>
> On 3/13/15 6:34 AM, Alexander Scherbatiy wrote:
>>
>>    Hello,
>>
>>    Could you review the proposed API based on MultiresolutionImage
>> interface:
>>      http://cr.openjdk.java.net/~alexsch/8029339/webrev.06/
>>
>>   - return unmodifiable list comment is added to the
>> getResolutionVariants() method javadoc in MultiresolutionImage interface
>>   - base image size arguments are removed form the
>> getResolutionVariant(...) method in MultiresolutionImage interface
>>   - BaseMultiResolutionImage class that allows to create a
>> multi-resolution image based on resolution variant array is added
>>   - the test for the BaseMultiResolutionImage is added
>>
>>   Thanks,
>>   Alexandr.
>>
>> On 2/14/2015 3:23 AM, Jim Graham wrote:
>>> The second solution looks good.  I'd make it standard practice
>>> (perhaps even mentioned in the documentation) to return unmodifiable
>>> lists from the getVariants() method.  The Collections class provides
>>> easy methods to create these lists, and it sends a clear message to
>>> the caller that the list was provided for them to read, but not write
>>> to.  Otherwise they may add a new image to the list you provided them
>>> and wonder why it wasn't showing up.  Also, an unmodifiable list can
>>> be cached and reused for subsequent calls without having to create a
>>> new list every time.
>>>
>>> In getResolutionVariant() was there a reason why the base dimensions
>>> were provided as float?  The destination dimensions make sense as
>>> float since they could be the result of a scale, but the source
>>> dimensions are typically getWidth/getHeight() on the base image.  A
>>> related question would be if they are needed at all, since the
>>> implementation should probably already be aware of what the base image
>>> is and what its dimensions are.  I'm guessing they are provided
>>> because the implementation in SG2D already knows them and it makes it
>>> easier to forward the implementation on to a shared (static?) method?
>>>
>>> With respect to default implementations, I take it that the BaseMRI is
>>> along the pattern that we see in Swing for Base classes. Would it be
>>> helpful to provide an implementation (in addition or instead) that
>>> allows a developer to take a bunch of images and quickly make an MRI
>>> without having to override anything?  The implementations of
>>> getBaseImage() and getResolutionVariants() are pretty straightforward
>>> and a fairly reasonable default algorithm can be provided for
>>> getRV(dimensions).  This question is more of an idle question for my
>>> own curiosity than a stumbling block...
>>>
>>>             ...jim
>>>
>>> On 1/22/2015 6:49 AM, Alexander Scherbatiy wrote:
>>>>
>>>>    Hi Phil,
>>>>
>>>>    I have prepared two versions of the proposed API:
>>>>
>>>>    I) Resolution variants are added directly to the Image:
>>>> http://cr.openjdk.java.net/~alexsch/8029339/list/webrev.00
>>>>
>>>>    II)  MultiResolutionImage interface is used:
>>>>      http://cr.openjdk.java.net/~alexsch/8029339/webrev.05
>>>>
>>>>    It could help to decide which way should be chosen for the the
>>>> multi-resolution image support.
>>>>
>>>>    Below are some comments:
>>>>
>>>>    1. High level goal:
>>>>       Introduce an API that allows to create and handle an image with
>>>> resolution variants.
>>>>
>>>>    2. What is not subject of the provided API
>>>>      - Scale naming convention for high-resolution images
>>>>      - Providing pixel scale factor for the screen/window
>>>>
>>>>    3. Use cases
>>>>     3.1 Loading and drawing high-resolution icons in IntelliJ IDEA
>>>>       A high-resolution image is loaded from resources and stored in
>>>> JBHiDPIScaledImage class  which is a subclass of the buffered image.
>>>>       The high-resolution image is used to create a disabled icon
>>>> in the
>>>> IconLoader.getDisabledIcon(icon) method.
>>>> https://github.com/JetBrains/intellij-community/blob/master/platform/util/src/com/intellij/openapi/util/IconLoader.java 
>>>>
>>>>
>>>>
>>>>
>>>>     3.2 Loading and drawing high-resolution icons in NetBeans
>>>>       NetBeans does not have support for the high-resolution icons
>>>> loading.
>>>>       It loads an icon from the file system using
>>>> Toolkit.getDefaultToolkit().getImage(url) method or from resources
>>>>       by  ImageReader  and store it in ToolTipImage class which is
>>>> subclass of the buffered image.
>>>>       ImageUtilities.createDisabledIcon(icon) method creates a
>>>> disabled
>>>> icon by applying  RGBImageFilter to the icon.
>>>> http://hg.netbeans.org/main/file/97dcf49eb4a7/openide.util/src/org/openide/util/ImageUtilities.java 
>>>>
>>>>
>>>>
>>>>
>>>>     3.3 Loading system icons in JDK 1.8
>>>>       JDK requests icons from the native system for system L&Fs and
>>>> applies filters for them.
>>>>       See for example AquaUtils.generateLightenedImage() method:
>>>> http://hg.openjdk.java.net/jdk9/client/jdk/file/e6f48c4fad38/src/java.desktop/macosx/classes/com/apple/laf/AquaUtils.java 
>>>>
>>>>
>>>>
>>>>
>>>>    4. HiDPI support for Images on different OSes
>>>>
>>>>      4.1 Mac OS X
>>>>        Cocoa API contains NSImage that allows to work with image
>>>> representations: add/remove/get all representations.
>>>>        It picks up an image with necessary resolution based on the
>>>> screen backing store pixel scale factor and applied transforms.
>>>> https://developer.apple.com/library/mac/documentation/Cocoa/Reference/ApplicationKit/Classes/NSImage_Class/Reference/Reference.html 
>>>>
>>>>
>>>>
>>>>
>>>>      4.2 Linux
>>>>        GTK+ 3 API has gtkcssimagescaled lib (it seems that it is not
>>>> public/stable)
>>>>        that parses the -gtk-scaled css property and draws a
>>>> GtkCssImage
>>>> according to the given scale factor.
>>>>
>>>>        I have not found information about the HiDPI support in Xlib.
>>>>
>>>>      4.3 Windows
>>>>        I have only found the tutorial that suggests to select and
>>>> draw a
>>>> bitmap using the queried DPI
>>>>        and scale the coordinates for drawing a rectangular frame
>>>> http://msdn.microsoft.com/en-us/library/dd464659%28v=vs.85%29.aspx
>>>>
>>>>        Windows also provides the horizontal and vertical DPI of the
>>>> desktop
>>>> http://msdn.microsoft.com/en-us/library/windows/apps/dd371316
>>>>
>>>>    5. Pseudo API
>>>>       Below are some ways which illustrates how multi-resolution
>>>> images
>>>> can be created and used.
>>>>
>>>>      5.1 Resolution variants are stored directly in Image class.
>>>>      To query a resolution variant it needs to compare the resolution
>>>> variant width/height
>>>>      with the requested high-resolution size.
>>>>      ------------
>>>>      public abstract class Image {
>>>>
>>>>          public void addResolutionVariant(Image image) {...}
>>>>          public List<Image> getResolutionVariants() {...}
>>>>      }
>>>>      ------------
>>>>      // create a disabled image with resolution variants
>>>>
>>>>      Image disabledImage = getDisabledImage(image);
>>>>
>>>>      for (Image rv : image.getResolutionVariants()) {
>>>> disabledImage.addResolutionVariant(getDisabledImage(rv));
>>>>      }
>>>>      ------------
>>>>      This approach requires that all resolution variants have been
>>>> created even not of them are really used.
>>>>
>>>>      5.2  Resolution variants are stored in a separate object that
>>>> allows to create them by demand.
>>>>      To query a resolution variant it needs to compare the resolution
>>>> variant scale factor
>>>>      with the requested scale (that can include both screen DPI scale
>>>> and applied transforms).
>>>>      ------------
>>>>      public abstract class Image {
>>>>
>>>>          public static interface ResolutionVariant {
>>>>              Image getImage();
>>>>              float getScaleFactor();
>>>>          }
>>>>
>>>>          public void addResolutionVariant(ResolutionVariant
>>>> resolutionVariant) {...}
>>>>          public List<ResolutionVariant> getResolutionVariants() {...}
>>>>      }
>>>>      ------------
>>>>      // create a disabled image with resolution variants
>>>>      Image disabledImage = getDisabledImage(image);
>>>>
>>>>      for (Image.ResolutionVariant rv :
>>>> image.getResolutionVariants()) {
>>>>          disabledImage.addResolutionVariant(new
>>>> Image.ResolutionVariant() {
>>>>
>>>>              public Image getImage() {
>>>>                  return getDisabledImage(rv.getImage());
>>>>              }
>>>>
>>>>              public float getScaleFactor() {
>>>>                  return rv.getScaleFactor();
>>>>              }
>>>>          });
>>>>      }
>>>>      ------------
>>>>
>>>>      It does not have problem if a predefined set of images is
>>>> provided
>>>> (like image.png and [hidden email] on the file system).
>>>>      This does not cover cases where a resolution variant can be
>>>> created
>>>> using the exact requested size (like loading icons from the native
>>>> system).
>>>>      A resolution variant can be queried based on a scale factor and
>>>> applied transforms.
>>>>
>>>>      5.3 The provided example allows to create a resolution variant
>>>> using the requested high-resolution image size.
>>>>      ------------
>>>>      public interface MultiResolutionImage {
>>>>          Image getResolutionVariant(float width, float height);
>>>>      }
>>>>      ------------
>>>>      // create a multi-resolution image
>>>>      Image mrImage = new AbstractMultiResolutionImage() {
>>>>
>>>>              public Image getResolutionVariant(float width, float
>>>> height) {
>>>>                  // create and return a resolution variant with exact
>>>> requested width/height size
>>>>              }
>>>>
>>>>              protected Image getBaseImage() {
>>>>                  return baseImage;
>>>>              }
>>>>          };
>>>>      ------------
>>>>      // create a disabled image with resolution variants
>>>>      Image disabledImage = null;
>>>>      if (image instanceof MultiResolutionImage) {
>>>>          final MultiResolutionImage mrImage = (MultiResolutionImage)
>>>> image;
>>>>          disabledImage = new AbstractMultiResolutionImage(){
>>>>
>>>>              public Image getResolutionVariant(float width, float
>>>> height) {
>>>>                  return
>>>> getDisabledImage(mrImage.getResolutionVariant(width, height));
>>>>              }
>>>>
>>>>              protected Image getBaseImage() {
>>>>                  return getDisabledImage(mrImage);
>>>>              }
>>>>          };
>>>>      } else {
>>>>          disabledImage = getDisabledImage(image);
>>>>      }
>>>>      ------------
>>>>
>>>>    Thanks,
>>>>    Alexandr.
>>


--
Best regards, Sergey.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Review request for 8029339 Custom MultiResolution image support on HiDPI displays

Alexandr Scherbatiy
In reply to this post by Jim Graham-5
On 3/27/2015 12:48 AM, Jim Graham wrote:
> Hi Alexander,

    http://cr.openjdk.java.net/~alexsch/8029339/webrev.07/
    I have updated the fix according to the comments except RenderingHints.

    RenderingHints are supposed to be set on Graphics2D and they have
keys/values which are not relevant to the getResolutionVariant() method.
    Graphics objects chooses an alternative according to the set
rendering hints.
    May be what SG2D should do just calculate dest image size for the
given resolution variant hint (_BASE - base image size, _SIZE_FIT -
including scale factor and graphics transform (Mac algorithm), _DPI_FIT
- scaled according to the system DPI) and then pass them to the
getResolutionVariant() method?

   Thanks,
   Alexandr.

>
> MRI.java:
>
> Who is the intended audience for the recommendation in the interface
> to cache image variants?  Are we asking the developers who call the
> methods on the interface to cache the answers?  That would be unwise
> because the list of variants may change over time for some MRIs.  Are
> we speaking to the implementer?  If so, then I think that it is
> unnecessary to remind implementers of basic implementation strategies
> like "cache complicated objects".
>
> How about this wording for the getRV() method? - "Gets a specific
> image that is the best variant to represent this logical image at the
> indicated size and screen resolution."
>
> Perhaps we should clarify in the doc comments for the dimension
> arguments in getRV() that the dimensions are measured in pixels?
>
> line 56 - delete blank line between doc comment and method declaration
>
> Also, it is probably overkill to document that the interface
> getVariants() method returns an unmodifiable list.  Responsible
> implementations would probably use an unmodifiable list, but I don't
> think it should be required by the interface.  We do need to specify
> that it isn't required to be modifiable so that a caller doesn't
> expect to be able to modify it, but that is a looser spec.  How about
> "Gets a readable list of all resolution variants.  Note that many
> implementations might return an unmodifiable list."?
>
> AbstractMIR.java:
>
> "provides a default implementation for the MRI" or "provides default
> implementations of several methods in the <MRI> interface and the
> <Image> class"?  Actually, I'll note that it provides none of the
> implementations of the MRI methods so maybe it should be "provides
> default implementations of several <Image> methods for classes that
> want to implement the <MRI> interface"?
>
> In the doc example - wrap the list to make it unmodifiable
>
> The doc example could be made 2 or 3 lines shorter by simply assuming
> the base image is in index 0 (it's just an example so I'm not sure the
> flexibility needs to be a part of the example).
>
> getGraphics is allowed by the Image interface to return null.
> Actually, the exact wording is that it can only be called for
> "offscreen images" and a MRI is technically not "an offscreen image".  
> Perhaps we should return null here since modifying the base image is
> unlikely to modify the variants and arguably it would be required by
> the wording in the method docs (even if the base image was an
> offscreen, the MRI produced from it stops being an offscreen)?
>
> Are all of the empty "@Inherit" comments needed?  I thought
> inheritance was the default?
>
> BaseMRI.java:
>
> "This class is [an] array-based implementation of the {AMRI} class"
> (missing "an" and "the")
>
> We should probably include the comment about the sorting of the images
> in the class comments as well as document that the algorithm is a
> simple scan from the beginning for the first image large enough (and
> default == last image).  The algorithm also might not work very well
> if the images are not monotonically both wider and taller.  How about
> adding this to the class comments:
>
> "The implementation will return the first image variant in the array
> that is large enough to satisfy the rendering request.  For best
> effect the array of images should be sorted with each image being both
> wider and taller than the previous image.  The base image need not be
> the first image in the array."
>
> getVariants() - you need to return an unmodifiable list.  asList()
> returns a list that "writes back" to the array.  You need to use
> Collections.unmodifiableList(Arrays.asList(array)).
>
> RenderingHints.java:
>
> Where do we process this hint?  Don't we need to pass it to the
> getVariant() method?
>
> I don't understand the hint values other than the one that always uses
> the base image.  Default I guess gives us the ability to use the Mac
> algorithm of "next larger size" on Mac and "based on Screen DPI" on
> Windows, but the 3rd value "ON" is so vague as to not have any
> meaning.  Perhaps we should just delete it, but then do we just always
> do the Mac method?  Or do we vaguely have our internal images have a
> platform-specific method and the Abstract/Base classes just do what
> they want with no control from the user?  In FX we are also still
> struggling with this issue and we may likely just do what the Mac does
> in all cases, but perhaps AWT needs to "behave like the platform"
> more?  If we are going to have actual values, then we need to have
> them do something, which means:
>
> - SG2D needs to track the hint just like we do the other hints that
> affect our processing
> - MRI.getVariant() needs to have the hint as an argument
> - BaseMRI should probably do something with that hint
> - hint values should include:
>     - ..._DEFAULT - implementation gets to decide
>     - ..._BASE - always use the base image
>     - ..._SIZE_FIT - Mac algorithm of smallest image that is big enough
>     - ..._DPI_FIT - choose based on DPI of the screen, ignoring transform
>
> line 978 - missing blank line between fields
>
> SG2D.java:
>
> - The interface says that you will be passing in the "logical DPI" of
> the display, but here you are actually passing in the screen's scale
> factor.
>
> BaseMRITest.java:
>
> - testBaseMRI also passes in a scale rather than a DPI to the MRI method.
>
> - How does the modification test pass when the implementation doesn't
> use unmodifiable lists?
>
> MRITest.java:
>
> - also uses scale rather than DPI in several places
>
>         ...jim
>
> On 3/13/15 6:34 AM, Alexander Scherbatiy wrote:
>>
>>    Hello,
>>
>>    Could you review the proposed API based on MultiresolutionImage
>> interface:
>>      http://cr.openjdk.java.net/~alexsch/8029339/webrev.06/
>>
>>   - return unmodifiable list comment is added to the
>> getResolutionVariants() method javadoc in MultiresolutionImage interface
>>   - base image size arguments are removed form the
>> getResolutionVariant(...) method in MultiresolutionImage interface
>>   - BaseMultiResolutionImage class that allows to create a
>> multi-resolution image based on resolution variant array is added
>>   - the test for the BaseMultiResolutionImage is added
>>
>>   Thanks,
>>   Alexandr.
>>
>> On 2/14/2015 3:23 AM, Jim Graham wrote:
>>> The second solution looks good.  I'd make it standard practice
>>> (perhaps even mentioned in the documentation) to return unmodifiable
>>> lists from the getVariants() method.  The Collections class provides
>>> easy methods to create these lists, and it sends a clear message to
>>> the caller that the list was provided for them to read, but not write
>>> to.  Otherwise they may add a new image to the list you provided them
>>> and wonder why it wasn't showing up.  Also, an unmodifiable list can
>>> be cached and reused for subsequent calls without having to create a
>>> new list every time.
>>>
>>> In getResolutionVariant() was there a reason why the base dimensions
>>> were provided as float?  The destination dimensions make sense as
>>> float since they could be the result of a scale, but the source
>>> dimensions are typically getWidth/getHeight() on the base image.  A
>>> related question would be if they are needed at all, since the
>>> implementation should probably already be aware of what the base image
>>> is and what its dimensions are.  I'm guessing they are provided
>>> because the implementation in SG2D already knows them and it makes it
>>> easier to forward the implementation on to a shared (static?) method?
>>>
>>> With respect to default implementations, I take it that the BaseMRI is
>>> along the pattern that we see in Swing for Base classes. Would it be
>>> helpful to provide an implementation (in addition or instead) that
>>> allows a developer to take a bunch of images and quickly make an MRI
>>> without having to override anything?  The implementations of
>>> getBaseImage() and getResolutionVariants() are pretty straightforward
>>> and a fairly reasonable default algorithm can be provided for
>>> getRV(dimensions).  This question is more of an idle question for my
>>> own curiosity than a stumbling block...
>>>
>>>             ...jim
>>>
>>> On 1/22/2015 6:49 AM, Alexander Scherbatiy wrote:
>>>>
>>>>    Hi Phil,
>>>>
>>>>    I have prepared two versions of the proposed API:
>>>>
>>>>    I) Resolution variants are added directly to the Image:
>>>> http://cr.openjdk.java.net/~alexsch/8029339/list/webrev.00
>>>>
>>>>    II)  MultiResolutionImage interface is used:
>>>>      http://cr.openjdk.java.net/~alexsch/8029339/webrev.05
>>>>
>>>>    It could help to decide which way should be chosen for the the
>>>> multi-resolution image support.
>>>>
>>>>    Below are some comments:
>>>>
>>>>    1. High level goal:
>>>>       Introduce an API that allows to create and handle an image with
>>>> resolution variants.
>>>>
>>>>    2. What is not subject of the provided API
>>>>      - Scale naming convention for high-resolution images
>>>>      - Providing pixel scale factor for the screen/window
>>>>
>>>>    3. Use cases
>>>>     3.1 Loading and drawing high-resolution icons in IntelliJ IDEA
>>>>       A high-resolution image is loaded from resources and stored in
>>>> JBHiDPIScaledImage class  which is a subclass of the buffered image.
>>>>       The high-resolution image is used to create a disabled icon
>>>> in the
>>>> IconLoader.getDisabledIcon(icon) method.
>>>> https://github.com/JetBrains/intellij-community/blob/master/platform/util/src/com/intellij/openapi/util/IconLoader.java 
>>>>
>>>>
>>>>
>>>>
>>>>     3.2 Loading and drawing high-resolution icons in NetBeans
>>>>       NetBeans does not have support for the high-resolution icons
>>>> loading.
>>>>       It loads an icon from the file system using
>>>> Toolkit.getDefaultToolkit().getImage(url) method or from resources
>>>>       by  ImageReader  and store it in ToolTipImage class which is
>>>> subclass of the buffered image.
>>>>       ImageUtilities.createDisabledIcon(icon) method creates a
>>>> disabled
>>>> icon by applying  RGBImageFilter to the icon.
>>>> http://hg.netbeans.org/main/file/97dcf49eb4a7/openide.util/src/org/openide/util/ImageUtilities.java 
>>>>
>>>>
>>>>
>>>>
>>>>     3.3 Loading system icons in JDK 1.8
>>>>       JDK requests icons from the native system for system L&Fs and
>>>> applies filters for them.
>>>>       See for example AquaUtils.generateLightenedImage() method:
>>>> http://hg.openjdk.java.net/jdk9/client/jdk/file/e6f48c4fad38/src/java.desktop/macosx/classes/com/apple/laf/AquaUtils.java 
>>>>
>>>>
>>>>
>>>>
>>>>    4. HiDPI support for Images on different OSes
>>>>
>>>>      4.1 Mac OS X
>>>>        Cocoa API contains NSImage that allows to work with image
>>>> representations: add/remove/get all representations.
>>>>        It picks up an image with necessary resolution based on the
>>>> screen backing store pixel scale factor and applied transforms.
>>>> https://developer.apple.com/library/mac/documentation/Cocoa/Reference/ApplicationKit/Classes/NSImage_Class/Reference/Reference.html 
>>>>
>>>>
>>>>
>>>>
>>>>      4.2 Linux
>>>>        GTK+ 3 API has gtkcssimagescaled lib (it seems that it is not
>>>> public/stable)
>>>>        that parses the -gtk-scaled css property and draws a
>>>> GtkCssImage
>>>> according to the given scale factor.
>>>>
>>>>        I have not found information about the HiDPI support in Xlib.
>>>>
>>>>      4.3 Windows
>>>>        I have only found the tutorial that suggests to select and
>>>> draw a
>>>> bitmap using the queried DPI
>>>>        and scale the coordinates for drawing a rectangular frame
>>>> http://msdn.microsoft.com/en-us/library/dd464659%28v=vs.85%29.aspx
>>>>
>>>>        Windows also provides the horizontal and vertical DPI of the
>>>> desktop
>>>> http://msdn.microsoft.com/en-us/library/windows/apps/dd371316
>>>>
>>>>    5. Pseudo API
>>>>       Below are some ways which illustrates how multi-resolution
>>>> images
>>>> can be created and used.
>>>>
>>>>      5.1 Resolution variants are stored directly in Image class.
>>>>      To query a resolution variant it needs to compare the resolution
>>>> variant width/height
>>>>      with the requested high-resolution size.
>>>>      ------------
>>>>      public abstract class Image {
>>>>
>>>>          public void addResolutionVariant(Image image) {...}
>>>>          public List<Image> getResolutionVariants() {...}
>>>>      }
>>>>      ------------
>>>>      // create a disabled image with resolution variants
>>>>
>>>>      Image disabledImage = getDisabledImage(image);
>>>>
>>>>      for (Image rv : image.getResolutionVariants()) {
>>>> disabledImage.addResolutionVariant(getDisabledImage(rv));
>>>>      }
>>>>      ------------
>>>>      This approach requires that all resolution variants have been
>>>> created even not of them are really used.
>>>>
>>>>      5.2  Resolution variants are stored in a separate object that
>>>> allows to create them by demand.
>>>>      To query a resolution variant it needs to compare the resolution
>>>> variant scale factor
>>>>      with the requested scale (that can include both screen DPI scale
>>>> and applied transforms).
>>>>      ------------
>>>>      public abstract class Image {
>>>>
>>>>          public static interface ResolutionVariant {
>>>>              Image getImage();
>>>>              float getScaleFactor();
>>>>          }
>>>>
>>>>          public void addResolutionVariant(ResolutionVariant
>>>> resolutionVariant) {...}
>>>>          public List<ResolutionVariant> getResolutionVariants() {...}
>>>>      }
>>>>      ------------
>>>>      // create a disabled image with resolution variants
>>>>      Image disabledImage = getDisabledImage(image);
>>>>
>>>>      for (Image.ResolutionVariant rv :
>>>> image.getResolutionVariants()) {
>>>>          disabledImage.addResolutionVariant(new
>>>> Image.ResolutionVariant() {
>>>>
>>>>              public Image getImage() {
>>>>                  return getDisabledImage(rv.getImage());
>>>>              }
>>>>
>>>>              public float getScaleFactor() {
>>>>                  return rv.getScaleFactor();
>>>>              }
>>>>          });
>>>>      }
>>>>      ------------
>>>>
>>>>      It does not have problem if a predefined set of images is
>>>> provided
>>>> (like image.png and [hidden email] on the file system).
>>>>      This does not cover cases where a resolution variant can be
>>>> created
>>>> using the exact requested size (like loading icons from the native
>>>> system).
>>>>      A resolution variant can be queried based on a scale factor and
>>>> applied transforms.
>>>>
>>>>      5.3 The provided example allows to create a resolution variant
>>>> using the requested high-resolution image size.
>>>>      ------------
>>>>      public interface MultiResolutionImage {
>>>>          Image getResolutionVariant(float width, float height);
>>>>      }
>>>>      ------------
>>>>      // create a multi-resolution image
>>>>      Image mrImage = new AbstractMultiResolutionImage() {
>>>>
>>>>              public Image getResolutionVariant(float width, float
>>>> height) {
>>>>                  // create and return a resolution variant with exact
>>>> requested width/height size
>>>>              }
>>>>
>>>>              protected Image getBaseImage() {
>>>>                  return baseImage;
>>>>              }
>>>>          };
>>>>      ------------
>>>>      // create a disabled image with resolution variants
>>>>      Image disabledImage = null;
>>>>      if (image instanceof MultiResolutionImage) {
>>>>          final MultiResolutionImage mrImage = (MultiResolutionImage)
>>>> image;
>>>>          disabledImage = new AbstractMultiResolutionImage(){
>>>>
>>>>              public Image getResolutionVariant(float width, float
>>>> height) {
>>>>                  return
>>>> getDisabledImage(mrImage.getResolutionVariant(width, height));
>>>>              }
>>>>
>>>>              protected Image getBaseImage() {
>>>>                  return getDisabledImage(mrImage);
>>>>              }
>>>>          };
>>>>      } else {
>>>>          disabledImage = getDisabledImage(image);
>>>>      }
>>>>      ------------
>>>>
>>>>    Thanks,
>>>>    Alexandr.
>>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: <AWT Dev> Review request for 8029339 Custom MultiResolution image support on HiDPI displays

Hendrik Schreiber

> On Apr 1, 2015, at 15:46, Alexander Scherbatiy <[hidden email]> wrote:
>
> On 3/27/2015 12:48 AM, Jim Graham wrote:
>> Hi Alexander,
>
>   http://cr.openjdk.java.net/~alexsch/8029339/webrev.07/
>   I have updated the fix according to the comments except RenderingHints.

Alexandr,

I've noticed that the MultiResolutionToolkitImage only supports a high and regular resolution image, but not an arbitrary number of images like most (all?) of the other classes/interfaces.

Given, that when you create a native application in XCode 6.2 today, you already include @2x and @3x image variants and that Apple apparently plans on shipping 8k monitors soon (http://www.macrumors.com/2015/04/06/lg-display-imac-8k/), restricting ourselves to just two variants seems to make this code out-of-date before JDK9 ships.

Therefore, I'd like to suggest that we support an arbitrary number of resolutions variants in Toolkit images. I guess the OS X naming convention for this is self-explanatory. Not sure about Windows.

Cheers,

-hendrik

PS: java.desktop/macosx/native/libsplashscreen/splashscreen_sys.m would have to be adjusted to support @3x (or other variants) as well.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Review request for 8029339 Custom MultiResolution image support on HiDPI displays

Jim Graham-5
In reply to this post by Alexandr Scherbatiy
This is an interesting suggestion that would let us keep the
implementation of the various hints centralized and simplify the
interface to the part of the mechanism that is most tied in to the
implementation specifics of the database of media variants - which is
housed in the MRI-implementing object.

I'm not sure we ever identified any need to customize the logic of "what
size is needed for this render operation" beyond what we expressed in
the hints, and given that the platform defaults may not be easy to
express to an arbitrary implementation, it is probably better to put
that part of the logic in SG2D, controlled by the easy to express hints
and keep the current API both simple to implement and flexible to use.  
Even if there was a need to customize that part of the operation (the
"what size is relevant to this rendering operation" decision) beyond
what the hints suggest, it would be inappropriate to tie that in to the
"find me media" aspect of the MRI interface anyway.  So, best to keep
them separate and have the hints affect what SG2D does and have the MRI
focused on just storing (possibly creating) and managing/finding the
variants.

             ...jim

On 4/1/15 6:46 AM, Alexander Scherbatiy wrote:

> On 3/27/2015 12:48 AM, Jim Graham wrote:
>> Hi Alexander,
>
>    http://cr.openjdk.java.net/~alexsch/8029339/webrev.07/
>    I have updated the fix according to the comments except
> RenderingHints.
>
>    RenderingHints are supposed to be set on Graphics2D and they have
> keys/values which are not relevant to the getResolutionVariant() method.
>    Graphics objects chooses an alternative according to the set
> rendering hints.
>    May be what SG2D should do just calculate dest image size for the
> given resolution variant hint (_BASE - base image size, _SIZE_FIT -
> including scale factor and graphics transform (Mac algorithm),
> _DPI_FIT - scaled according to the system DPI) and then pass them to
> the getResolutionVariant() method?
>
>   Thanks,
>   Alexandr.
>
>>
>> MRI.java:
>>
>> Who is the intended audience for the recommendation in the interface
>> to cache image variants?  Are we asking the developers who call the
>> methods on the interface to cache the answers? That would be unwise
>> because the list of variants may change over time for some MRIs.  Are
>> we speaking to the implementer? If so, then I think that it is
>> unnecessary to remind implementers of basic implementation strategies
>> like "cache complicated objects".
>>
>> How about this wording for the getRV() method? - "Gets a specific
>> image that is the best variant to represent this logical image at the
>> indicated size and screen resolution."
>>
>> Perhaps we should clarify in the doc comments for the dimension
>> arguments in getRV() that the dimensions are measured in pixels?
>>
>> line 56 - delete blank line between doc comment and method declaration
>>
>> Also, it is probably overkill to document that the interface
>> getVariants() method returns an unmodifiable list.  Responsible
>> implementations would probably use an unmodifiable list, but I don't
>> think it should be required by the interface.  We do need to specify
>> that it isn't required to be modifiable so that a caller doesn't
>> expect to be able to modify it, but that is a looser spec.  How about
>> "Gets a readable list of all resolution variants.  Note that many
>> implementations might return an unmodifiable list."?
>>
>> AbstractMIR.java:
>>
>> "provides a default implementation for the MRI" or "provides default
>> implementations of several methods in the <MRI> interface and the
>> <Image> class"?  Actually, I'll note that it provides none of the
>> implementations of the MRI methods so maybe it should be "provides
>> default implementations of several <Image> methods for classes that
>> want to implement the <MRI> interface"?
>>
>> In the doc example - wrap the list to make it unmodifiable
>>
>> The doc example could be made 2 or 3 lines shorter by simply assuming
>> the base image is in index 0 (it's just an example so I'm not sure
>> the flexibility needs to be a part of the example).
>>
>> getGraphics is allowed by the Image interface to return null.
>> Actually, the exact wording is that it can only be called for
>> "offscreen images" and a MRI is technically not "an offscreen
>> image".  Perhaps we should return null here since modifying the base
>> image is unlikely to modify the variants and arguably it would be
>> required by the wording in the method docs (even if the base image
>> was an offscreen, the MRI produced from it stops being an offscreen)?
>>
>> Are all of the empty "@Inherit" comments needed?  I thought
>> inheritance was the default?
>>
>> BaseMRI.java:
>>
>> "This class is [an] array-based implementation of the {AMRI} class"
>> (missing "an" and "the")
>>
>> We should probably include the comment about the sorting of the
>> images in the class comments as well as document that the algorithm
>> is a simple scan from the beginning for the first image large enough
>> (and default == last image).  The algorithm also might not work very
>> well if the images are not monotonically both wider and taller.  How
>> about adding this to the class comments:
>>
>> "The implementation will return the first image variant in the array
>> that is large enough to satisfy the rendering request. For best
>> effect the array of images should be sorted with each image being
>> both wider and taller than the previous image.  The base image need
>> not be the first image in the array."
>>
>> getVariants() - you need to return an unmodifiable list. asList()
>> returns a list that "writes back" to the array.  You need to use
>> Collections.unmodifiableList(Arrays.asList(array)).
>>
>> RenderingHints.java:
>>
>> Where do we process this hint?  Don't we need to pass it to the
>> getVariant() method?
>>
>> I don't understand the hint values other than the one that always
>> uses the base image.  Default I guess gives us the ability to use the
>> Mac algorithm of "next larger size" on Mac and "based on Screen DPI"
>> on Windows, but the 3rd value "ON" is so vague as to not have any
>> meaning.  Perhaps we should just delete it, but then do we just
>> always do the Mac method?  Or do we vaguely have our internal images
>> have a platform-specific method and the Abstract/Base classes just do
>> what they want with no control from the user?  In FX we are also
>> still struggling with this issue and we may likely just do what the
>> Mac does in all cases, but perhaps AWT needs to "behave like the
>> platform" more?  If we are going to have actual values, then we need
>> to have them do something, which means:
>>
>> - SG2D needs to track the hint just like we do the other hints that
>> affect our processing
>> - MRI.getVariant() needs to have the hint as an argument
>> - BaseMRI should probably do something with that hint
>> - hint values should include:
>>     - ..._DEFAULT - implementation gets to decide
>>     - ..._BASE - always use the base image
>>     - ..._SIZE_FIT - Mac algorithm of smallest image that is big enough
>>     - ..._DPI_FIT - choose based on DPI of the screen, ignoring
>> transform
>>
>> line 978 - missing blank line between fields
>>
>> SG2D.java:
>>
>> - The interface says that you will be passing in the "logical DPI" of
>> the display, but here you are actually passing in the screen's scale
>> factor.
>>
>> BaseMRITest.java:
>>
>> - testBaseMRI also passes in a scale rather than a DPI to the MRI
>> method.
>>
>> - How does the modification test pass when the implementation doesn't
>> use unmodifiable lists?
>>
>> MRITest.java:
>>
>> - also uses scale rather than DPI in several places
>>
>>         ...jim
>>
>> On 3/13/15 6:34 AM, Alexander Scherbatiy wrote:
>>>
>>>    Hello,
>>>
>>>    Could you review the proposed API based on MultiresolutionImage
>>> interface:
>>>      http://cr.openjdk.java.net/~alexsch/8029339/webrev.06/
>>>
>>>   - return unmodifiable list comment is added to the
>>> getResolutionVariants() method javadoc in MultiresolutionImage
>>> interface
>>>   - base image size arguments are removed form the
>>> getResolutionVariant(...) method in MultiresolutionImage interface
>>>   - BaseMultiResolutionImage class that allows to create a
>>> multi-resolution image based on resolution variant array is added
>>>   - the test for the BaseMultiResolutionImage is added
>>>
>>>   Thanks,
>>>   Alexandr.
>>>
>>> On 2/14/2015 3:23 AM, Jim Graham wrote:
>>>> The second solution looks good.  I'd make it standard practice
>>>> (perhaps even mentioned in the documentation) to return unmodifiable
>>>> lists from the getVariants() method.  The Collections class provides
>>>> easy methods to create these lists, and it sends a clear message to
>>>> the caller that the list was provided for them to read, but not write
>>>> to.  Otherwise they may add a new image to the list you provided them
>>>> and wonder why it wasn't showing up.  Also, an unmodifiable list can
>>>> be cached and reused for subsequent calls without having to create a
>>>> new list every time.
>>>>
>>>> In getResolutionVariant() was there a reason why the base dimensions
>>>> were provided as float?  The destination dimensions make sense as
>>>> float since they could be the result of a scale, but the source
>>>> dimensions are typically getWidth/getHeight() on the base image.  A
>>>> related question would be if they are needed at all, since the
>>>> implementation should probably already be aware of what the base image
>>>> is and what its dimensions are.  I'm guessing they are provided
>>>> because the implementation in SG2D already knows them and it makes it
>>>> easier to forward the implementation on to a shared (static?) method?
>>>>
>>>> With respect to default implementations, I take it that the BaseMRI is
>>>> along the pattern that we see in Swing for Base classes. Would it be
>>>> helpful to provide an implementation (in addition or instead) that
>>>> allows a developer to take a bunch of images and quickly make an MRI
>>>> without having to override anything?  The implementations of
>>>> getBaseImage() and getResolutionVariants() are pretty straightforward
>>>> and a fairly reasonable default algorithm can be provided for
>>>> getRV(dimensions).  This question is more of an idle question for my
>>>> own curiosity than a stumbling block...
>>>>
>>>>             ...jim
>>>>
>>>> On 1/22/2015 6:49 AM, Alexander Scherbatiy wrote:
>>>>>
>>>>>    Hi Phil,
>>>>>
>>>>>    I have prepared two versions of the proposed API:
>>>>>
>>>>>    I) Resolution variants are added directly to the Image:
>>>>> http://cr.openjdk.java.net/~alexsch/8029339/list/webrev.00
>>>>>
>>>>>    II)  MultiResolutionImage interface is used:
>>>>>      http://cr.openjdk.java.net/~alexsch/8029339/webrev.05
>>>>>
>>>>>    It could help to decide which way should be chosen for the the
>>>>> multi-resolution image support.
>>>>>
>>>>>    Below are some comments:
>>>>>
>>>>>    1. High level goal:
>>>>>       Introduce an API that allows to create and handle an image with
>>>>> resolution variants.
>>>>>
>>>>>    2. What is not subject of the provided API
>>>>>      - Scale naming convention for high-resolution images
>>>>>      - Providing pixel scale factor for the screen/window
>>>>>
>>>>>    3. Use cases
>>>>>     3.1 Loading and drawing high-resolution icons in IntelliJ IDEA
>>>>>       A high-resolution image is loaded from resources and stored in
>>>>> JBHiDPIScaledImage class  which is a subclass of the buffered image.
>>>>>       The high-resolution image is used to create a disabled icon
>>>>> in the
>>>>> IconLoader.getDisabledIcon(icon) method.
>>>>> https://github.com/JetBrains/intellij-community/blob/master/platform/util/src/com/intellij/openapi/util/IconLoader.java 
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>     3.2 Loading and drawing high-resolution icons in NetBeans
>>>>>       NetBeans does not have support for the high-resolution icons
>>>>> loading.
>>>>>       It loads an icon from the file system using
>>>>> Toolkit.getDefaultToolkit().getImage(url) method or from resources
>>>>>       by  ImageReader  and store it in ToolTipImage class which is
>>>>> subclass of the buffered image.
>>>>>       ImageUtilities.createDisabledIcon(icon) method creates a
>>>>> disabled
>>>>> icon by applying  RGBImageFilter to the icon.
>>>>> http://hg.netbeans.org/main/file/97dcf49eb4a7/openide.util/src/org/openide/util/ImageUtilities.java 
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>     3.3 Loading system icons in JDK 1.8
>>>>>       JDK requests icons from the native system for system L&Fs and
>>>>> applies filters for them.
>>>>>       See for example AquaUtils.generateLightenedImage() method:
>>>>> http://hg.openjdk.java.net/jdk9/client/jdk/file/e6f48c4fad38/src/java.desktop/macosx/classes/com/apple/laf/AquaUtils.java 
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>    4. HiDPI support for Images on different OSes
>>>>>
>>>>>      4.1 Mac OS X
>>>>>        Cocoa API contains NSImage that allows to work with image
>>>>> representations: add/remove/get all representations.
>>>>>        It picks up an image with necessary resolution based on the
>>>>> screen backing store pixel scale factor and applied transforms.
>>>>> https://developer.apple.com/library/mac/documentation/Cocoa/Reference/ApplicationKit/Classes/NSImage_Class/Reference/Reference.html 
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>      4.2 Linux
>>>>>        GTK+ 3 API has gtkcssimagescaled lib (it seems that it is not
>>>>> public/stable)
>>>>>        that parses the -gtk-scaled css property and draws a
>>>>> GtkCssImage
>>>>> according to the given scale factor.
>>>>>
>>>>>        I have not found information about the HiDPI support in Xlib.
>>>>>
>>>>>      4.3 Windows
>>>>>        I have only found the tutorial that suggests to select and
>>>>> draw a
>>>>> bitmap using the queried DPI
>>>>>        and scale the coordinates for drawing a rectangular frame
>>>>> http://msdn.microsoft.com/en-us/library/dd464659%28v=vs.85%29.aspx
>>>>>
>>>>>        Windows also provides the horizontal and vertical DPI of the
>>>>> desktop
>>>>> http://msdn.microsoft.com/en-us/library/windows/apps/dd371316
>>>>>
>>>>>    5. Pseudo API
>>>>>       Below are some ways which illustrates how multi-resolution
>>>>> images
>>>>> can be created and used.
>>>>>
>>>>>      5.1 Resolution variants are stored directly in Image class.
>>>>>      To query a resolution variant it needs to compare the resolution
>>>>> variant width/height
>>>>>      with the requested high-resolution size.
>>>>>      ------------
>>>>>      public abstract class Image {
>>>>>
>>>>>          public void addResolutionVariant(Image image) {...}
>>>>>          public List<Image> getResolutionVariants() {...}
>>>>>      }
>>>>>      ------------
>>>>>      // create a disabled image with resolution variants
>>>>>
>>>>>      Image disabledImage = getDisabledImage(image);
>>>>>
>>>>>      for (Image rv : image.getResolutionVariants()) {
>>>>> disabledImage.addResolutionVariant(getDisabledImage(rv));
>>>>>      }
>>>>>      ------------
>>>>>      This approach requires that all resolution variants have been
>>>>> created even not of them are really used.
>>>>>
>>>>>      5.2  Resolution variants are stored in a separate object that
>>>>> allows to create them by demand.
>>>>>      To query a resolution variant it needs to compare the resolution
>>>>> variant scale factor
>>>>>      with the requested scale (that can include both screen DPI scale
>>>>> and applied transforms).
>>>>>      ------------
>>>>>      public abstract class Image {
>>>>>
>>>>>          public static interface ResolutionVariant {
>>>>>              Image getImage();
>>>>>              float getScaleFactor();
>>>>>          }
>>>>>
>>>>>          public void addResolutionVariant(ResolutionVariant
>>>>> resolutionVariant) {...}
>>>>>          public List<ResolutionVariant> getResolutionVariants() {...}
>>>>>      }
>>>>>      ------------
>>>>>      // create a disabled image with resolution variants
>>>>>      Image disabledImage = getDisabledImage(image);
>>>>>
>>>>>      for (Image.ResolutionVariant rv :
>>>>> image.getResolutionVariants()) {
>>>>>          disabledImage.addResolutionVariant(new
>>>>> Image.ResolutionVariant() {
>>>>>
>>>>>              public Image getImage() {
>>>>>                  return getDisabledImage(rv.getImage());
>>>>>              }
>>>>>
>>>>>              public float getScaleFactor() {
>>>>>                  return rv.getScaleFactor();
>>>>>              }
>>>>>          });
>>>>>      }
>>>>>      ------------
>>>>>
>>>>>      It does not have problem if a predefined set of images is
>>>>> provided
>>>>> (like image.png and [hidden email] on the file system).
>>>>>      This does not cover cases where a resolution variant can be
>>>>> created
>>>>> using the exact requested size (like loading icons from the native
>>>>> system).
>>>>>      A resolution variant can be queried based on a scale factor and
>>>>> applied transforms.
>>>>>
>>>>>      5.3 The provided example allows to create a resolution variant
>>>>> using the requested high-resolution image size.
>>>>>      ------------
>>>>>      public interface MultiResolutionImage {
>>>>>          Image getResolutionVariant(float width, float height);
>>>>>      }
>>>>>      ------------
>>>>>      // create a multi-resolution image
>>>>>      Image mrImage = new AbstractMultiResolutionImage() {
>>>>>
>>>>>              public Image getResolutionVariant(float width, float
>>>>> height) {
>>>>>                  // create and return a resolution variant with exact
>>>>> requested width/height size
>>>>>              }
>>>>>
>>>>>              protected Image getBaseImage() {
>>>>>                  return baseImage;
>>>>>              }
>>>>>          };
>>>>>      ------------
>>>>>      // create a disabled image with resolution variants
>>>>>      Image disabledImage = null;
>>>>>      if (image instanceof MultiResolutionImage) {
>>>>>          final MultiResolutionImage mrImage = (MultiResolutionImage)
>>>>> image;
>>>>>          disabledImage = new AbstractMultiResolutionImage(){
>>>>>
>>>>>              public Image getResolutionVariant(float width, float
>>>>> height) {
>>>>>                  return
>>>>> getDisabledImage(mrImage.getResolutionVariant(width, height));
>>>>>              }
>>>>>
>>>>>              protected Image getBaseImage() {
>>>>>                  return getDisabledImage(mrImage);
>>>>>              }
>>>>>          };
>>>>>      } else {
>>>>>          disabledImage = getDisabledImage(image);
>>>>>      }
>>>>>      ------------
>>>>>
>>>>>    Thanks,
>>>>>    Alexandr.
>>>
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Review request for 8029339 Custom MultiResolution image support on HiDPI displays

Alexandr Scherbatiy

   Hello,

  Could you review the updated fix:
    http://cr.openjdk.java.net/~alexsch/8029339/webrev.08/

  - SunGraphics2D is updated to calculate the resolution variant size
according to the _BASE, _DPI_FIT, and _SIZE_FIT resolution rendering hint
  - MultiResolutionRenderingHints test is added

  Thanks,
  Alexandr.


On 4/7/2015 1:02 PM, Jim Graham wrote:

> This is an interesting suggestion that would let us keep the
> implementation of the various hints centralized and simplify the
> interface to the part of the mechanism that is most tied in to the
> implementation specifics of the database of media variants - which is
> housed in the MRI-implementing object.
>
> I'm not sure we ever identified any need to customize the logic of
> "what size is needed for this render operation" beyond what we
> expressed in the hints, and given that the platform defaults may not
> be easy to express to an arbitrary implementation, it is probably
> better to put that part of the logic in SG2D, controlled by the easy
> to express hints and keep the current API both simple to implement and
> flexible to use.  Even if there was a need to customize that part of
> the operation (the "what size is relevant to this rendering operation"
> decision) beyond what the hints suggest, it would be inappropriate to
> tie that in to the "find me media" aspect of the MRI interface
> anyway.  So, best to keep them separate and have the hints affect what
> SG2D does and have the MRI focused on just storing (possibly creating)
> and managing/finding the variants.
>
>             ...jim
>
> On 4/1/15 6:46 AM, Alexander Scherbatiy wrote:
>> On 3/27/2015 12:48 AM, Jim Graham wrote:
>>> Hi Alexander,
>>
>>    http://cr.openjdk.java.net/~alexsch/8029339/webrev.07/
>>    I have updated the fix according to the comments except
>> RenderingHints.
>>
>>    RenderingHints are supposed to be set on Graphics2D and they have
>> keys/values which are not relevant to the getResolutionVariant() method.
>>    Graphics objects chooses an alternative according to the set
>> rendering hints.
>>    May be what SG2D should do just calculate dest image size for the
>> given resolution variant hint (_BASE - base image size, _SIZE_FIT -
>> including scale factor and graphics transform (Mac algorithm),
>> _DPI_FIT - scaled according to the system DPI) and then pass them to
>> the getResolutionVariant() method?
>>
>>   Thanks,
>>   Alexandr.
>>
>>>
>>> MRI.java:
>>>
>>> Who is the intended audience for the recommendation in the interface
>>> to cache image variants?  Are we asking the developers who call the
>>> methods on the interface to cache the answers? That would be unwise
>>> because the list of variants may change over time for some MRIs.  
>>> Are we speaking to the implementer? If so, then I think that it is
>>> unnecessary to remind implementers of basic implementation
>>> strategies like "cache complicated objects".
>>>
>>> How about this wording for the getRV() method? - "Gets a specific
>>> image that is the best variant to represent this logical image at
>>> the indicated size and screen resolution."
>>>
>>> Perhaps we should clarify in the doc comments for the dimension
>>> arguments in getRV() that the dimensions are measured in pixels?
>>>
>>> line 56 - delete blank line between doc comment and method declaration
>>>
>>> Also, it is probably overkill to document that the interface
>>> getVariants() method returns an unmodifiable list. Responsible
>>> implementations would probably use an unmodifiable list, but I don't
>>> think it should be required by the interface.  We do need to specify
>>> that it isn't required to be modifiable so that a caller doesn't
>>> expect to be able to modify it, but that is a looser spec.  How
>>> about "Gets a readable list of all resolution variants.  Note that
>>> many implementations might return an unmodifiable list."?
>>>
>>> AbstractMIR.java:
>>>
>>> "provides a default implementation for the MRI" or "provides default
>>> implementations of several methods in the <MRI> interface and the
>>> <Image> class"?  Actually, I'll note that it provides none of the
>>> implementations of the MRI methods so maybe it should be "provides
>>> default implementations of several <Image> methods for classes that
>>> want to implement the <MRI> interface"?
>>>
>>> In the doc example - wrap the list to make it unmodifiable
>>>
>>> The doc example could be made 2 or 3 lines shorter by simply
>>> assuming the base image is in index 0 (it's just an example so I'm
>>> not sure the flexibility needs to be a part of the example).
>>>
>>> getGraphics is allowed by the Image interface to return null.
>>> Actually, the exact wording is that it can only be called for
>>> "offscreen images" and a MRI is technically not "an offscreen
>>> image".  Perhaps we should return null here since modifying the base
>>> image is unlikely to modify the variants and arguably it would be
>>> required by the wording in the method docs (even if the base image
>>> was an offscreen, the MRI produced from it stops being an offscreen)?
>>>
>>> Are all of the empty "@Inherit" comments needed?  I thought
>>> inheritance was the default?
>>>
>>> BaseMRI.java:
>>>
>>> "This class is [an] array-based implementation of the {AMRI} class"
>>> (missing "an" and "the")
>>>
>>> We should probably include the comment about the sorting of the
>>> images in the class comments as well as document that the algorithm
>>> is a simple scan from the beginning for the first image large enough
>>> (and default == last image).  The algorithm also might not work very
>>> well if the images are not monotonically both wider and taller.  How
>>> about adding this to the class comments:
>>>
>>> "The implementation will return the first image variant in the array
>>> that is large enough to satisfy the rendering request. For best
>>> effect the array of images should be sorted with each image being
>>> both wider and taller than the previous image. The base image need
>>> not be the first image in the array."
>>>
>>> getVariants() - you need to return an unmodifiable list. asList()
>>> returns a list that "writes back" to the array.  You need to use
>>> Collections.unmodifiableList(Arrays.asList(array)).
>>>
>>> RenderingHints.java:
>>>
>>> Where do we process this hint?  Don't we need to pass it to the
>>> getVariant() method?
>>>
>>> I don't understand the hint values other than the one that always
>>> uses the base image.  Default I guess gives us the ability to use
>>> the Mac algorithm of "next larger size" on Mac and "based on Screen
>>> DPI" on Windows, but the 3rd value "ON" is so vague as to not have
>>> any meaning.  Perhaps we should just delete it, but then do we just
>>> always do the Mac method? Or do we vaguely have our internal images
>>> have a platform-specific method and the Abstract/Base classes just
>>> do what they want with no control from the user?  In FX we are also
>>> still struggling with this issue and we may likely just do what the
>>> Mac does in all cases, but perhaps AWT needs to "behave like the
>>> platform" more?  If we are going to have actual values, then we need
>>> to have them do something, which means:
>>>
>>> - SG2D needs to track the hint just like we do the other hints that
>>> affect our processing
>>> - MRI.getVariant() needs to have the hint as an argument
>>> - BaseMRI should probably do something with that hint
>>> - hint values should include:
>>>     - ..._DEFAULT - implementation gets to decide
>>>     - ..._BASE - always use the base image
>>>     - ..._SIZE_FIT - Mac algorithm of smallest image that is big enough
>>>     - ..._DPI_FIT - choose based on DPI of the screen, ignoring
>>> transform
>>>
>>> line 978 - missing blank line between fields
>>>
>>> SG2D.java:
>>>
>>> - The interface says that you will be passing in the "logical DPI"
>>> of the display, but here you are actually passing in the screen's
>>> scale factor.
>>>
>>> BaseMRITest.java:
>>>
>>> - testBaseMRI also passes in a scale rather than a DPI to the MRI
>>> method.
>>>
>>> - How does the modification test pass when the implementation
>>> doesn't use unmodifiable lists?
>>>
>>> MRITest.java:
>>>
>>> - also uses scale rather than DPI in several places
>>>
>>>         ...jim
>>>
>>> On 3/13/15 6:34 AM, Alexander Scherbatiy wrote:
>>>>
>>>>    Hello,
>>>>
>>>>    Could you review the proposed API based on MultiresolutionImage
>>>> interface:
>>>>      http://cr.openjdk.java.net/~alexsch/8029339/webrev.06/
>>>>
>>>>   - return unmodifiable list comment is added to the
>>>> getResolutionVariants() method javadoc in MultiresolutionImage
>>>> interface
>>>>   - base image size arguments are removed form the
>>>> getResolutionVariant(...) method in MultiresolutionImage interface
>>>>   - BaseMultiResolutionImage class that allows to create a
>>>> multi-resolution image based on resolution variant array is added
>>>>   - the test for the BaseMultiResolutionImage is added
>>>>
>>>>   Thanks,
>>>>   Alexandr.
>>>>
>>>> On 2/14/2015 3:23 AM, Jim Graham wrote:
>>>>> The second solution looks good.  I'd make it standard practice
>>>>> (perhaps even mentioned in the documentation) to return unmodifiable
>>>>> lists from the getVariants() method.  The Collections class provides
>>>>> easy methods to create these lists, and it sends a clear message to
>>>>> the caller that the list was provided for them to read, but not write
>>>>> to.  Otherwise they may add a new image to the list you provided them
>>>>> and wonder why it wasn't showing up.  Also, an unmodifiable list can
>>>>> be cached and reused for subsequent calls without having to create a
>>>>> new list every time.
>>>>>
>>>>> In getResolutionVariant() was there a reason why the base dimensions
>>>>> were provided as float?  The destination dimensions make sense as
>>>>> float since they could be the result of a scale, but the source
>>>>> dimensions are typically getWidth/getHeight() on the base image.  A
>>>>> related question would be if they are needed at all, since the
>>>>> implementation should probably already be aware of what the base
>>>>> image
>>>>> is and what its dimensions are.  I'm guessing they are provided
>>>>> because the implementation in SG2D already knows them and it makes it
>>>>> easier to forward the implementation on to a shared (static?) method?
>>>>>
>>>>> With respect to default implementations, I take it that the
>>>>> BaseMRI is
>>>>> along the pattern that we see in Swing for Base classes. Would it be
>>>>> helpful to provide an implementation (in addition or instead) that
>>>>> allows a developer to take a bunch of images and quickly make an MRI
>>>>> without having to override anything?  The implementations of
>>>>> getBaseImage() and getResolutionVariants() are pretty straightforward
>>>>> and a fairly reasonable default algorithm can be provided for
>>>>> getRV(dimensions).  This question is more of an idle question for my
>>>>> own curiosity than a stumbling block...
>>>>>
>>>>>             ...jim
>>>>>
>>>>> On 1/22/2015 6:49 AM, Alexander Scherbatiy wrote:
>>>>>>
>>>>>>    Hi Phil,
>>>>>>
>>>>>>    I have prepared two versions of the proposed API:
>>>>>>
>>>>>>    I) Resolution variants are added directly to the Image:
>>>>>> http://cr.openjdk.java.net/~alexsch/8029339/list/webrev.00
>>>>>>
>>>>>>    II)  MultiResolutionImage interface is used:
>>>>>> http://cr.openjdk.java.net/~alexsch/8029339/webrev.05
>>>>>>
>>>>>>    It could help to decide which way should be chosen for the the
>>>>>> multi-resolution image support.
>>>>>>
>>>>>>    Below are some comments:
>>>>>>
>>>>>>    1. High level goal:
>>>>>>       Introduce an API that allows to create and handle an image
>>>>>> with
>>>>>> resolution variants.
>>>>>>
>>>>>>    2. What is not subject of the provided API
>>>>>>      - Scale naming convention for high-resolution images
>>>>>>      - Providing pixel scale factor for the screen/window
>>>>>>
>>>>>>    3. Use cases
>>>>>>     3.1 Loading and drawing high-resolution icons in IntelliJ IDEA
>>>>>>       A high-resolution image is loaded from resources and stored in
>>>>>> JBHiDPIScaledImage class  which is a subclass of the buffered image.
>>>>>>       The high-resolution image is used to create a disabled icon
>>>>>> in the
>>>>>> IconLoader.getDisabledIcon(icon) method.
>>>>>> https://github.com/JetBrains/intellij-community/blob/master/platform/util/src/com/intellij/openapi/util/IconLoader.java 
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>     3.2 Loading and drawing high-resolution icons in NetBeans
>>>>>>       NetBeans does not have support for the high-resolution icons
>>>>>> loading.
>>>>>>       It loads an icon from the file system using
>>>>>> Toolkit.getDefaultToolkit().getImage(url) method or from resources
>>>>>>       by  ImageReader  and store it in ToolTipImage class which is
>>>>>> subclass of the buffered image.
>>>>>>       ImageUtilities.createDisabledIcon(icon) method creates a
>>>>>> disabled
>>>>>> icon by applying  RGBImageFilter to the icon.
>>>>>> http://hg.netbeans.org/main/file/97dcf49eb4a7/openide.util/src/org/openide/util/ImageUtilities.java 
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>     3.3 Loading system icons in JDK 1.8
>>>>>>       JDK requests icons from the native system for system L&Fs and
>>>>>> applies filters for them.
>>>>>>       See for example AquaUtils.generateLightenedImage() method:
>>>>>> http://hg.openjdk.java.net/jdk9/client/jdk/file/e6f48c4fad38/src/java.desktop/macosx/classes/com/apple/laf/AquaUtils.java 
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>    4. HiDPI support for Images on different OSes
>>>>>>
>>>>>>      4.1 Mac OS X
>>>>>>        Cocoa API contains NSImage that allows to work with image
>>>>>> representations: add/remove/get all representations.
>>>>>>        It picks up an image with necessary resolution based on the
>>>>>> screen backing store pixel scale factor and applied transforms.
>>>>>> https://developer.apple.com/library/mac/documentation/Cocoa/Reference/ApplicationKit/Classes/NSImage_Class/Reference/Reference.html 
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>      4.2 Linux
>>>>>>        GTK+ 3 API has gtkcssimagescaled lib (it seems that it is not
>>>>>> public/stable)
>>>>>>        that parses the -gtk-scaled css property and draws a
>>>>>> GtkCssImage
>>>>>> according to the given scale factor.
>>>>>>
>>>>>>        I have not found information about the HiDPI support in Xlib.
>>>>>>
>>>>>>      4.3 Windows
>>>>>>        I have only found the tutorial that suggests to select and
>>>>>> draw a
>>>>>> bitmap using the queried DPI
>>>>>>        and scale the coordinates for drawing a rectangular frame
>>>>>> http://msdn.microsoft.com/en-us/library/dd464659%28v=vs.85%29.aspx
>>>>>>
>>>>>>        Windows also provides the horizontal and vertical DPI of the
>>>>>> desktop
>>>>>> http://msdn.microsoft.com/en-us/library/windows/apps/dd371316
>>>>>>
>>>>>>    5. Pseudo API
>>>>>>       Below are some ways which illustrates how multi-resolution
>>>>>> images
>>>>>> can be created and used.
>>>>>>
>>>>>>      5.1 Resolution variants are stored directly in Image class.
>>>>>>      To query a resolution variant it needs to compare the
>>>>>> resolution
>>>>>> variant width/height
>>>>>>      with the requested high-resolution size.
>>>>>>      ------------
>>>>>>      public abstract class Image {
>>>>>>
>>>>>>          public void addResolutionVariant(Image image) {...}
>>>>>>          public List<Image> getResolutionVariants() {...}
>>>>>>      }
>>>>>>      ------------
>>>>>>      // create a disabled image with resolution variants
>>>>>>
>>>>>>      Image disabledImage = getDisabledImage(image);
>>>>>>
>>>>>>      for (Image rv : image.getResolutionVariants()) {
>>>>>> disabledImage.addResolutionVariant(getDisabledImage(rv));
>>>>>>      }
>>>>>>      ------------
>>>>>>      This approach requires that all resolution variants have been
>>>>>> created even not of them are really used.
>>>>>>
>>>>>>      5.2  Resolution variants are stored in a separate object that
>>>>>> allows to create them by demand.
>>>>>>      To query a resolution variant it needs to compare the
>>>>>> resolution
>>>>>> variant scale factor
>>>>>>      with the requested scale (that can include both screen DPI
>>>>>> scale
>>>>>> and applied transforms).
>>>>>>      ------------
>>>>>>      public abstract class Image {
>>>>>>
>>>>>>          public static interface ResolutionVariant {
>>>>>>              Image getImage();
>>>>>>              float getScaleFactor();
>>>>>>          }
>>>>>>
>>>>>>          public void addResolutionVariant(ResolutionVariant
>>>>>> resolutionVariant) {...}
>>>>>>          public List<ResolutionVariant> getResolutionVariants()
>>>>>> {...}
>>>>>>      }
>>>>>>      ------------
>>>>>>      // create a disabled image with resolution variants
>>>>>>      Image disabledImage = getDisabledImage(image);
>>>>>>
>>>>>>      for (Image.ResolutionVariant rv :
>>>>>> image.getResolutionVariants()) {
>>>>>>          disabledImage.addResolutionVariant(new
>>>>>> Image.ResolutionVariant() {
>>>>>>
>>>>>>              public Image getImage() {
>>>>>>                  return getDisabledImage(rv.getImage());
>>>>>>              }
>>>>>>
>>>>>>              public float getScaleFactor() {
>>>>>>                  return rv.getScaleFactor();
>>>>>>              }
>>>>>>          });
>>>>>>      }
>>>>>>      ------------
>>>>>>
>>>>>>      It does not have problem if a predefined set of images is
>>>>>> provided
>>>>>> (like image.png and [hidden email] on the file system).
>>>>>>      This does not cover cases where a resolution variant can be
>>>>>> created
>>>>>> using the exact requested size (like loading icons from the native
>>>>>> system).
>>>>>>      A resolution variant can be queried based on a scale factor and
>>>>>> applied transforms.
>>>>>>
>>>>>>      5.3 The provided example allows to create a resolution variant
>>>>>> using the requested high-resolution image size.
>>>>>>      ------------
>>>>>>      public interface MultiResolutionImage {
>>>>>>          Image getResolutionVariant(float width, float height);
>>>>>>      }
>>>>>>      ------------
>>>>>>      // create a multi-resolution image
>>>>>>      Image mrImage = new AbstractMultiResolutionImage() {
>>>>>>
>>>>>>              public Image getResolutionVariant(float width, float
>>>>>> height) {
>>>>>>                  // create and return a resolution variant with
>>>>>> exact
>>>>>> requested width/height size
>>>>>>              }
>>>>>>
>>>>>>              protected Image getBaseImage() {
>>>>>>                  return baseImage;
>>>>>>              }
>>>>>>          };
>>>>>>      ------------
>>>>>>      // create a disabled image with resolution variants
>>>>>>      Image disabledImage = null;
>>>>>>      if (image instanceof MultiResolutionImage) {
>>>>>>          final MultiResolutionImage mrImage = (MultiResolutionImage)
>>>>>> image;
>>>>>>          disabledImage = new AbstractMultiResolutionImage(){
>>>>>>
>>>>>>              public Image getResolutionVariant(float width, float
>>>>>> height) {
>>>>>>                  return
>>>>>> getDisabledImage(mrImage.getResolutionVariant(width, height));
>>>>>>              }
>>>>>>
>>>>>>              protected Image getBaseImage() {
>>>>>>                  return getDisabledImage(mrImage);
>>>>>>              }
>>>>>>          };
>>>>>>      } else {
>>>>>>          disabledImage = getDisabledImage(image);
>>>>>>      }
>>>>>>      ------------
>>>>>>
>>>>>>    Thanks,
>>>>>>    Alexandr.
>>>>
>>
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Review request for 8029339 Custom MultiResolution image support on HiDPI displays

Jim Graham-5
Hi Alexandr,

Sorry that this fell into the cracks.  Here are some fairly minor updates:

AbstractMRI - getGraphics() should include "getGraphics() not supported
on Multi-Resolution Images" as the exception description text.

AbstractMRI - getBaseImage() should have doc comments:
/**
  * Return the base image representing the best version of the image
  * for rendering at the default width and height.
  * @return the base image of the set of multi-resolution images
  */
(Does it need an @since given that the entire interface is @since 1.9?)

BaseMRI - the doc comments (both the class comment and the constructor
comments) don't start with "/**" so they aren't really doc comments, but
they look good so make them so.

BaseMRI - class comment - "The implementation will return the first ...
satisfy the rendering request."  Add another sentence right there "The
last image in the array will be returned if no suitable image is found
that is larger than the rendering request."

BaseMRI - move "For best effect ..." should be moved to a new paragraph
and mention that no exception will be thrown if the images are not
sorted as suggested.

RenderingHints - comments:
DEFAULT, SIZE_FIT, DPI_FIT - "an image resolution variant is chosen ..."
(add "an")
DEFAULT - "... which may depend on the policies of the platform"
SIZE_FIT, DPI_FIT "... on the DPI of ..." (add "the")

SunHints - descriptor texts:
SIZE_FIT, DPI_FIT "based on the DPI" (add "the")

MRCachedImage - remind me what this is used for?
MRCachedImage.getResolutionVariant - use ceil or round instead of (int)
cast?  ceil() would match the actions of MRToolkitImage better.  Also
note following comment about precision with SG2D.

SG2D/MRI - the interface declares the values as float, the usage in SG2D
computes values in double and then truncates them to int to pass to the
interface - should we upgrade the interface to double for completeness?
  If not, then the usage in SG2D should at least pass in float precision.

SG2D.getResolutionVariant - using destRegionWH to calculate destImageWH
can involve a lot of "do some math and then later have additional code
undo it".  Would it be faster to just compute destImageWH directly, as in:

float destImageWidth, destImageHeight;

if (BASE) {
     destImageWH = srcWH;
} else if (DPI) {
     destImageWH = (float) abs(srcWH * devScale);
} else {
     double destRegionWidth, destRegionHeight;
     if (type) {
     ...
     }
     destImageWH = (float) abs(srcWH * destRegionWH / swh);
}
Image rv = img.getRV(destImageWH);

On the other hand, the last "else" is probably the most common case so
this doesn't save anything in the common case, but it avoids a bunch of
math that could introduce rounding error for the BASE/DPI cases (even
though it is done in doubles).

Is there intent to have the default case be mapped to DPI_FIT for Windows?

MultiResolutionRenderinHints.java - should have "Test" appended to the name

MRRHTest - why not render to a BufferedImage and avoid Robot?  Could it
tap into internal APIs to create a BImg/compatibleImage with a given
devScale?

MRRHTest - why allow up to delta=50 on the comparison?

MRRHTest - since the scale factor comes from a dialog, we depend on
running this on a HiDPI display to fully test the hints?  Could using
"scaled compatible images" allow testing this more definitively?

MRRHTest - remove println before pushing?

MRRHTest - probably shouldn't have a "right answer" for DEFAULT - it
should probably just pass if the operation doesn't throw an exception?

                        ...jim


On 4/15/15 7:04 AM, Alexander Scherbatiy wrote:

>
>    Hello,
>
>   Could you review the updated fix:
>     http://cr.openjdk.java.net/~alexsch/8029339/webrev.08/
>
>   - SunGraphics2D is updated to calculate the resolution variant size
> according to the _BASE, _DPI_FIT, and _SIZE_FIT resolution rendering hint
>   - MultiResolutionRenderingHints test is added
>
>   Thanks,
>   Alexandr.
>
>
> On 4/7/2015 1:02 PM, Jim Graham wrote:
>> This is an interesting suggestion that would let us keep the
>> implementation of the various hints centralized and simplify the
>> interface to the part of the mechanism that is most tied in to the
>> implementation specifics of the database of media variants - which is
>> housed in the MRI-implementing object.
>>
>> I'm not sure we ever identified any need to customize the logic of
>> "what size is needed for this render operation" beyond what we
>> expressed in the hints, and given that the platform defaults may not
>> be easy to express to an arbitrary implementation, it is probably
>> better to put that part of the logic in SG2D, controlled by the easy
>> to express hints and keep the current API both simple to implement and
>> flexible to use.  Even if there was a need to customize that part of
>> the operation (the "what size is relevant to this rendering operation"
>> decision) beyond what the hints suggest, it would be inappropriate to
>> tie that in to the "find me media" aspect of the MRI interface
>> anyway.  So, best to keep them separate and have the hints affect what
>> SG2D does and have the MRI focused on just storing (possibly creating)
>> and managing/finding the variants.
>>
>>             ...jim
>>
>> On 4/1/15 6:46 AM, Alexander Scherbatiy wrote:
>>> On 3/27/2015 12:48 AM, Jim Graham wrote:
>>>> Hi Alexander,
>>>
>>>    http://cr.openjdk.java.net/~alexsch/8029339/webrev.07/
>>>    I have updated the fix according to the comments except
>>> RenderingHints.
>>>
>>>    RenderingHints are supposed to be set on Graphics2D and they have
>>> keys/values which are not relevant to the getResolutionVariant() method.
>>>    Graphics objects chooses an alternative according to the set
>>> rendering hints.
>>>    May be what SG2D should do just calculate dest image size for the
>>> given resolution variant hint (_BASE - base image size, _SIZE_FIT -
>>> including scale factor and graphics transform (Mac algorithm),
>>> _DPI_FIT - scaled according to the system DPI) and then pass them to
>>> the getResolutionVariant() method?
>>>
>>>   Thanks,
>>>   Alexandr.
>>>
>>>>
>>>> MRI.java:
>>>>
>>>> Who is the intended audience for the recommendation in the interface
>>>> to cache image variants?  Are we asking the developers who call the
>>>> methods on the interface to cache the answers? That would be unwise
>>>> because the list of variants may change over time for some MRIs. Are
>>>> we speaking to the implementer? If so, then I think that it is
>>>> unnecessary to remind implementers of basic implementation
>>>> strategies like "cache complicated objects".
>>>>
>>>> How about this wording for the getRV() method? - "Gets a specific
>>>> image that is the best variant to represent this logical image at
>>>> the indicated size and screen resolution."
>>>>
>>>> Perhaps we should clarify in the doc comments for the dimension
>>>> arguments in getRV() that the dimensions are measured in pixels?
>>>>
>>>> line 56 - delete blank line between doc comment and method declaration
>>>>
>>>> Also, it is probably overkill to document that the interface
>>>> getVariants() method returns an unmodifiable list. Responsible
>>>> implementations would probably use an unmodifiable list, but I don't
>>>> think it should be required by the interface.  We do need to specify
>>>> that it isn't required to be modifiable so that a caller doesn't
>>>> expect to be able to modify it, but that is a looser spec.  How
>>>> about "Gets a readable list of all resolution variants.  Note that
>>>> many implementations might return an unmodifiable list."?
>>>>
>>>> AbstractMIR.java:
>>>>
>>>> "provides a default implementation for the MRI" or "provides default
>>>> implementations of several methods in the <MRI> interface and the
>>>> <Image> class"?  Actually, I'll note that it provides none of the
>>>> implementations of the MRI methods so maybe it should be "provides
>>>> default implementations of several <Image> methods for classes that
>>>> want to implement the <MRI> interface"?
>>>>
>>>> In the doc example - wrap the list to make it unmodifiable
>>>>
>>>> The doc example could be made 2 or 3 lines shorter by simply
>>>> assuming the base image is in index 0 (it's just an example so I'm
>>>> not sure the flexibility needs to be a part of the example).
>>>>
>>>> getGraphics is allowed by the Image interface to return null.
>>>> Actually, the exact wording is that it can only be called for
>>>> "offscreen images" and a MRI is technically not "an offscreen
>>>> image".  Perhaps we should return null here since modifying the base
>>>> image is unlikely to modify the variants and arguably it would be
>>>> required by the wording in the method docs (even if the base image
>>>> was an offscreen, the MRI produced from it stops being an offscreen)?
>>>>
>>>> Are all of the empty "@Inherit" comments needed?  I thought
>>>> inheritance was the default?
>>>>
>>>> BaseMRI.java:
>>>>
>>>> "This class is [an] array-based implementation of the {AMRI} class"
>>>> (missing "an" and "the")
>>>>
>>>> We should probably include the comment about the sorting of the
>>>> images in the class comments as well as document that the algorithm
>>>> is a simple scan from the beginning for the first image large enough
>>>> (and default == last image).  The algorithm also might not work very
>>>> well if the images are not monotonically both wider and taller.  How
>>>> about adding this to the class comments:
>>>>
>>>> "The implementation will return the first image variant in the array
>>>> that is large enough to satisfy the rendering request. For best
>>>> effect the array of images should be sorted with each image being
>>>> both wider and taller than the previous image. The base image need
>>>> not be the first image in the array."
>>>>
>>>> getVariants() - you need to return an unmodifiable list. asList()
>>>> returns a list that "writes back" to the array.  You need to use
>>>> Collections.unmodifiableList(Arrays.asList(array)).
>>>>
>>>> RenderingHints.java:
>>>>
>>>> Where do we process this hint?  Don't we need to pass it to the
>>>> getVariant() method?
>>>>
>>>> I don't understand the hint values other than the one that always
>>>> uses the base image.  Default I guess gives us the ability to use
>>>> the Mac algorithm of "next larger size" on Mac and "based on Screen
>>>> DPI" on Windows, but the 3rd value "ON" is so vague as to not have
>>>> any meaning.  Perhaps we should just delete it, but then do we just
>>>> always do the Mac method? Or do we vaguely have our internal images
>>>> have a platform-specific method and the Abstract/Base classes just
>>>> do what they want with no control from the user?  In FX we are also
>>>> still struggling with this issue and we may likely just do what the
>>>> Mac does in all cases, but perhaps AWT needs to "behave like the
>>>> platform" more?  If we are going to have actual values, then we need
>>>> to have them do something, which means:
>>>>
>>>> - SG2D needs to track the hint just like we do the other hints that
>>>> affect our processing
>>>> - MRI.getVariant() needs to have the hint as an argument
>>>> - BaseMRI should probably do something with that hint
>>>> - hint values should include:
>>>>     - ..._DEFAULT - implementation gets to decide
>>>>     - ..._BASE - always use the base image
>>>>     - ..._SIZE_FIT - Mac algorithm of smallest image that is big enough
>>>>     - ..._DPI_FIT - choose based on DPI of the screen, ignoring
>>>> transform
>>>>
>>>> line 978 - missing blank line between fields
>>>>
>>>> SG2D.java:
>>>>
>>>> - The interface says that you will be passing in the "logical DPI"
>>>> of the display, but here you are actually passing in the screen's
>>>> scale factor.
>>>>
>>>> BaseMRITest.java:
>>>>
>>>> - testBaseMRI also passes in a scale rather than a DPI to the MRI
>>>> method.
>>>>
>>>> - How does the modification test pass when the implementation
>>>> doesn't use unmodifiable lists?
>>>>
>>>> MRITest.java:
>>>>
>>>> - also uses scale rather than DPI in several places
>>>>
>>>>         ...jim
>>>>
>>>> On 3/13/15 6:34 AM, Alexander Scherbatiy wrote:
>>>>>
>>>>>    Hello,
>>>>>
>>>>>    Could you review the proposed API based on MultiresolutionImage
>>>>> interface:
>>>>>      http://cr.openjdk.java.net/~alexsch/8029339/webrev.06/
>>>>>
>>>>>   - return unmodifiable list comment is added to the
>>>>> getResolutionVariants() method javadoc in MultiresolutionImage
>>>>> interface
>>>>>   - base image size arguments are removed form the
>>>>> getResolutionVariant(...) method in MultiresolutionImage interface
>>>>>   - BaseMultiResolutionImage class that allows to create a
>>>>> multi-resolution image based on resolution variant array is added
>>>>>   - the test for the BaseMultiResolutionImage is added
>>>>>
>>>>>   Thanks,
>>>>>   Alexandr.
>>>>>
>>>>> On 2/14/2015 3:23 AM, Jim Graham wrote:
>>>>>> The second solution looks good.  I'd make it standard practice
>>>>>> (perhaps even mentioned in the documentation) to return unmodifiable
>>>>>> lists from the getVariants() method.  The Collections class provides
>>>>>> easy methods to create these lists, and it sends a clear message to
>>>>>> the caller that the list was provided for them to read, but not write
>>>>>> to.  Otherwise they may add a new image to the list you provided them
>>>>>> and wonder why it wasn't showing up.  Also, an unmodifiable list can
>>>>>> be cached and reused for subsequent calls without having to create a
>>>>>> new list every time.
>>>>>>
>>>>>> In getResolutionVariant() was there a reason why the base dimensions
>>>>>> were provided as float?  The destination dimensions make sense as
>>>>>> float since they could be the result of a scale, but the source
>>>>>> dimensions are typically getWidth/getHeight() on the base image.  A
>>>>>> related question would be if they are needed at all, since the
>>>>>> implementation should probably already be aware of what the base
>>>>>> image
>>>>>> is and what its dimensions are.  I'm guessing they are provided
>>>>>> because the implementation in SG2D already knows them and it makes it
>>>>>> easier to forward the implementation on to a shared (static?) method?
>>>>>>
>>>>>> With respect to default implementations, I take it that the
>>>>>> BaseMRI is
>>>>>> along the pattern that we see in Swing for Base classes. Would it be
>>>>>> helpful to provide an implementation (in addition or instead) that
>>>>>> allows a developer to take a bunch of images and quickly make an MRI
>>>>>> without having to override anything?  The implementations of
>>>>>> getBaseImage() and getResolutionVariants() are pretty straightforward
>>>>>> and a fairly reasonable default algorithm can be provided for
>>>>>> getRV(dimensions).  This question is more of an idle question for my
>>>>>> own curiosity than a stumbling block...
>>>>>>
>>>>>>             ...jim
>>>>>>
>>>>>> On 1/22/2015 6:49 AM, Alexander Scherbatiy wrote:
>>>>>>>
>>>>>>>    Hi Phil,
>>>>>>>
>>>>>>>    I have prepared two versions of the proposed API:
>>>>>>>
>>>>>>>    I) Resolution variants are added directly to the Image:
>>>>>>> http://cr.openjdk.java.net/~alexsch/8029339/list/webrev.00
>>>>>>>
>>>>>>>    II)  MultiResolutionImage interface is used:
>>>>>>> http://cr.openjdk.java.net/~alexsch/8029339/webrev.05
>>>>>>>
>>>>>>>    It could help to decide which way should be chosen for the the
>>>>>>> multi-resolution image support.
>>>>>>>
>>>>>>>    Below are some comments:
>>>>>>>
>>>>>>>    1. High level goal:
>>>>>>>       Introduce an API that allows to create and handle an image
>>>>>>> with
>>>>>>> resolution variants.
>>>>>>>
>>>>>>>    2. What is not subject of the provided API
>>>>>>>      - Scale naming convention for high-resolution images
>>>>>>>      - Providing pixel scale factor for the screen/window
>>>>>>>
>>>>>>>    3. Use cases
>>>>>>>     3.1 Loading and drawing high-resolution icons in IntelliJ IDEA
>>>>>>>       A high-resolution image is loaded from resources and stored in
>>>>>>> JBHiDPIScaledImage class  which is a subclass of the buffered image.
>>>>>>>       The high-resolution image is used to create a disabled icon
>>>>>>> in the
>>>>>>> IconLoader.getDisabledIcon(icon) method.
>>>>>>> https://github.com/JetBrains/intellij-community/blob/master/platform/util/src/com/intellij/openapi/util/IconLoader.java
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>     3.2 Loading and drawing high-resolution icons in NetBeans
>>>>>>>       NetBeans does not have support for the high-resolution icons
>>>>>>> loading.
>>>>>>>       It loads an icon from the file system using
>>>>>>> Toolkit.getDefaultToolkit().getImage(url) method or from resources
>>>>>>>       by  ImageReader  and store it in ToolTipImage class which is
>>>>>>> subclass of the buffered image.
>>>>>>>       ImageUtilities.createDisabledIcon(icon) method creates a
>>>>>>> disabled
>>>>>>> icon by applying  RGBImageFilter to the icon.
>>>>>>> http://hg.netbeans.org/main/file/97dcf49eb4a7/openide.util/src/org/openide/util/ImageUtilities.java
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>     3.3 Loading system icons in JDK 1.8
>>>>>>>       JDK requests icons from the native system for system L&Fs and
>>>>>>> applies filters for them.
>>>>>>>       See for example AquaUtils.generateLightenedImage() method:
>>>>>>> http://hg.openjdk.java.net/jdk9/client/jdk/file/e6f48c4fad38/src/java.desktop/macosx/classes/com/apple/laf/AquaUtils.java
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>    4. HiDPI support for Images on different OSes
>>>>>>>
>>>>>>>      4.1 Mac OS X
>>>>>>>        Cocoa API contains NSImage that allows to work with image
>>>>>>> representations: add/remove/get all representations.
>>>>>>>        It picks up an image with necessary resolution based on the
>>>>>>> screen backing store pixel scale factor and applied transforms.
>>>>>>> https://developer.apple.com/library/mac/documentation/Cocoa/Reference/ApplicationKit/Classes/NSImage_Class/Reference/Reference.html
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>      4.2 Linux
>>>>>>>        GTK+ 3 API has gtkcssimagescaled lib (it seems that it is not
>>>>>>> public/stable)
>>>>>>>        that parses the -gtk-scaled css property and draws a
>>>>>>> GtkCssImage
>>>>>>> according to the given scale factor.
>>>>>>>
>>>>>>>        I have not found information about the HiDPI support in Xlib.
>>>>>>>
>>>>>>>      4.3 Windows
>>>>>>>        I have only found the tutorial that suggests to select and
>>>>>>> draw a
>>>>>>> bitmap using the queried DPI
>>>>>>>        and scale the coordinates for drawing a rectangular frame
>>>>>>> http://msdn.microsoft.com/en-us/library/dd464659%28v=vs.85%29.aspx
>>>>>>>
>>>>>>>        Windows also provides the horizontal and vertical DPI of the
>>>>>>> desktop
>>>>>>> http://msdn.microsoft.com/en-us/library/windows/apps/dd371316
>>>>>>>
>>>>>>>    5. Pseudo API
>>>>>>>       Below are some ways which illustrates how multi-resolution
>>>>>>> images
>>>>>>> can be created and used.
>>>>>>>
>>>>>>>      5.1 Resolution variants are stored directly in Image class.
>>>>>>>      To query a resolution variant it needs to compare the
>>>>>>> resolution
>>>>>>> variant width/height
>>>>>>>      with the requested high-resolution size.
>>>>>>>      ------------
>>>>>>>      public abstract class Image {
>>>>>>>
>>>>>>>          public void addResolutionVariant(Image image) {...}
>>>>>>>          public List<Image> getResolutionVariants() {...}
>>>>>>>      }
>>>>>>>      ------------
>>>>>>>      // create a disabled image with resolution variants
>>>>>>>
>>>>>>>      Image disabledImage = getDisabledImage(image);
>>>>>>>
>>>>>>>      for (Image rv : image.getResolutionVariants()) {
>>>>>>> disabledImage.addResolutionVariant(getDisabledImage(rv));
>>>>>>>      }
>>>>>>>      ------------
>>>>>>>      This approach requires that all resolution variants have been
>>>>>>> created even not of them are really used.
>>>>>>>
>>>>>>>      5.2  Resolution variants are stored in a separate object that
>>>>>>> allows to create them by demand.
>>>>>>>      To query a resolution variant it needs to compare the
>>>>>>> resolution
>>>>>>> variant scale factor
>>>>>>>      with the requested scale (that can include both screen DPI
>>>>>>> scale
>>>>>>> and applied transforms).
>>>>>>>      ------------
>>>>>>>      public abstract class Image {
>>>>>>>
>>>>>>>          public static interface ResolutionVariant {
>>>>>>>              Image getImage();
>>>>>>>              float getScaleFactor();
>>>>>>>          }
>>>>>>>
>>>>>>>          public void addResolutionVariant(ResolutionVariant
>>>>>>> resolutionVariant) {...}
>>>>>>>          public List<ResolutionVariant> getResolutionVariants()
>>>>>>> {...}
>>>>>>>      }
>>>>>>>      ------------
>>>>>>>      // create a disabled image with resolution variants
>>>>>>>      Image disabledImage = getDisabledImage(image);
>>>>>>>
>>>>>>>      for (Image.ResolutionVariant rv :
>>>>>>> image.getResolutionVariants()) {
>>>>>>>          disabledImage.addResolutionVariant(new
>>>>>>> Image.ResolutionVariant() {
>>>>>>>
>>>>>>>              public Image getImage() {
>>>>>>>                  return getDisabledImage(rv.getImage());
>>>>>>>              }
>>>>>>>
>>>>>>>              public float getScaleFactor() {
>>>>>>>                  return rv.getScaleFactor();
>>>>>>>              }
>>>>>>>          });
>>>>>>>      }
>>>>>>>      ------------
>>>>>>>
>>>>>>>      It does not have problem if a predefined set of images is
>>>>>>> provided
>>>>>>> (like image.png and [hidden email] on the file system).
>>>>>>>      This does not cover cases where a resolution variant can be
>>>>>>> created
>>>>>>> using the exact requested size (like loading icons from the native
>>>>>>> system).
>>>>>>>      A resolution variant can be queried based on a scale factor and
>>>>>>> applied transforms.
>>>>>>>
>>>>>>>      5.3 The provided example allows to create a resolution variant
>>>>>>> using the requested high-resolution image size.
>>>>>>>      ------------
>>>>>>>      public interface MultiResolutionImage {
>>>>>>>          Image getResolutionVariant(float width, float height);
>>>>>>>      }
>>>>>>>      ------------
>>>>>>>      // create a multi-resolution image
>>>>>>>      Image mrImage = new AbstractMultiResolutionImage() {
>>>>>>>
>>>>>>>              public Image getResolutionVariant(float width, float
>>>>>>> height) {
>>>>>>>                  // create and return a resolution variant with
>>>>>>> exact
>>>>>>> requested width/height size
>>>>>>>              }
>>>>>>>
>>>>>>>              protected Image getBaseImage() {
>>>>>>>                  return baseImage;
>>>>>>>              }
>>>>>>>          };
>>>>>>>      ------------
>>>>>>>      // create a disabled image with resolution variants
>>>>>>>      Image disabledImage = null;
>>>>>>>      if (image instanceof MultiResolutionImage) {
>>>>>>>          final MultiResolutionImage mrImage = (MultiResolutionImage)
>>>>>>> image;
>>>>>>>          disabledImage = new AbstractMultiResolutionImage(){
>>>>>>>
>>>>>>>              public Image getResolutionVariant(float width, float
>>>>>>> height) {
>>>>>>>                  return
>>>>>>> getDisabledImage(mrImage.getResolutionVariant(width, height));
>>>>>>>              }
>>>>>>>
>>>>>>>              protected Image getBaseImage() {
>>>>>>>                  return getDisabledImage(mrImage);
>>>>>>>              }
>>>>>>>          };
>>>>>>>      } else {
>>>>>>>          disabledImage = getDisabledImage(image);
>>>>>>>      }
>>>>>>>      ------------
>>>>>>>
>>>>>>>    Thanks,
>>>>>>>    Alexandr.
>>>>>
>>>
>>
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: <AWT Dev> Review request for 8029339 Custom MultiResolution image support on HiDPI displays

Alan Snyder-3
I have a concern about how custom multiresolution images are supported based on a problem I ran into using the JDK 8 classes, which are similar.

The problem is that the selection of a variant image happens during painting. That is a very bad time to actually try to render an image, if one is in a situation where images of arbitrary resolution can be produced by rendering.

If I have code now that uses a background thread to create an image in this manner, how can I support HiDPI displays? Creating a multiresolution image defeats the purpose of using a background thread if it defers rendering until paint time. It seems that the best I can do is to guess what resolutions might be wanted and generate them all in advance or, alternatively, generate the highest resolution I expect to need, and have the graphics system scale it down for lower resolutions when requested. Neither of these alternatives is optimal.

Although the Image class does allow images to be produced in the background, that API is way too complex for use by an application, and the supporting code for Toolkit images is not accessible.

It would be nice to have a simple way to return an image that is produced in the background. Something like Future<Image>?

  Alan

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: <AWT Dev> Review request for 8029339 Custom MultiResolution image support on HiDPI displays

Alexandr Scherbatiy
On 7/14/2015 3:18 AM, Alan Snyder wrote:
> I have a concern about how custom multiresolution images are supported based on a problem I ran into using the JDK 8 classes, which are similar.
>
> The problem is that the selection of a variant image happens during painting. That is a very bad time to actually try to render an image, if one is in a situation where images of arbitrary resolution can be produced by rendering.
>
> If I have code now that uses a background thread to create an image in this manner, how can I support HiDPI displays? Creating a multiresolution image defeats the purpose of using a background thread if it defers rendering until paint time. It seems that the best I can do is to guess what resolutions might be wanted and generate them all in advance or, alternatively, generate the highest resolution I expect to need, and have the graphics system scale it down for lower resolutions when requested. Neither of these alternatives is optimal.
    JDK 9 has the fix that includes the scale factor in the
GraphicsConfiguration default transform:
http://hg.openjdk.java.net/jdk9/client/jdk/rev/661136704d07

    It is possible to iterate over all graphics devices, decide on which
device the image is supposed to be rendered and generate an image
resolution according to the scale factor from the device graphics
configuration.

   Thanks,
   Alexandr.

>
> Although the Image class does allow images to be produced in the background, that API is way too complex for use by an application, and the supporting code for Toolkit images is not accessible.
>
> It would be nice to have a simple way to return an image that is produced in the background. Something like Future<Image>?
>
>    Alan
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: <AWT Dev> Review request for 8029339 Custom MultiResolution image support on HiDPI displays

Alan Snyder-3

It seems you are making an assumption that the code that creates the image is somehow initiated from a paint method so that the display scale factor can be obtained from the graphics context and used to determine the required image resolution. That is not the scenario I am concerned about.

I am talking about what I would consider a common case: a program that was written before the concept of HiDPI displays existed and is being adapted to support HiDPI displays. In this case, because the image creation is potentially a long running process, the program is written to use a background thread to create the image. Once the image is created, it is stored somewhere that can be accessed by a painting method of a component. Prior to the image becoming available, the component will paint some default background or perhaps paint a default image.

I do not want to rearchitect the program to support HiDPI displays. Using the display scale information for the available displays, I could create a multiresolution image in the background thread that either contains multiple images for all available displays or contains one image at the maximum required resolution of the available displays. However, these solutions are non-optimal. Either images may be created that will never be used or image scaling may be used to reduce the resolution, which may result in a lower quality image.

What I would like to do is create a multiresolution image that tells my code what resolution image is needed and allows me to create that image on a background thread. In additional to solving the HiDPI problem optimally, this solution would support less common cases, such as displays added to the system at run time and graphics contexts that are scaled for some reason other than supporting a HiDPI display.

I am assuming this capability already exists to support toolkit images. It would be nice to make that capability available to applications.

  Alan


On Jul 15, 2015, at 4:07 AM, Alexander Scherbatiy <[hidden email]> wrote:

On 7/14/2015 3:18 AM, Alan Snyder wrote:
I have a concern about how custom multiresolution images are supported based on a problem I ran into using the JDK 8 classes, which are similar.

The problem is that the selection of a variant image happens during painting. That is a very bad time to actually try to render an image, if one is in a situation where images of arbitrary resolution can be produced by rendering.

If I have code now that uses a background thread to create an image in this manner, how can I support HiDPI displays? Creating a multiresolution image defeats the purpose of using a background thread if it defers rendering until paint time. It seems that the best I can do is to guess what resolutions might be wanted and generate them all in advance or, alternatively, generate the highest resolution I expect to need, and have the graphics system scale it down for lower resolutions when requested. Neither of these alternatives is optimal.
  JDK 9 has the fix that includes the scale factor in the GraphicsConfiguration default transform: http://hg.openjdk.java.net/jdk9/client/jdk/rev/661136704d07

  It is possible to iterate over all graphics devices, decide on which device the image is supposed to be rendered and generate an image resolution according to the scale factor from the device graphics configuration.

 Thanks,
 Alexandr.


Although the Image class does allow images to be produced in the background, that API is way too complex for use by an application, and the supporting code for Toolkit images is not accessible.

It would be nice to have a simple way to return an image that is produced in the background. Something like Future<Image>?

  Alan



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: <AWT Dev> Review request for 8029339 Custom MultiResolution image support on HiDPI displays

Alexandr Scherbatiy
On 7/15/2015 5:09 PM, Alan Snyder wrote:
>
> It seems you are making an assumption that the code that creates the
> image is somehow initiated from a paint method so that the display
> scale factor can be obtained from the graphics context and used to
> determine the required image resolution. That is not the scenario I am
> concerned about.
>
    The information about screen devices are available from the
GraphicsEnvironment.
    Something like:
   ----------------------
     public static void main(String[] args) {
         GraphicsEnvironment ge =
GraphicsEnvironment.getLocalGraphicsEnvironment();
         for(GraphicsDevice device: ge.getScreenDevices()){
             for(GraphicsConfiguration config: device.getConfigurations()){
                 if(!config.getDefaultTransform().isIdentity()){
                     System.out.println("Start high-resolution image
loading");
                 }
             }
         }
     }
   ----------------------

   Thanks,
   Alexandr.

>
>
> I am talking about what I would consider a common case: a program that
> was written before the concept of HiDPI displays existed and is being
> adapted to support HiDPI displays. In this case, because the image
> creation is potentially a long running process, the program is written
> to use a background thread to create the image. Once the image is
> created, it is stored somewhere that can be accessed by a painting
> method of a component. Prior to the image becoming available, the
> component will paint some default background or perhaps paint a
> default image.
>
> I do not want to rearchitect the program to support HiDPI displays.
> Using the display scale information for the available displays, I
> could create a multiresolution image in the background thread that
> either contains multiple images for all available displays or contains
> one image at the maximum required resolution of the available
> displays. However, these solutions are non-optimal. Either images may
> be created that will never be used or image scaling may be used to
> reduce the resolution, which may result in a lower quality image.
>
> What I would like to do is create a multiresolution image that tells
> my code what resolution image is needed and allows me to create that
> image on a background thread. In additional to solving the HiDPI
> problem optimally, this solution would support less common cases, such
> as displays added to the system at run time and graphics contexts that
> are scaled for some reason other than supporting a HiDPI display.
>
> I am assuming this capability already exists to support toolkit
> images. It would be nice to make that capability available to
> applications.
>
>   Alan
>
>
>> On Jul 15, 2015, at 4:07 AM, Alexander Scherbatiy
>> <[hidden email]
>> <mailto:[hidden email]>> wrote:
>>
>> On 7/14/2015 3:18 AM, Alan Snyder wrote:
>>> I have a concern about how custom multiresolution images are
>>> supported based on a problem I ran into using the JDK 8 classes,
>>> which are similar.
>>>
>>> The problem is that the selection of a variant image happens during
>>> painting. That is a very bad time to actually try to render an
>>> image, if one is in a situation where images of arbitrary resolution
>>> can be produced by rendering.
>>>
>>> If I have code now that uses a background thread to create an image
>>> in this manner, how can I support HiDPI displays? Creating a
>>> multiresolution image defeats the purpose of using a background
>>> thread if it defers rendering until paint time. It seems that the
>>> best I can do is to guess what resolutions might be wanted and
>>> generate them all in advance or, alternatively, generate the highest
>>> resolution I expect to need, and have the graphics system scale it
>>> down for lower resolutions when requested. Neither of these
>>> alternatives is optimal.
>>   JDK 9 has the fix that includes the scale factor in the
>> GraphicsConfiguration default transform:
>> http://hg.openjdk.java.net/jdk9/client/jdk/rev/661136704d07
>>
>>   It is possible to iterate over all graphics devices, decide on
>> which device the image is supposed to be rendered and generate an
>> image resolution according to the scale factor from the device
>> graphics configuration.
>>
>>  Thanks,
>>  Alexandr.
>>
>>>
>>> Although the Image class does allow images to be produced in the
>>> background, that API is way too complex for use by an application,
>>> and the supporting code for Toolkit images is not accessible.
>>>
>>> It would be nice to have a simple way to return an image that is
>>> produced in the background. Something like Future<Image>?
>>>
>>>   Alan
>>>
>>
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: <AWT Dev> Review request for 8029339 Custom MultiResolution image support on HiDPI displays

Alan Snyder-3
I am commenting on your suggestion that I can determine which display the image will displayed on.



On Jul 15, 2015, at 7:42 AM, Alexander Scherbatiy <[hidden email]> wrote:

On 7/15/2015 5:09 PM, Alan Snyder wrote:

It seems you are making an assumption that the code that creates the image is somehow initiated from a paint method so that the display scale factor can be obtained from the graphics context and used to determine the required image resolution. That is not the scenario I am concerned about.

  The information about screen devices are available from the GraphicsEnvironment.
  Something like:
 ----------------------
   public static void main(String[] args) {
       GraphicsEnvironment ge = GraphicsEnvironment.getLocalGraphicsEnvironment();
       for(GraphicsDevice device: ge.getScreenDevices()){
           for(GraphicsConfiguration config: device.getConfigurations()){
               if(!config.getDefaultTransform().isIdentity()){
                   System.out.println("Start high-resolution image loading");
               }
           }
       }
   }
 ----------------------

 Thanks,
 Alexandr.


I am talking about what I would consider a common case: a program that was written before the concept of HiDPI displays existed and is being adapted to support HiDPI displays. In this case, because the image creation is potentially a long running process, the program is written to use a background thread to create the image. Once the image is created, it is stored somewhere that can be accessed by a painting method of a component. Prior to the image becoming available, the component will paint some default background or perhaps paint a default image.

I do not want to rearchitect the program to support HiDPI displays. Using the display scale information for the available displays, I could create a multiresolution image in the background thread that either contains multiple images for all available displays or contains one image at the maximum required resolution of the available displays. However, these solutions are non-optimal. Either images may be created that will never be used or image scaling may be used to reduce the resolution, which may result in a lower quality image.

What I would like to do is create a multiresolution image that tells my code what resolution image is needed and allows me to create that image on a background thread. In additional to solving the HiDPI problem optimally, this solution would support less common cases, such as displays added to the system at run time and graphics contexts that are scaled for some reason other than supporting a HiDPI display.

I am assuming this capability already exists to support toolkit images. It would be nice to make that capability available to applications.

 Alan


On Jul 15, 2015, at 4:07 AM, Alexander Scherbatiy <[hidden email] <[hidden email]>> wrote:

On 7/14/2015 3:18 AM, Alan Snyder wrote:
I have a concern about how custom multiresolution images are supported based on a problem I ran into using the JDK 8 classes, which are similar.

The problem is that the selection of a variant image happens during painting. That is a very bad time to actually try to render an image, if one is in a situation where images of arbitrary resolution can be produced by rendering.

If I have code now that uses a background thread to create an image in this manner, how can I support HiDPI displays? Creating a multiresolution image defeats the purpose of using a background thread if it defers rendering until paint time. It seems that the best I can do is to guess what resolutions might be wanted and generate them all in advance or, alternatively, generate the highest resolution I expect to need, and have the graphics system scale it down for lower resolutions when requested. Neither of these alternatives is optimal.
 JDK 9 has the fix that includes the scale factor in the GraphicsConfiguration default transform: http://hg.openjdk.java.net/jdk9/client/jdk/rev/661136704d07

 It is possible to iterate over all graphics devices, decide on which device the image is supposed to be rendered and generate an image resolution according to the scale factor from the device graphics configuration.

Thanks,
Alexandr.


Although the Image class does allow images to be produced in the background, that API is way too complex for use by an application, and the supporting code for Toolkit images is not accessible.

It would be nice to have a simple way to return an image that is produced in the background. Something like Future<Image>?

 Alan

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Review request for 8029339 Custom MultiResolution image support on HiDPI displays

Alexandr Scherbatiy
In reply to this post by Jim Graham-5

   Hello Jim,

  Thank your for the review.

  Could you review the updated fix there the suggested comments are updated:
     http://cr.openjdk.java.net/~alexsch/8029339/webrev.09/


On 7/14/2015 2:36 AM, Jim Graham wrote:

> Hi Alexandr,
>
> Sorry that this fell into the cracks.  Here are some fairly minor
> updates:
>
> AbstractMRI - getGraphics() should include "getGraphics() not
> supported on Multi-Resolution Images" as the exception description text.
>
> AbstractMRI - getBaseImage() should have doc comments:
> /**
>  * Return the base image representing the best version of the image
>  * for rendering at the default width and height.
>  * @return the base image of the set of multi-resolution images
>  */
> (Does it need an @since given that the entire interface is @since 1.9?)
>
> BaseMRI - the doc comments (both the class comment and the constructor
> comments) don't start with "/**" so they aren't really doc comments,
> but they look good so make them so.
>
> BaseMRI - class comment - "The implementation will return the first
> ... satisfy the rendering request."  Add another sentence right there
> "The last image in the array will be returned if no suitable image is
> found that is larger than the rendering request."
>
> BaseMRI - move "For best effect ..." should be moved to a new
> paragraph and mention that no exception will be thrown if the images
> are not sorted as suggested.
>
> RenderingHints - comments:
> DEFAULT, SIZE_FIT, DPI_FIT - "an image resolution variant is chosen
> ..." (add "an")
> DEFAULT - "... which may depend on the policies of the platform"
> SIZE_FIT, DPI_FIT "... on the DPI of ..." (add "the")
>
> SunHints - descriptor texts:
> SIZE_FIT, DPI_FIT "based on the DPI" (add "the")
>
> MRCachedImage - remind me what this is used for?
     It is used by AquaLookAndFeel for images and icons  rendering
requested from the native system.
> MRCachedImage.getResolutionVariant - use ceil or round instead of
> (int) cast?  ceil() would match the actions of MRToolkitImage better.  
> Also note following comment about precision with SG2D.
>
> SG2D/MRI - the interface declares the values as float, the usage in
> SG2D computes values in double and then truncates them to int to pass
> to the interface - should we upgrade the interface to double for
> completeness?  If not, then the usage in SG2D should at least pass in
> float precision.
    I changed the argument types to double in the
MRI.getResolutionVariant(...).

>
> SG2D.getResolutionVariant - using destRegionWH to calculate
> destImageWH can involve a lot of "do some math and then later have
> additional code undo it".  Would it be faster to just compute
> destImageWH directly, as in:
>
> float destImageWidth, destImageHeight;
>
> if (BASE) {
>     destImageWH = srcWH;
> } else if (DPI) {
>     destImageWH = (float) abs(srcWH * devScale);
> } else {
>     double destRegionWidth, destRegionHeight;
>     if (type) {
>     ...
>     }
>     destImageWH = (float) abs(srcWH * destRegionWH / swh);
> }
> Image rv = img.getRV(destImageWH);
>
> On the other hand, the last "else" is probably the most common case so
> this doesn't save anything in the common case, but it avoids a bunch
> of math that could introduce rounding error for the BASE/DPI cases
> (even though it is done in doubles).
>
> Is there intent to have the default case be mapped to DPI_FIT for
> Windows?
     I think yes.
>
> MultiResolutionRenderinHints.java - should have "Test" appended to the
> name
>
> MRRHTest - why not render to a BufferedImage and avoid Robot? Could it
> tap into internal APIs to create a BImg/compatibleImage with a given
> devScale?

    The DPI_FIT test includes case there a graphics transform is
identity but the device configuration transform has scale 2.
    There should be a way to set scale for the device configuration
transform;

>
> MRRHTest - why allow up to delta=50 on the comparison?
     It is just for monitors that are calibrated on Mac OS X and they
colors are different from that which was set. The test uses colors which
have at least one color component differ by 255.

>
> MRRHTest - since the scale factor comes from a dialog, we depend on
> running this on a HiDPI display to fully test the hints?  Could using
> "scaled compatible images" allow testing this more definitively?

    Could you explain more what does it mean "scaled compatible images"?

   Thanks,
   Alexandr.

>
> MRRHTest - remove println before pushing?
>
> MRRHTest - probably shouldn't have a "right answer" for DEFAULT - it
> should probably just pass if the operation doesn't throw an exception?
>
>             ...jim
>
>
> On 4/15/15 7:04 AM, Alexander Scherbatiy wrote:
>>
>>    Hello,
>>
>>   Could you review the updated fix:
>>     http://cr.openjdk.java.net/~alexsch/8029339/webrev.08/
>>
>>   - SunGraphics2D is updated to calculate the resolution variant size
>> according to the _BASE, _DPI_FIT, and _SIZE_FIT resolution rendering
>> hint
>>   - MultiResolutionRenderingHints test is added
>>
>>   Thanks,
>>   Alexandr.
>>
>>
>> On 4/7/2015 1:02 PM, Jim Graham wrote:
>>> This is an interesting suggestion that would let us keep the
>>> implementation of the various hints centralized and simplify the
>>> interface to the part of the mechanism that is most tied in to the
>>> implementation specifics of the database of media variants - which is
>>> housed in the MRI-implementing object.
>>>
>>> I'm not sure we ever identified any need to customize the logic of
>>> "what size is needed for this render operation" beyond what we
>>> expressed in the hints, and given that the platform defaults may not
>>> be easy to express to an arbitrary implementation, it is probably
>>> better to put that part of the logic in SG2D, controlled by the easy
>>> to express hints and keep the current API both simple to implement and
>>> flexible to use.  Even if there was a need to customize that part of
>>> the operation (the "what size is relevant to this rendering operation"
>>> decision) beyond what the hints suggest, it would be inappropriate to
>>> tie that in to the "find me media" aspect of the MRI interface
>>> anyway.  So, best to keep them separate and have the hints affect what
>>> SG2D does and have the MRI focused on just storing (possibly creating)
>>> and managing/finding the variants.
>>>
>>>             ...jim
>>>
>>> On 4/1/15 6:46 AM, Alexander Scherbatiy wrote:
>>>> On 3/27/2015 12:48 AM, Jim Graham wrote:
>>>>> Hi Alexander,
>>>>
>>>>    http://cr.openjdk.java.net/~alexsch/8029339/webrev.07/
>>>>    I have updated the fix according to the comments except
>>>> RenderingHints.
>>>>
>>>>    RenderingHints are supposed to be set on Graphics2D and they have
>>>> keys/values which are not relevant to the getResolutionVariant()
>>>> method.
>>>>    Graphics objects chooses an alternative according to the set
>>>> rendering hints.
>>>>    May be what SG2D should do just calculate dest image size for the
>>>> given resolution variant hint (_BASE - base image size, _SIZE_FIT -
>>>> including scale factor and graphics transform (Mac algorithm),
>>>> _DPI_FIT - scaled according to the system DPI) and then pass them to
>>>> the getResolutionVariant() method?
>>>>
>>>>   Thanks,
>>>>   Alexandr.
>>>>
>>>>>
>>>>> MRI.java:
>>>>>
>>>>> Who is the intended audience for the recommendation in the interface
>>>>> to cache image variants?  Are we asking the developers who call the
>>>>> methods on the interface to cache the answers? That would be unwise
>>>>> because the list of variants may change over time for some MRIs. Are
>>>>> we speaking to the implementer? If so, then I think that it is
>>>>> unnecessary to remind implementers of basic implementation
>>>>> strategies like "cache complicated objects".
>>>>>
>>>>> How about this wording for the getRV() method? - "Gets a specific
>>>>> image that is the best variant to represent this logical image at
>>>>> the indicated size and screen resolution."
>>>>>
>>>>> Perhaps we should clarify in the doc comments for the dimension
>>>>> arguments in getRV() that the dimensions are measured in pixels?
>>>>>
>>>>> line 56 - delete blank line between doc comment and method
>>>>> declaration
>>>>>
>>>>> Also, it is probably overkill to document that the interface
>>>>> getVariants() method returns an unmodifiable list. Responsible
>>>>> implementations would probably use an unmodifiable list, but I don't
>>>>> think it should be required by the interface.  We do need to specify
>>>>> that it isn't required to be modifiable so that a caller doesn't
>>>>> expect to be able to modify it, but that is a looser spec.  How
>>>>> about "Gets a readable list of all resolution variants. Note that
>>>>> many implementations might return an unmodifiable list."?
>>>>>
>>>>> AbstractMIR.java:
>>>>>
>>>>> "provides a default implementation for the MRI" or "provides default
>>>>> implementations of several methods in the <MRI> interface and the
>>>>> <Image> class"?  Actually, I'll note that it provides none of the
>>>>> implementations of the MRI methods so maybe it should be "provides
>>>>> default implementations of several <Image> methods for classes that
>>>>> want to implement the <MRI> interface"?
>>>>>
>>>>> In the doc example - wrap the list to make it unmodifiable
>>>>>
>>>>> The doc example could be made 2 or 3 lines shorter by simply
>>>>> assuming the base image is in index 0 (it's just an example so I'm
>>>>> not sure the flexibility needs to be a part of the example).
>>>>>
>>>>> getGraphics is allowed by the Image interface to return null.
>>>>> Actually, the exact wording is that it can only be called for
>>>>> "offscreen images" and a MRI is technically not "an offscreen
>>>>> image".  Perhaps we should return null here since modifying the base
>>>>> image is unlikely to modify the variants and arguably it would be
>>>>> required by the wording in the method docs (even if the base image
>>>>> was an offscreen, the MRI produced from it stops being an offscreen)?
>>>>>
>>>>> Are all of the empty "@Inherit" comments needed?  I thought
>>>>> inheritance was the default?
>>>>>
>>>>> BaseMRI.java:
>>>>>
>>>>> "This class is [an] array-based implementation of the {AMRI} class"
>>>>> (missing "an" and "the")
>>>>>
>>>>> We should probably include the comment about the sorting of the
>>>>> images in the class comments as well as document that the algorithm
>>>>> is a simple scan from the beginning for the first image large enough
>>>>> (and default == last image).  The algorithm also might not work very
>>>>> well if the images are not monotonically both wider and taller.  How
>>>>> about adding this to the class comments:
>>>>>
>>>>> "The implementation will return the first image variant in the array
>>>>> that is large enough to satisfy the rendering request. For best
>>>>> effect the array of images should be sorted with each image being
>>>>> both wider and taller than the previous image. The base image need
>>>>> not be the first image in the array."
>>>>>
>>>>> getVariants() - you need to return an unmodifiable list. asList()
>>>>> returns a list that "writes back" to the array.  You need to use
>>>>> Collections.unmodifiableList(Arrays.asList(array)).
>>>>>
>>>>> RenderingHints.java:
>>>>>
>>>>> Where do we process this hint?  Don't we need to pass it to the
>>>>> getVariant() method?
>>>>>
>>>>> I don't understand the hint values other than the one that always
>>>>> uses the base image.  Default I guess gives us the ability to use
>>>>> the Mac algorithm of "next larger size" on Mac and "based on Screen
>>>>> DPI" on Windows, but the 3rd value "ON" is so vague as to not have
>>>>> any meaning.  Perhaps we should just delete it, but then do we just
>>>>> always do the Mac method? Or do we vaguely have our internal images
>>>>> have a platform-specific method and the Abstract/Base classes just
>>>>> do what they want with no control from the user?  In FX we are also
>>>>> still struggling with this issue and we may likely just do what the
>>>>> Mac does in all cases, but perhaps AWT needs to "behave like the
>>>>> platform" more?  If we are going to have actual values, then we need
>>>>> to have them do something, which means:
>>>>>
>>>>> - SG2D needs to track the hint just like we do the other hints that
>>>>> affect our processing
>>>>> - MRI.getVariant() needs to have the hint as an argument
>>>>> - BaseMRI should probably do something with that hint
>>>>> - hint values should include:
>>>>>     - ..._DEFAULT - implementation gets to decide
>>>>>     - ..._BASE - always use the base image
>>>>>     - ..._SIZE_FIT - Mac algorithm of smallest image that is big
>>>>> enough
>>>>>     - ..._DPI_FIT - choose based on DPI of the screen, ignoring
>>>>> transform
>>>>>
>>>>> line 978 - missing blank line between fields
>>>>>
>>>>> SG2D.java:
>>>>>
>>>>> - The interface says that you will be passing in the "logical DPI"
>>>>> of the display, but here you are actually passing in the screen's
>>>>> scale factor.
>>>>>
>>>>> BaseMRITest.java:
>>>>>
>>>>> - testBaseMRI also passes in a scale rather than a DPI to the MRI
>>>>> method.
>>>>>
>>>>> - How does the modification test pass when the implementation
>>>>> doesn't use unmodifiable lists?
>>>>>
>>>>> MRITest.java:
>>>>>
>>>>> - also uses scale rather than DPI in several places
>>>>>
>>>>>         ...jim
>>>>>
>>>>> On 3/13/15 6:34 AM, Alexander Scherbatiy wrote:
>>>>>>
>>>>>>    Hello,
>>>>>>
>>>>>>    Could you review the proposed API based on MultiresolutionImage
>>>>>> interface:
>>>>>> http://cr.openjdk.java.net/~alexsch/8029339/webrev.06/
>>>>>>
>>>>>>   - return unmodifiable list comment is added to the
>>>>>> getResolutionVariants() method javadoc in MultiresolutionImage
>>>>>> interface
>>>>>>   - base image size arguments are removed form the
>>>>>> getResolutionVariant(...) method in MultiresolutionImage interface
>>>>>>   - BaseMultiResolutionImage class that allows to create a
>>>>>> multi-resolution image based on resolution variant array is added
>>>>>>   - the test for the BaseMultiResolutionImage is added
>>>>>>
>>>>>>   Thanks,
>>>>>>   Alexandr.
>>>>>>
>>>>>> On 2/14/2015 3:23 AM, Jim Graham wrote:
>>>>>>> The second solution looks good. I'd make it standard practice
>>>>>>> (perhaps even mentioned in the documentation) to return
>>>>>>> unmodifiable
>>>>>>> lists from the getVariants() method.  The Collections class
>>>>>>> provides
>>>>>>> easy methods to create these lists, and it sends a clear message to
>>>>>>> the caller that the list was provided for them to read, but not
>>>>>>> write
>>>>>>> to.  Otherwise they may add a new image to the list you provided
>>>>>>> them
>>>>>>> and wonder why it wasn't showing up.  Also, an unmodifiable list
>>>>>>> can
>>>>>>> be cached and reused for subsequent calls without having to
>>>>>>> create a
>>>>>>> new list every time.
>>>>>>>
>>>>>>> In getResolutionVariant() was there a reason why the base
>>>>>>> dimensions
>>>>>>> were provided as float?  The destination dimensions make sense as
>>>>>>> float since they could be the result of a scale, but the source
>>>>>>> dimensions are typically getWidth/getHeight() on the base image.  A
>>>>>>> related question would be if they are needed at all, since the
>>>>>>> implementation should probably already be aware of what the base
>>>>>>> image
>>>>>>> is and what its dimensions are.  I'm guessing they are provided
>>>>>>> because the implementation in SG2D already knows them and it
>>>>>>> makes it
>>>>>>> easier to forward the implementation on to a shared (static?)
>>>>>>> method?
>>>>>>>
>>>>>>> With respect to default implementations, I take it that the
>>>>>>> BaseMRI is
>>>>>>> along the pattern that we see in Swing for Base classes. Would
>>>>>>> it be
>>>>>>> helpful to provide an implementation (in addition or instead) that
>>>>>>> allows a developer to take a bunch of images and quickly make an
>>>>>>> MRI
>>>>>>> without having to override anything?  The implementations of
>>>>>>> getBaseImage() and getResolutionVariants() are pretty
>>>>>>> straightforward
>>>>>>> and a fairly reasonable default algorithm can be provided for
>>>>>>> getRV(dimensions).  This question is more of an idle question
>>>>>>> for my
>>>>>>> own curiosity than a stumbling block...
>>>>>>>
>>>>>>>             ...jim
>>>>>>>
>>>>>>> On 1/22/2015 6:49 AM, Alexander Scherbatiy wrote:
>>>>>>>>
>>>>>>>>    Hi Phil,
>>>>>>>>
>>>>>>>>    I have prepared two versions of the proposed API:
>>>>>>>>
>>>>>>>>    I) Resolution variants are added directly to the Image:
>>>>>>>> http://cr.openjdk.java.net/~alexsch/8029339/list/webrev.00
>>>>>>>>
>>>>>>>>    II)  MultiResolutionImage interface is used:
>>>>>>>> http://cr.openjdk.java.net/~alexsch/8029339/webrev.05
>>>>>>>>
>>>>>>>>    It could help to decide which way should be chosen for the the
>>>>>>>> multi-resolution image support.
>>>>>>>>
>>>>>>>>    Below are some comments:
>>>>>>>>
>>>>>>>>    1. High level goal:
>>>>>>>>       Introduce an API that allows to create and handle an image
>>>>>>>> with
>>>>>>>> resolution variants.
>>>>>>>>
>>>>>>>>    2. What is not subject of the provided API
>>>>>>>>      - Scale naming convention for high-resolution images
>>>>>>>>      - Providing pixel scale factor for the screen/window
>>>>>>>>
>>>>>>>>    3. Use cases
>>>>>>>>     3.1 Loading and drawing high-resolution icons in IntelliJ IDEA
>>>>>>>>       A high-resolution image is loaded from resources and
>>>>>>>> stored in
>>>>>>>> JBHiDPIScaledImage class  which is a subclass of the buffered
>>>>>>>> image.
>>>>>>>>       The high-resolution image is used to create a disabled icon
>>>>>>>> in the
>>>>>>>> IconLoader.getDisabledIcon(icon) method.
>>>>>>>> https://github.com/JetBrains/intellij-community/blob/master/platform/util/src/com/intellij/openapi/util/IconLoader.java 
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>     3.2 Loading and drawing high-resolution icons in NetBeans
>>>>>>>>       NetBeans does not have support for the high-resolution icons
>>>>>>>> loading.
>>>>>>>>       It loads an icon from the file system using
>>>>>>>> Toolkit.getDefaultToolkit().getImage(url) method or from resources
>>>>>>>>       by  ImageReader  and store it in ToolTipImage class which is
>>>>>>>> subclass of the buffered image.
>>>>>>>>       ImageUtilities.createDisabledIcon(icon) method creates a
>>>>>>>> disabled
>>>>>>>> icon by applying  RGBImageFilter to the icon.
>>>>>>>> http://hg.netbeans.org/main/file/97dcf49eb4a7/openide.util/src/org/openide/util/ImageUtilities.java 
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>     3.3 Loading system icons in JDK 1.8
>>>>>>>>       JDK requests icons from the native system for system L&Fs
>>>>>>>> and
>>>>>>>> applies filters for them.
>>>>>>>>       See for example AquaUtils.generateLightenedImage() method:
>>>>>>>> http://hg.openjdk.java.net/jdk9/client/jdk/file/e6f48c4fad38/src/java.desktop/macosx/classes/com/apple/laf/AquaUtils.java 
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>    4. HiDPI support for Images on different OSes
>>>>>>>>
>>>>>>>>      4.1 Mac OS X
>>>>>>>>        Cocoa API contains NSImage that allows to work with image
>>>>>>>> representations: add/remove/get all representations.
>>>>>>>>        It picks up an image with necessary resolution based on the
>>>>>>>> screen backing store pixel scale factor and applied transforms.
>>>>>>>> https://developer.apple.com/library/mac/documentation/Cocoa/Reference/ApplicationKit/Classes/NSImage_Class/Reference/Reference.html 
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>      4.2 Linux
>>>>>>>>        GTK+ 3 API has gtkcssimagescaled lib (it seems that it
>>>>>>>> is not
>>>>>>>> public/stable)
>>>>>>>>        that parses the -gtk-scaled css property and draws a
>>>>>>>> GtkCssImage
>>>>>>>> according to the given scale factor.
>>>>>>>>
>>>>>>>>        I have not found information about the HiDPI support in
>>>>>>>> Xlib.
>>>>>>>>
>>>>>>>>      4.3 Windows
>>>>>>>>        I have only found the tutorial that suggests to select and
>>>>>>>> draw a
>>>>>>>> bitmap using the queried DPI
>>>>>>>>        and scale the coordinates for drawing a rectangular frame
>>>>>>>> http://msdn.microsoft.com/en-us/library/dd464659%28v=vs.85%29.aspx
>>>>>>>>
>>>>>>>>        Windows also provides the horizontal and vertical DPI of
>>>>>>>> the
>>>>>>>> desktop
>>>>>>>> http://msdn.microsoft.com/en-us/library/windows/apps/dd371316
>>>>>>>>
>>>>>>>>    5. Pseudo API
>>>>>>>>       Below are some ways which illustrates how multi-resolution
>>>>>>>> images
>>>>>>>> can be created and used.
>>>>>>>>
>>>>>>>>      5.1 Resolution variants are stored directly in Image class.
>>>>>>>>      To query a resolution variant it needs to compare the
>>>>>>>> resolution
>>>>>>>> variant width/height
>>>>>>>>      with the requested high-resolution size.
>>>>>>>>      ------------
>>>>>>>>      public abstract class Image {
>>>>>>>>
>>>>>>>>          public void addResolutionVariant(Image image) {...}
>>>>>>>>          public List<Image> getResolutionVariants() {...}
>>>>>>>>      }
>>>>>>>>      ------------
>>>>>>>>      // create a disabled image with resolution variants
>>>>>>>>
>>>>>>>>      Image disabledImage = getDisabledImage(image);
>>>>>>>>
>>>>>>>>      for (Image rv : image.getResolutionVariants()) {
>>>>>>>> disabledImage.addResolutionVariant(getDisabledImage(rv));
>>>>>>>>      }
>>>>>>>>      ------------
>>>>>>>>      This approach requires that all resolution variants have been
>>>>>>>> created even not of them are really used.
>>>>>>>>
>>>>>>>>      5.2  Resolution variants are stored in a separate object that
>>>>>>>> allows to create them by demand.
>>>>>>>>      To query a resolution variant it needs to compare the
>>>>>>>> resolution
>>>>>>>> variant scale factor
>>>>>>>>      with the requested scale (that can include both screen DPI
>>>>>>>> scale
>>>>>>>> and applied transforms).
>>>>>>>>      ------------
>>>>>>>>      public abstract class Image {
>>>>>>>>
>>>>>>>>          public static interface ResolutionVariant {
>>>>>>>>              Image getImage();
>>>>>>>>              float getScaleFactor();
>>>>>>>>          }
>>>>>>>>
>>>>>>>>          public void addResolutionVariant(ResolutionVariant
>>>>>>>> resolutionVariant) {...}
>>>>>>>>          public List<ResolutionVariant> getResolutionVariants()
>>>>>>>> {...}
>>>>>>>>      }
>>>>>>>>      ------------
>>>>>>>>      // create a disabled image with resolution variants
>>>>>>>>      Image disabledImage = getDisabledImage(image);
>>>>>>>>
>>>>>>>>      for (Image.ResolutionVariant rv :
>>>>>>>> image.getResolutionVariants()) {
>>>>>>>>          disabledImage.addResolutionVariant(new
>>>>>>>> Image.ResolutionVariant() {
>>>>>>>>
>>>>>>>>              public Image getImage() {
>>>>>>>>                  return getDisabledImage(rv.getImage());
>>>>>>>>              }
>>>>>>>>
>>>>>>>>              public float getScaleFactor() {
>>>>>>>>                  return rv.getScaleFactor();
>>>>>>>>              }
>>>>>>>>          });
>>>>>>>>      }
>>>>>>>>      ------------
>>>>>>>>
>>>>>>>>      It does not have problem if a predefined set of images is
>>>>>>>> provided
>>>>>>>> (like image.png and [hidden email] on the file system).
>>>>>>>>      This does not cover cases where a resolution variant can be
>>>>>>>> created
>>>>>>>> using the exact requested size (like loading icons from the native
>>>>>>>> system).
>>>>>>>>      A resolution variant can be queried based on a scale
>>>>>>>> factor and
>>>>>>>> applied transforms.
>>>>>>>>
>>>>>>>>      5.3 The provided example allows to create a resolution
>>>>>>>> variant
>>>>>>>> using the requested high-resolution image size.
>>>>>>>>      ------------
>>>>>>>>      public interface MultiResolutionImage {
>>>>>>>>          Image getResolutionVariant(float width, float height);
>>>>>>>>      }
>>>>>>>>      ------------
>>>>>>>>      // create a multi-resolution image
>>>>>>>>      Image mrImage = new AbstractMultiResolutionImage() {
>>>>>>>>
>>>>>>>>              public Image getResolutionVariant(float width, float
>>>>>>>> height) {
>>>>>>>>                  // create and return a resolution variant with
>>>>>>>> exact
>>>>>>>> requested width/height size
>>>>>>>>              }
>>>>>>>>
>>>>>>>>              protected Image getBaseImage() {
>>>>>>>>                  return baseImage;
>>>>>>>>              }
>>>>>>>>          };
>>>>>>>>      ------------
>>>>>>>>      // create a disabled image with resolution variants
>>>>>>>>      Image disabledImage = null;
>>>>>>>>      if (image instanceof MultiResolutionImage) {
>>>>>>>>          final MultiResolutionImage mrImage =
>>>>>>>> (MultiResolutionImage)
>>>>>>>> image;
>>>>>>>>          disabledImage = new AbstractMultiResolutionImage(){
>>>>>>>>
>>>>>>>>              public Image getResolutionVariant(float width, float
>>>>>>>> height) {
>>>>>>>>                  return
>>>>>>>> getDisabledImage(mrImage.getResolutionVariant(width, height));
>>>>>>>>              }
>>>>>>>>
>>>>>>>>              protected Image getBaseImage() {
>>>>>>>>                  return getDisabledImage(mrImage);
>>>>>>>>              }
>>>>>>>>          };
>>>>>>>>      } else {
>>>>>>>>          disabledImage = getDisabledImage(image);
>>>>>>>>      }
>>>>>>>>      ------------
>>>>>>>>
>>>>>>>>    Thanks,
>>>>>>>>    Alexandr.
>>>>>>
>>>>
>>>
>>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Review request for 8029339 Custom MultiResolution image support on HiDPI displays

Jim Graham-5
Hi Alexandr,

On 7/21/15 7:41 AM, Alexander Scherbatiy wrote:
>
>    Hello Jim,
>
>   Thank your for the review.
>
>   Could you review the updated fix there the suggested comments are
> updated:
>      http://cr.openjdk.java.net/~alexsch/8029339/webrev.09/

Minor comments embedded below.

> On 7/14/2015 2:36 AM, Jim Graham wrote:
>> AbstractMRI - getGraphics() should include "getGraphics() not
>> supported on Multi-Resolution Images" as the exception description text.

AbstractMRI - the getGraphics() description string contains an embedded
'\n' and an embedded '\"', are those typos?

>> BaseMRI - class comment - "The implementation will return the first
>> ... satisfy the rendering request."  Add another sentence right there
>> "The last image in the array will be returned if no suitable image is
>> found that is larger than the rendering request."

BaseMRI - whoops, my bad.  "that is larger than" should probably be
"that is as large as" to match the <= comparison

>> SG2D.getResolutionVariant - using destRegionWH to calculate
>> destImageWH can involve a lot of "do some math and then later have
>> additional code undo it".  Would it be faster to just compute
>> destImageWH directly, as in:
>>
>> float destImageWidth, destImageHeight;
>>
>> if (BASE) {
>>     destImageWH = srcWH;
>> } else if (DPI) {
>>     destImageWH = (float) abs(srcWH * devScale);
>> } else {
>>     double destRegionWidth, destRegionHeight;
>>     if (type) {
>>     ...
>>     }
>>     destImageWH = (float) abs(srcWH * destRegionWH / swh);
>> }
>> Image rv = img.getRV(destImageWH);

For the BASE and DPI_FIT cases shouldn't you use srcWidth,srcHeight
instead of sw,sh?  The sw,sh are affected by sub-image parameters, but
srcWidth,srcHeight are literally the size of the original image, which
is what you want to search the MRI for.  The sw,sh should only be used
to scale the srcWidth in the "else" clause - as in "srcWidth *
destRegionWidth / sw".

>> Is there intent to have the default case be mapped to DPI_FIT for
>> Windows?
>      I think yes.

I didn't see where this was being done - is that a TBD follow-on fix or
did I miss a default somewhere?

>> MRRHTest - why not render to a BufferedImage and avoid Robot? Could it
>> tap into internal APIs to create a BImg/compatibleImage with a given
>> devScale?
>
>     The DPI_FIT test includes case there a graphics transform is
> identity but the device configuration transform has scale 2.
>     There should be a way to set scale for the device configuration
> transform;

Ah yes, DPI_FIT needs a default transform.  Also, without a way to
manually create a device with a transform, that means that DPI_FIT is
only successfully tested if the tests are run on a HiDPI machine, right?

>> MRRHTest - why allow up to delta=50 on the comparison?
>      It is just for monitors that are calibrated on Mac OS X and they
> colors are different from that which was set. The test uses colors which
> have at least one color component differ by 255.

Another issue that might go away if we could use BImg instead of robot.

>> MRRHTest - since the scale factor comes from a dialog, we depend on
>> running this on a HiDPI display to fully test the hints?  Could using
>> "scaled compatible images" allow testing this more definitively?
>
>     Could you explain more what does it mean "scaled compatible images"?

I seem to recall that there is a mechanism in there to create backbuffer
images for swing that record the fact that they are scaled.  I forget
how this is done, but I'm wondering if it could be used to run all of
this test code in a simulated scale environment rather than using the
actual configurations that AWT creates for the current system.

                        ...jim
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Review request for 8029339 Custom MultiResolution image support on HiDPI displays

Alexandr Scherbatiy

  Hello Jim,

  Could you review the updated fix:
http://cr.openjdk.java.net/~alexsch/8029339/webrev.10/
   - the suggested comments are updated
   - the MultiResolutionRenderingHintsTest is updated to use custom
GraphicsConfiguration and SurfaceData

On 7/23/2015 8:25 PM, Jim Graham wrote:

> Hi Alexandr,
>
> On 7/21/15 7:41 AM, Alexander Scherbatiy wrote:
>>
>>    Hello Jim,
>>
>>   Thank your for the review.
>>
>>   Could you review the updated fix there the suggested comments are
>> updated:
>>      http://cr.openjdk.java.net/~alexsch/8029339/webrev.09/
>
> Minor comments embedded below.
>
>> On 7/14/2015 2:36 AM, Jim Graham wrote:
>>> AbstractMRI - getGraphics() should include "getGraphics() not
>>> supported on Multi-Resolution Images" as the exception description
>>> text.
>
> AbstractMRI - the getGraphics() description string contains an
> embedded '\n' and an embedded '\"', are those typos?
    Updated.
>
>>> BaseMRI - class comment - "The implementation will return the first
>>> ... satisfy the rendering request."  Add another sentence right there
>>> "The last image in the array will be returned if no suitable image is
>>> found that is larger than the rendering request."
>
> BaseMRI - whoops, my bad.  "that is larger than" should probably be
> "that is as large as" to match the <= comparison
     Updated.

>
>>> SG2D.getResolutionVariant - using destRegionWH to calculate
>>> destImageWH can involve a lot of "do some math and then later have
>>> additional code undo it".  Would it be faster to just compute
>>> destImageWH directly, as in:
>>>
>>> float destImageWidth, destImageHeight;
>>>
>>> if (BASE) {
>>>     destImageWH = srcWH;
>>> } else if (DPI) {
>>>     destImageWH = (float) abs(srcWH * devScale);
>>> } else {
>>>     double destRegionWidth, destRegionHeight;
>>>     if (type) {
>>>     ...
>>>     }
>>>     destImageWH = (float) abs(srcWH * destRegionWH / swh);
>>> }
>>> Image rv = img.getRV(destImageWH);
>
> For the BASE and DPI_FIT cases shouldn't you use srcWidth,srcHeight
> instead of sw,sh?  The sw,sh are affected by sub-image parameters, but
> srcWidth,srcHeight are literally the size of the original image, which
> is what you want to search the MRI for.  The sw,sh should only be used
> to scale the srcWidth in the "else" clause - as in "srcWidth *
> destRegionWidth / sw".

    You are right. sw and sh are sizes of a region in the original
image. I have updated the fix to use srcWidth/Height.

>
>>> Is there intent to have the default case be mapped to DPI_FIT for
>>> Windows?
>>      I think yes.
>
> I didn't see where this was being done - is that a TBD follow-on fix
> or did I miss a default somewhere?
    I think that it could be fixed with the updating WToolkit to load
high-resolution toolkit images in the same way as it is done in LWCToolkit.
    At least it can be filled as a separate issue.

>
>>> MRRHTest - why not render to a BufferedImage and avoid Robot? Could it
>>> tap into internal APIs to create a BImg/compatibleImage with a given
>>> devScale?
>>
>>     The DPI_FIT test includes case there a graphics transform is
>> identity but the device configuration transform has scale 2.
>>     There should be a way to set scale for the device configuration
>> transform;
>
> Ah yes, DPI_FIT needs a default transform.  Also, without a way to
> manually create a device with a transform, that means that DPI_FIT is
> only successfully tested if the tests are run on a HiDPI machine, right?
>
>>> MRRHTest - why allow up to delta=50 on the comparison?
>>      It is just for monitors that are calibrated on Mac OS X and they
>> colors are different from that which was set. The test uses colors which
>> have at least one color component differ by 255.
>
> Another issue that might go away if we could use BImg instead of robot.
>
>>> MRRHTest - since the scale factor comes from a dialog, we depend on
>>> running this on a HiDPI display to fully test the hints? Could using
>>> "scaled compatible images" allow testing this more definitively?
>>
>>     Could you explain more what does it mean "scaled compatible images"?
>
> I seem to recall that there is a mechanism in there to create
> backbuffer images for swing that record the fact that they are
> scaled.  I forget how this is done, but I'm wondering if it could be
> used to run all of this test code in a simulated scale environment
> rather than using the actual configurations that AWT creates for the
> current system.

    I have updated the test to use custom GraphicsConfiguration and
SurfaceData. This allows to use custom scales both on the graphics
configuration and on graphics.

    Thanks,
    Alexandr.

>
>             ...jim

12
Loading...