Hi Phil,
I need a reviewer from the 2d group for the fix. Could you take a look at the fix and review it?
Thanks, Alexandr. On 5/12/2014 6:35 PM, Alexander Scherbatiy wrote:
There was a long thread about the image with sub-pixel resolution drawing on Mac OS X: http://mail.openjdk.java.net/pipermail/macosx-port-dev/2013-April/005559.htmlIt was pointed out that Icon images that can be programmatically generated also need to have HiDPI support: http://mail.openjdk.java.net/pipermail/macosx-port-dev/2013-April/005566.html http://mail.openjdk.java.net/pipermail/macosx-port-dev/2013-April/005569.htmlAll requests about Mac OS X HiDPI support were included to the umbrella issue:7124410 [macosx] Lion HiDPI support https://bugs.openjdk.java.net/browse/JDK-7124410 Thanks, Alexandr. On 4/25/2014 6:45 PM, Alexander Scherbatiy wrote:On 4/25/2014 2:17 AM, Jim Graham wrote:Hi Alexandr,I asked for who was requesting these facilities and you responded with the solution you are planning to provide.I don't care what the solution looks like if we have nobody asking for the feature - I am asking who is asking for these capabilities?This is the request from the AWT team for the HiDPI support. Thanks, Alexandr....jim On 4/4/14 4:53 AM, Alexander Scherbatiy wrote:On 4/3/2014 2:23 AM, Jim Graham wrote:Hi Alexandr, The back and forth is getting confusing here, so I thought I'd try to summarize and start fresh(ish): 1. We need to support @2x internally for MacOS compatibility (done).2. We will need to support _DPI images for Win-DPI compatibility (TBD).3. Customers may have their own collection of images to bundle together into an MR image (working on that here). What is the push for this? Is this simply parity with Mac interfaces?---------- Image[] resolutionVariants = // get sorted by sizes array of resolution variants; Image mrImage = Toolkit.getDefaultToolkit().createMRImage(baseImageIndex, resolutionVariants); ---------- Here is the proposed patch: http://cr.openjdk.java.net/~alexsch/8029339/webrev.04/4. Customers may want to synthetically generate images at arbitrary resolutions (a variation that is impacting this solution). What is the push for this?---------- Image mrImage =Toolkit.getDefaultToolkit().createMRImage(baseImageWidth, baseImageHeight,new float[][]{{100, 100}, {150, 150}, {200, 200}}, // resolution variants sizes (rvWidth, rvHeight) -> { /* generate a resolution variant */ }); ----------5. I'm guessing that customers might want to override the logic to choose from among multiple resolutions. That came from me based on seeing Mac and Win using different selection logic and our history of developers split between those wanting cross-platform consistency and those wanting consistency with native apps on each platform. Also, the needs of an animator may differ from the needs of a resolution-settable-document editor as to how dynamically the images shift between resolution variants.---------- Image[] resolutionVariants = // get sorted by sizes array of resolution variants; Image mrImage = ImageResolutionHelper.createMRImage( (rvWidth, rvHeight, resolutionVariants) -> { /*use acustom logic to choose a resolution variant from an array of images*/},(logicalDPI, baseImageSize, destImageSize) -> destImageSize, // calculate the custom aware resolution variant size baseImageIndex, resolutionVariants); ----------or just extend the CustomMultiResolutionImage which has Image as theparent class: -------------------- public class CustomMultiResolutionImage extends AbstractMultiResolutionImage { @Override public Image getResolutionVariant(float logicalDPIX, float logicalDPIY, float baseImageWidth, float baseImageHeight, float destImageWidth, float destImageHeight) {// return a resolution variant based on the given logical DPI,// base image size, or destination image size } @Override public List<Image> getResolutionVariants() { // return a list of resolution variants } @Override protected Image getBaseImage() { // return the base image } } --------------------Is that a fair summary of all of the considerations so far, or did I miss something?I think it should cover the main needs. Thanks, Alexandr....jim On 3/27/14 7:43 AM, Alexander Scherbatiy wrote:Below are some thoughts about TK.createMRImage(...) method On 3/24/2014 4:52 PM, Alexander Scherbatiy wrote:Hello, Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8029339/webrev.03/ - baseImageWidth/Height arguments are added to the getResolutionVariant(...) method - dest image sizes are reverted to included DPI scale- AbstractMultiResolutionImage is added. It needs only to implementonly 3 methods from the AbstractMultiResolutionImage class to create a custom multi-resolution image. For example: On 3/22/2014 3:57 AM, Jim Graham wrote:Your code example below can be expressed as an implementation of thesingle-method, lambda-compatible interface that expresses just the getRV() method. They could easily do: final Image baseImage = ...; TK.createMRImage(new RVInterface() { public Image getRV(...) { // calculate rvWidth and rvHeight // look up rvWidth/rvHeight in a database of images // possibly contruct a new image return rvImage; } }, baseImage);The RVInterface mixes the logic that construct an image and chooses the necessary resolution variant.It is ok if a developer always implements this interface. If itneeds to have DPI/Transform/Platform aware RVInterface the image construction logic should be separated.Does TK.createMRImage() method implies that Platform aware logicshould be used for a resolution-variant choosing? If so, may be general createMRImage() can be placed in the ImageResolutionHelper.The main issue I see is if you might want the newly constructed variants to appear in the List returned from the getVariants() method. I'm not sure what value that would have beyond simplyreturning the base media that the object uses from which to constructits variants...?It can be solved by using something like array of image sizes orother seeds and a mapper that can create an image from the given seed.It can look like: ------------------------- public class ImageResolutionHelper { public interface RVChooser { public Image getRV( float logicalDPIX, float logicalDPIY, float baseImageWidth, float baseImageHeight, float destImageWidth, float destImageHeight, final Image... resolutionVariants); } public static final RVChooser DPI_AWARE = ...; public static final RVChooser TRANSFORM_AWARE = ...;// resolutionVariants is an array of sorted by width/height imagesstatic Image createMRImage(final RVChooser rvChooser, final int baseImageIndex, final Image... resolutionVariants) { ... } // sorted by width/height images should be generated from seeds static <Type> Image createMRImage(final RVChooser rvChooser, final Type baseImageSeed, final Function<Type, Image> mapper, final Type... rvSeeds) {...} } public abstract class Toolkit {public abstract Image createMRImage(int baseImageIndex, Image...resolutionVariants); // Platform aware rv chooser is used public abstract RVChooser getPlatformRVChooser() ; } -------------------------- Thanks, Alexandr.I think it is better to provide both the MultiResolutionImageand its implementation based on the given resolution variants array.It occurs to me that even if we don't go with a lambda-factory-based approach like what I'm describing, it might make sense to provide a baseMR implementation that they can subclass to keep them from tryingto subclass off of BufferedImage instead. I really would like to avoid "custom MR images are subclasses of BufImg" if we can as I think the mix of concepts is a little jarring... ...jimThe implementation could look like: --------------------------------- public class CustomMultiResolutionImage extends Image implements MultiResolutionImage { int baseImageIndex; Image[] resolutionVariants; public CustomMultiResolutionImage(int baseImageIndex, Image... resolutionVariants) { this.baseImageIndex = baseImageIndex; this.resolutionVariants = resolutionVariants; } @Override public int getWidth(ImageObserver observer) { return getBaseImage().getWidth(null); } @Override public int getHeight(ImageObserver observer) { return getBaseImage().getHeight(null); } @Override public ImageProducer getSource() { return getBaseImage().getSource(); } @Override public Graphics getGraphics() { return getBaseImage().getGraphics(); } @Overridepublic Object getProperty(String name, ImageObserver observer) {return getBaseImage().getProperty(name, observer); } @Override public Image getResolutionVariant(float logicalDPIX, float logicalDPIY, float destinationImageWidth, float destinationImageHeight) { // calculate resolution variant width/height return getResolutionVariant(rvWidth, rvHeight); } @Override public List<Image> getResolutionVariants() { return Arrays.asList(resolutionVariants); } private Image getResolutionVariant(float rvWidth, float rvHeight) {// return a resolution variant based on the given width andheight } private Image getBaseImage() { return resolutionVariants[baseImageIndex]; } } --------------------------------- Thanks, Alexandr.Then we provide one of these from TK.get/createImage() when the platform detects @2x, or Win8-style variants.For custom images we provide TK.createMRImage(lambda getRV, Imagevariants...) and TK.createMRImage(Image variants...); Since the get<List> method is just bookkeeping, I don't see them needing to override it, so the getRV() method is really the only thing they might want to override, and we can tie into the new Lambdacapabilities by making a single-method interface for it that theysupply in a factory method. I realize that the interface you created is more fundamentally OO, butthe Image class has always been special in this regard in the AWT ecosystem (in so far as we do not support someone implementing theirown Image subclass even though it is technically possible). Because ofthis special nature of Image, we end up with the situation that if someone were given a need to create a subclass of Image, then theywould turn to BufImg as their superclass even though BufImg isessentially an implementation-specific leaf node on the Image classhierarchy. This approach with a factory method to create an internalsubclass of the new MRI class mirrors the existing cases of Imageobjects that come from factories as well. Thoughts? ...jim On 3/20/14 7:52 AM, Alexander Scherbatiy wrote:Hello, Could you review the updated version of the fix: http://cr.openjdk.java.net/~alexsch/8029339/webrev.01/- The "getResolutionVariant(int width, int height)" method fromMultiResolutionImage class is changed to Image getResolutionVariant(float logicalDPIX, float logicalDPIY, float width, float height, AffineTransform transform); - sun.awt.image.ImageResolutionHelper class is added. The sun.awt.image.MultiResolutionToolkitImage and sun.awt.image.MultiResolutionBufferedImage classes are used PLATFORM ImageResolutionHelper. The MultiResolutionImage interface implementation could look like: ------------------------------ public class CustomMultiResolutionImage extends BufferedImage implements MultiResolutionImage { private final Image[] resolutionVariants; public CustomMultiResolutionImage(int baseIndex, Image... images) { super(images[baseIndex].getWidth(null), images[baseIndex].getHeight(null), BufferedImage.TYPE_INT_RGB); this.resolutionVariants = images; Graphics g = getGraphics(); g.drawImage(images[baseIndex], 0, 0, null); g.dispose(); } @Override public Image getResolutionVariant(float logicalDPIX, float logicalDPIY, float width, float height, AffineTransform transform) { return getResolutionVariant(logicalDPIX * width, logicalDPIY * height); } @Override public List<Image> getResolutionVariants() { return Arrays.asList(resolutionVariants); } public Image getResolutionVariant(double width, double height) { for (Image image : resolutionVariants) { if (width <= image.getWidth(null) && height <= image.getHeight(null)) { return image; } } return this; } } ------------------------------ Thanks, Alexandr. On 2/27/2014 4:54 PM, Alexander Scherbatiy wrote:On 2/22/2014 3:54 AM, Jim Graham wrote:Hi Alexandr, On 2/18/14 7:33 AM, Alexander Scherbatiy wrote:Hi Jim, Let's divide the discussion into two part. 1. Where it is better to hold resolution variants? Putting resolution variants in Image class brings some questions like:- Some type of images do not need to have resolution variants - Should resolution variants have the same type as the baseimage? - getResolutionVariants() method can return copy of the original list so add/removeRV methods should be also added.There are pros and cons for placing resolution variants toImage class or to a separate intreface.I agree that this could be a separate interface. In my examples below I was just sticking them inside an "Image{}" to show wherethey lived in the set of involved objects, not a specific recommendation that they actually be new methods on the base class itself. I probably should have put a comment there about that.With respect to add/remove - that is assuming a need for manualconstruction of an image set, right? Forgive me if I'm forgettingsomething, but I seem to recall that manual Multi-Res images wasproposed as a way for developers to introduce @2x support themselves,but if we are internally managing @2x and -DPI variants for them, then I'm not sure if there is actual developer need to manuallyconstruct their own. Am I forgetting something?The NSImage has addRepresentation/removeRepresentation methods to work with image representations on Mac OS X. The java.awt.Image class should provide similar functionality to have the possibilities as Cocoa on HiDPI displays.2. Using scale factor/image sizes/scaled image sizes to retreive a resolution variant. May be it is better to have a structure that provide all necessary information to query the resolution variant: scale factor, draw area width/height, transformed area width/height? For example: --------------------- public interface MultiResolutionImage { interface DrawAreaInfo { float getScaleFactor(); float getAreaWidth(); float getAreaHeight(); float getTransformedAreaWidth(); float getTransformedAreaHeight(); } public Image getResolutionVariant(DrawAreaInfo drawAreaInfo) ; public List<Image> getResolutionVariants(); } ---------------------The problem with a constructor is that this is something that is (potentially) done on every drawImage() call, which means we are inviting GC into the equation. If we can come up with a simple"justa couple/3/4 numbers" way to embed that data into a method callargument list then we can make this lighter weight.What about simply having floating point (double) dimensions onthe rendered sizeThere should be a way to choose a resolution variant based on requested drawing size or transformed drawing size.At least a current transformation should be included too.There is the ID2D1Factory::GetDesktopDpi method which returnsplus a single floating point "logical DPI" for the screen?dpiX and dpiY. http://msdn.microsoft.com/en-us/library/windows/apps/dd371316 That means that logicalDPIX/Y can have different values. At least it is described in the http://msdn.microsoft.com/en-us/library/windows/apps/ff684173"To get the DPI setting, call the ID2D1Factory::GetDesktopDpi method. The DPI is returned as two floating-point values, one forthex-axis and one for the y-axis. In theory, these values can differ.Calculate a separate scaling factor for each axis." The getResolutionVariant method could look like: -------------------------------------- public Image getResolutionVariant(float logicalDPIX, float logicalDPIY, float widthX, float widthY, AffineTransform transform); --------------------------------------If the image is known (either passed as an argument or the method is called on the image), then it can provide the original WH.The MultiResolutionImage default implementation could allowto use different strategies like scale factor/transfom/OS basedto query a resolution variant. The OS based strategy can beused by default.For Mac policy, all we need is the transformed dimensions, whichcanbe passed in as FP for generality. For Windows policy, all weneedis logical DPI for the screen. What other information would we need, or would an algorithm like to use, that can't be computedfrom those 2 pieces?The aim is to provide a base class that can be used to create a MultiResolutionImage like:http://hg.openjdk.java.net/jdk9/client/jdk/diff/ae53ebce5fa3/src/share/classes/sun/awt/image/MultiResolutionBufferedImage.javaA developer should be able to implement a custom algorithm toquery a resolution variant.It can be done by overriding the getResolutionVariant image:----------------------- Image mrImage = new MultiResolutionBufferedImage(){ @Override public Image getResolutionVariant(...) { // Custom logic here } }; ----------------------- Or it can be done by using resolution variant choosers so a developer can implement custom resolution variant query: ----------------------- public class MultiResolutionBufferedImage implements MultiResolutionImage{ interface ResolutionVariantChooser{ Image getResolutionVariant(dpi, size,..., List<Image> resolutionVariants); } ResolutionVariantChooser TRANSFORM_BASED = null; ResolutionVariantChooser DPI_BASED = null; ResolutionVariantChooser rvChooser; @Override public Image getResolutionVariant(dpi, size,...,) { return rvChooser.getResolutionVariant(dpi, size,..., getResolutionVariants()); } } ----------------------- Thanks, Alexandr....jimThanks, Alexandr. On 2/13/2014 4:42 AM, Jim Graham wrote:On 2/12/14 5:59 AM, Alexander Scherbatiy wrote:On 2/8/2014 4:19 AM, Jim Graham wrote:The primary thing that I was concerned about was the presence ofintegers in the API when Windows uses non-integer multiplesIt would make sense to pass real numbers to the getResolutionVariant() method if the difference between resolution variants sizes is 1. It seems that it is not a common case.I was thinking of other API that is related to this, such asthe API that queries the scaling factor from a SurfaceManager. I seem toremember some integer return values in that, but Windows mighthavethe answer 1.4 or 1.8, depending on the screen scaling factorthat was determined from the UI. In terms of the getResolutionVariant() method here, those non-integerscreen scaling factors don't directly impact this API. But, wehavesome issues with the use of integers there from other sources:- That API assumes that the caller will determine the pixel size needed, but the actual media choice is determined with different techniques on Mac and Windows so this means that the caller will have to worry about platform conventions. Is that the right tradeoff? - The technique recommended for Mac involves computing the precise size desired using the current transform, which may be a floatingpoint value, so the integer values used in this API are alreadyapproximations and there is no documentation on how to generate the proper integer. In particular, the current code in SG2D naively uses a cast to integer to determine the values to supply which means atransformed size of W+0.5 will be truncated to W and the lowerresolution image will be used. Does that conform to Mac guidelines? Dothey require the truncated size to reach W+1 before the nextsize isused? Passing in float or double values would sidestep all ofthatsince then the comparisons would be done with full precision,but aslong as we can determine a "best practices compatible with allplatforms" rule on how to round to integers, then integers are OK there. - The Windows document you cite below suggests that the determinationshould be made by looking at the Screen DPI and choosing thenext higher media variant based on that screen DPI. They do not specifychoosing media based on the current transform as is done forMac. Ifwe stick with supplying values that are used to determine whichmediato use, then on Windows we should not take the transform intoaccount,but instead query the SurfaceManager for the scale factor andonlytransform by those values (even if the current transform wasmanually overridden to identity). There are pros and cons to both approaches.Mac ensures that you are always using the best media for anygiven render operation. But, Windows ensure more consistency in the face of other scaling. The thing to consider is that if you have a 500x500 image with a 1000x1000 variant and you rendering it at 500x500 and then 501x501,that first jump will be fairly jarring as the scaled versionof the 1000x1000 will not look precisely like the original 500x500 did. With@2x images only, this effect is minimized so the advantage ofalways using "the best media for a given render operation" may outweigh the inconsistency issue. But, on Windows where the media are 1.4x or 1.8x in size, a downscaled image will start to show more interpolation noise and so the balance of the two choices may shift more towards not wanting a jarring shift. We might want one or more of the following: - Developer chooses policy (TX_AWARE, DPI_AWARE, ALWAYS_LARGEST, NONE,PLATFORM) where the last policy would use TX_AWARE on Mac andDPI_AWARE on Windows - We create our own policy and always use it (TX_AWARE? or DPI_AWARE?)- We create our own policy that dynamically chooses one of theabove strategies depending on platform or available media or ??? - We could create an optional interface for them to install their ownalgorithm as well. I think it would work best as a delegateinterfacethat one installs into Image so that it can be used with anyimagewithout having to subclass (it wouldn't really have much to dofor BufferedImages or VolatileImages, though): class Image { void setResolutionHelper(ImageResolutionHelper foo); List<Image> getResolutionVariants(); } or: class Graphics { void setResolutionHelper(ImageResolutionHelper foo); }or - anywhere else it could be installed more centrally (perApp context)? and the interface would be something like one of these variants: interface ImageResolutionHelper {// This version would prevent substituting a random image: // They have to return an index into the List<Image> forthat image...public int chooseVariant(Image img, double dpi, number w,number h); or: // This version would allow substituting an arbitrary image:public Image getVariant(Image img, double dpi, number w,number h); }Since they would be in full control of the policy, though, wewouldunfortunately always have to call this, there would be no moretestingin SG2D to see "if" we need to deal with DPI, though perhaps wecoulddocument some internal conditions in which we do not call itfor common cases (but that would have to be well agreed not to get in the way of reasonable uses of the API and well documented)? Note that we would have to do a security audit to make sure thatrandom image substitution couldn't allow any sort of "screenphishing" exploit. ...jimThere is an interesting doc that describes how to writeand also what policy they use for choosing scaled images.I don't see a mention of taking the current transform intoaccount,just physical issues like screen DPI and form factor. Theytalk aboutresolution plateaus and in their recommendations section theytell thedeveloper to use a particular property that tells them thescreen resolution to figure out which image to load if they are loadingmanually. There is no discussion about dynamically loadingmultiple versions of the image based on a dynamic program-applied transform factor as is done on MacOS.Also, they tell developers to draw images to a specific sizerather than using auto-sizing. That begs the question as to how theyinterpret a call to draw an image just using a location inthe presence of various DPI factors.DPI-aware Win32 applications:http://msdn.microsoft.com/en-us/library/dd464646%28v=vs.85%29.aspxIt is suggested to handle WM_DPICHANGED message, loadthe graphicthat has slightly greater resolution to the current DPI anduse StretchBlt to scale down the image. Thanks, Alexandr....jim On 2/7/14 3:00 AM, Alexander Scherbatiy wrote:On 1/22/2014 6:40 AM, Jim Graham wrote:Hi Alexander,Before we get too far down the road on this API, I think weunderstandthe way in which MacOS processes multi-resolution imagesfor HiDPI screens, but have we investigated the processes that Windows uses under Windows 8? My impression is that Windows 8 has included a number of new techniques for dealing with the high resolution displays that it will run on in the Windows tablet and mobile industries andthat these will also come into play as 4K displays (alreadyavailable) become more common on the desktop. We should make sure that what we come up with here can provide native compatibility with either platform's policies and standard practices.If you've investigated the MS policies I'd like to see asummary so that we can consider them as we review this API...There is the Windows Guidelines for scaling to pixel density:http://msdn.microsoft.com/en-us/library/windows/apps/hh465362.aspxwhich says that Windows has automatic resource loadingthat supports three version of images scaling (100%, 140%, and 180%) --------------------------------Without scaling, as the pixel density of a display deviceincreases, the physical sizes of objects on screen get smaller.When UI would otherwise be too small to touch and when textgets too small to read,Windows scales the system and app UI to one of the followingscaling plateaus: 1.0 (100%, no scaling is applied) 1.4 (140% scaling) 1.8 (180% scaling)Windows determines which scaling plateau to use based on thephysical screen size, the screen resolution, the DPI of the screen, and form factor.Use resource loading for bitmap images in the app packageFor bitmap images stored in the app package, provide a separate image for each scaling factor(100%, 140%, and 180%), and name your image files using the "scale" naming convention described below. Windows loads the right image for the current scale automatically. -------------------------------- The image name convention for the various scales is: images/logo.scale-100.png images/logo.scale-140.png images/logo.scale-180.pngThe 'ms-appx:///images/logo.png' uri is used to load theimage in an application.If we want to support this in the same way as it is donefor Mac OS X the WToolkit should return MultiResolution image in case if the loaded image has .scale-* qualifiers.The Graphics class can request an image with necessaryresolution from the MultiResolution image. It seems that nothing should be changed in the MultiResolution interface in this case. Thanks, Alexandr....jim On 1/14/14 2:54 AM, Alexander Scherbatiy wrote:Hello, Could you review the fix:bug: https://bugs.openjdk.java.net/browse/JDK-8029339webrev: http://cr.openjdk.java.net/~alexsch/8029339/webrev.00This is a proposal to introduce an API that allows tocreate a custom multi resolution image. I. It seems reasonable that the API should provide two basic operations: 1. Get the resolution variant based on the requested image width and height:- Image getResolutionVariant(int width, int height)Usually the system provides the scale factor which represents thenumber of pixels corresponding to each linear unit on thedisplay.However, it has sense to combine the scale factor andthe current transformations to get the actual image size to be displayed. 2. Get all provided resolution variants: - List<Image> getResolutionVariants() There are several uses cases: - Create a new multi-resolution image based on the given multi-resolution image. - Pass to the native system the multi-resolution image. For example,a use can set to the system the custom multi-resolutioncursor.II. There are some possible ways where the new API can beadded 1. java.awt.Image.The 2 new methods can be added to the Image class. Auser can overridethe getResolutionVariant() and getResolutionVariants()methods to provide the resolution variants or there can be default implementations of these methods if a user puts resolution variants to the list in the sorted order. To check that the image has resolution variants the following statement can be used: image.getResolutionVariants().size() != 1The disadvantage is that there is an overhead that theImage class should contain the List object and not all images can have resolution variants. 2. Introduce new MultiResolutionImage interface. A user should extend Image class and implement the MultiResolutionImage interface. For example: --------------------- public class CustomMultiResolutionImage extends BufferedImage implements MultiResolutionImage { Image highResolutionImage;public CustomMultiResolutionImage(BufferedImagebaseImage, BufferedImage highResolutionImage) { super(baseImage.getWidth(), baseImage.getHeight(), baseImage.getType()); this.highResolutionImage = highResolutionImage; Graphics g = getGraphics(); g.drawImage(baseImage, 0, 0, null); g.dispose(); } @Overridepublic Image getResolutionVariant(int width, intheight) { return ((width <= getWidth() && height <= getHeight())) ? this : highResolutionImage; } @Override public List<Image> getResolutionVariants() { return Arrays.asList(this, highResolutionImage); } } ---------------------The current fix adds the MultiResolutionImage interfaceand public resolution variant rendering hints. Thanks, Alexandr.