Hi Alexandr,
On 3/21/14 7:33 AM, Alexander Scherbatiy wrote:
Hello,
Could you review the updated fix:
http://cr.openjdk.java.net/~alexsch/8029339/webrev.02/
- transform is removed from the getResolutionVariant(...) method
I notice that you reverted the changes to SG2D so that it now calculates
the destination size as well, but it is different now:
- it calls getTransform() which requires constructing a new transform
and extracts out the pixel scale
- thus the new values calculated are not the size on the destination,
but the size on the destination ignoring its DPI - was that intentional?
I like the thought that the method would be told exactly how many real
pixels would be used for the image.
- width/height args from the getResolutionVariant(...) are renamed
to destImageWidth/destImageHeight
That looks good, but they aren't quite the true dest sizes if the
changes to SG2D remain, they are a representation of the DPI-unaware
(but CTM-scaled) size of the image.
- I have not added imgw/imgh args because they can be obtained as
getWidth(null), getHeight(null)
If this remains as a method on the image for which variants are being
provided, that is true. I had a concept that this method might be
broken out into a hint that could be added to SG2D to customize image
variant choices on all images, not just on a particular image - and that
would require the method signature to not assume it is implemented on
the actual Image object itself.
The thought I'm having there is what if an app doesn't like having
Windows semantics on a Windows box and Mac semantics on a Mac box and
wants to pick one or the other and use it on all platforms? How do they
override all standard resolution decisions without having to custom wrap
every image?
So the method looks like: getResolutionVariant(float logicalDPIX,
float logicalDPIY, float destImageWidth, float destImageHeight)
See others comments inline...
On 3/21/2014 3:50 AM, Jim Graham wrote:
Hi Alexandr,
SG2D leaks its internal transform to the implementation. A clone must
be created to prevent breaking encapsulation of graphics state. The
clone raises the cost of calling the interface so we need to evaluate
how important it is to have the transform be included in the interface.
In the docs for the getRV() method you say that the width and height
are of the destination drawing surface. I think you mean the width
and height of the image, right? The "destination drawing surface"
would be the window or screen you are rendering onto.
Given the trickiness of calculating the true destination size of the
image, perhaps we should leave the old code to do that in the
SG2D.getIR() method and pass the destination size to the choosing
method instead of the transform? That would eliminate the need to
clone the transform.
The class docs for the interface kind of imply that one would/should
use the w,h as the deciding factor, but that is just one strategy.
It's a subtle distinction, so I'll try to give some more specific
wording once we nail down the semantics. The pseudo-code could also
clean up things like providing an immutable List.
I'm a little disturbed by someone subclassing BufImg to implement this
interface. It goes against my perspective of a BufImg being a
concrete bag of one set of pixels. Let me see if I understand the
constraints here:
- We want TK images on Mac to advertise @2x variants this way?
- We want users to be able to create their own bag of images as well?
- Do we need users to be able to do that on a BufferedImage?
- Do we need users to be able to add an image variant to an existing
image?
If it is just the first 2 bullet items above, then what about:
Yes.
public class java.awt.image.MultiResolutionImage extends java.awt.Image {
public Image getRV(); // Same as in your interface
public List<Image> getVariants(); // Same as the i/f
}
We can create a MultiResolutionImage based on the given array/list
of resolution variants.
What about the case where resolution variants are not prepared and
should be created by demand?
I think that goes far beyond the current need and I don't think images
should really be used for cases like that. We are trying to solve the
case of existing alternate representations expressed through forms of
media. We should do that well. I'm not sure that embracing this
additional concept really helps us here, but it shouldn't get in the way.
Having said that...
The MultiResolutionImage class implementation should be too
generic to take it into account.
My design is perfectly appropriate for doing that. Your code example
below can be expressed as an implementation of the single-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 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 simply returning the base media
that the object uses from which to construct its variants...?
I think it is better to provide both the MultiResolutionImage and
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 trying to
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...
...jim
The 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();
}
@Override
public 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 and height
}
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, Image
variants...) 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 Lambda
capabilities by making a single-method interface for it that they
supply in a factory method.
I realize that the interface you created is more fundamentally OO, but
the Image class has always been special in this regard in the AWT
ecosystem (in so far as we do not support someone implementing their
own Image subclass even though it is technically possible). Because of
this special nature of Image, we end up with the situation that if
someone were given a need to create a subclass of Image, then they
would turn to BufImg as their superclass even though BufImg is
essentially an implementation-specific leaf node on the Image class
hierarchy. This approach with a factory method to create an internal
subclass of the new MRI class mirrors the existing cases of Image
objects 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 from
MultiResolutionImage 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 base image?
- 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 to Image
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 where they
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 manual
construction of an image set, right? Forgive me if I'm forgetting
something, but I seem to recall that manual Multi-Res images was
proposed 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 manually
construct 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 "just
a couple/3/4 numbers" way to embed that data into a method call
argument list then we can make this lighter weight.
What about simply having floating point (double) dimensions on the
rendered size
There 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.
plus a single floating point "logical DPI" for the screen?
There is the ID2D1Factory::GetDesktopDpi method which returns
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 for the
x-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 allow to use
different strategies like scale factor/transfom/OS based
to query a resolution variant. The OS based strategy can be
used by
default.
For Mac policy, all we need is the transformed dimensions, which can
be passed in as FP for generality. For Windows policy, all we need
is logical DPI for the screen. What other information would we
need, or would an algorithm like to use, that can't be computed from
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.java
A developer should be able to implement a custom algorithm to
query 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.
...jim
Thanks,
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 of
integers in the API when Windows uses non-integer multiples
It 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 as the API
that queries the scaling factor from a SurfaceManager. I seem to
remember some integer return values in that, but Windows might have
the answer 1.4 or 1.8, depending on the screen scaling factor
that was
determined from the UI.
In terms of the getResolutionVariant() method here, those
non-integer
screen scaling factors don't directly impact this API. But, we have
some 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 floating
point value, so the integer values used in this API are already
approximations 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 a
transformed size of W+0.5 will be truncated to W and the lower
resolution image will be used. Does that conform to Mac
guidelines? Do
they require the truncated size to reach W+1 before the next size is
used? Passing in float or double values would sidestep all of that
since then the comparisons would be done with full precision, but as
long as we can determine a "best practices compatible with all
platforms" rule on how to round to integers, then integers are OK
there.
- The Windows document you cite below suggests that the
determination
should be made by looking at the Screen DPI and choosing the next
higher media variant based on that screen DPI. They do not specify
choosing media based on the current transform as is done for
Mac. If
we stick with supplying values that are used to determine which
media
to use, then on Windows we should not take the transform into
account,
but instead query the SurfaceManager for the scale factor and only
transform by those values (even if the current transform was
manually
overridden to identity).
There are pros and cons to both approaches.
Mac ensures that you are always using the best media for any given
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 version of the
1000x1000 will not look precisely like the original 500x500 did.
With
@2x images only, this effect is minimized so the advantage of always
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 and
DPI_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 the above
strategies depending on platform or available media or ???
- We could create an optional interface for them to install their
own
algorithm as well. I think it would work best as a delegate
interface
that one installs into Image so that it can be used with any image
without having to subclass (it wouldn't really have much to do for
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 (per App
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> for that
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, we would
unfortunately always have to call this, there would be no more
testing
in SG2D to see "if" we need to deal with DPI, though perhaps we
could
document some internal conditions in which we do not call it for
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 that
random image substitution couldn't allow any sort of "screen
phishing"
exploit.
...jim
and also what policy they use for choosing scaled images.
I don't see a mention of taking the current transform into
account,
just physical issues like screen DPI and form factor. They talk
about
resolution plateaus and in their recommendations section they
tell the
developer to use a particular property that tells them the screen
resolution to figure out which image to load if they are loading
manually. There is no discussion about dynamically loading
multiple
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 size
rather
than using auto-sizing. That begs the question as to how they
interpret a call to draw an image just using a location in the
presence of various DPI factors.
There is an interesting doc that describes how to write
DPI-aware
Win32 applications:
http://msdn.microsoft.com/en-us/library/dd464646%28v=vs.85%29.aspx
It is suggested to handle WM_DPICHANGED message, load the
graphic
that has slightly greater resolution to the current DPI and use
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 we
understand
the way in which MacOS processes multi-resolution images for
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
and
that these will also come into play as 4K displays (already
available)
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 a
summary 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.aspx
which says that Windows has automatic resource loading that
supports
three version of images scaling (100%, 140%, and 180%)
--------------------------------
Without scaling, as the pixel density of a display device
increases, the
physical sizes of objects on screen get smaller.
When UI would otherwise be too small to touch and when text gets
too
small to read,
Windows scales the system and app UI to one of the following
scaling
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 the
physical
screen size, the screen resolution, the DPI of the screen, and
form
factor.
Use resource loading for bitmap images in the app package For
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.png
The 'ms-appx:///images/logo.png' uri is used to load the image
in an
application.
If we want to support this in the same way as it is done
for 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 necessary
resolution
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-8029339
webrev:
http://cr.openjdk.java.net/~alexsch/8029339/webrev.00
This is a proposal to introduce an API that allows to
create 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
the
number of pixels corresponding to each linear unit on the
display.
However, it has sense to combine the scale factor and the
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-resolution cursor.
II. There are some possible ways where the new API can be added
1. java.awt.Image.
The 2 new methods can be added to the Image class. A user
can
override
the 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()
!= 1
The disadvantage is that there is an overhead that the Image
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(BufferedImage
baseImage,
BufferedImage highResolutionImage) {
super(baseImage.getWidth(), baseImage.getHeight(),
baseImage.getType());
this.highResolutionImage = highResolutionImage;
Graphics g = getGraphics();
g.drawImage(baseImage, 0, 0, null);
g.dispose();
}
@Override
public Image getResolutionVariant(int width, int
height) {
return ((width <= getWidth() && height <=
getHeight()))
? this : highResolutionImage;
}
@Override
public List<Image> getResolutionVariants() {
return Arrays.asList(this, highResolutionImage);
}
}
---------------------
The current fix adds the MultiResolutionImage interface and
public
resolution variant rendering hints.
Thanks,
Alexandr.
