Looks good. +1
...jim
On 08/15/2016 08:47 AM, Alexander Scherbatiy wrote:
Could you review the updated fix:
http://cr.openjdk.java.net/~alexsch/8151303/webrev.04
The MRCI.sizes arrays is reused for for MultiResolutionCachedImage.
Thanks,
Alexandr.
On 11/08/16 23:10, Jim Graham wrote:
Hi Alexandr,
Should something be done to transfer the array of sizes to the new
instance if the source is an MRCI? Perhaps a special case for MRCI as
well that calls mrciInstance.map(mapper) instead of constructing a
brand new object from scratch?
...jim
On 08/11/2016 01:32 AM, Alexander Scherbatiy wrote:
Could you review the updated fix:
http://cr.openjdk.java.net/~alexsch/8151303/webrev.03
MultiResolutionToolkitImage handing is added to the
MultiResolutionCachedImage.map() method.
Thanks,
Alexandr.
On 11/08/16 01:46, Jim Graham wrote:
Ah, yes, only ToolkitImages can be "not yet loaded" in that manner.
A quick look suggests that a MRCI should not be an instance of MRTI,
but MRCI.map() does not force its argument to be an instance of MRCI,
just MRI, so it would be possible for someone to pass an MRTI to
MRCI.map() and then it would have this problem.
Should we change the argument of MRCI.map() to MRCI? (And then you
don't need to cast to Image and use getWidth/Height(Observer) to get
its dimensions.)
If that is not feasible, we should have it do something different for
a non-MRCI...
...jim
On 8/10/16 5:35 AM, Alexander Scherbatiy wrote:
On 09/08/16 03:49, Jim Graham wrote:
Does MultiResolutionCachedImage.map() work if the Image hasn't been
loaded yet (where getWidth/Height(Observer) can
return -1)? Can it ever be called in a case like that?
Could we rely on the fact that getWidth/Height(Observer) returns -1
only for ToolkitImage?
If so, the passed MultiResolutionToolkitImage is handled in
MultiResolutionToolkitImage.map() method.
If no, the fix should be updated to load the image size in some way.
Thanks,
Alexandr.
...jim
On 08/08/2016 12:48 PM, Alexander Scherbatiy wrote:
Just a friendly reminder.
Thanks,
Alexandr.
On 27/06/16 22:17, Alexander Scherbatiy wrote:
Hello,
Could you review the updated fix:
http://cr.openjdk.java.net/~alexsch/8151303/webrev.02
The fix does not use a new public API to apply filters to
multi-resolution images.
Thanks,
Alexandr.
On 14/05/16 02:54, Jim Graham wrote:
Another reason to avoid new API is that we don't have to
involve the
CCC to get this "bug fix" in...
...jim
On 5/13/16 3:50 PM, Jim Graham wrote:
That looks very tight. The only issue I'd have is that it
would be
better if this could be done with non-public API for
now - the map() methods could live on one of the sun.awt.image
classes or even in a Swing implementation utility class
and still work just fine. When we have more time to figure
out how
we're going to handle filtering of MRIs in general
we can decide if this API should live on the base MRI interface.
The only thing we'd lose is BaseMRI having an optimized
implementation of the map() method, but I don't think it's
implementation represents enough of an optimization to matter
when
we are creating and loading dozens of images...
...jim
On 5/12/16 10:08 AM, Alexander Scherbatiy wrote:
Hello,
Could you review the updated fix:
http://cr.openjdk.java.net/~alexsch/8151303/webrev.01
There was a suggestion to postpone the fix for the issue 8152309
Seamless way of using image filters with
multi-resolution images
and continue with the current one:
http://mail.openjdk.java.net/pipermail/2d-dev/2016-April/006766.html
The new version of the fix introduces a mapper method which
allows
to map all resolution variants of one
multi-resolution image to another:
------------
Image image = // original image
Image filteredImage = MultiResolutionImage.map(image,
(img) ->
/* return filtered image */);
------------
Thanks,
Alexandr.
On 21/03/16 22:31, Jim Graham wrote:
We could do that for our own filters, but any random custom
filter
could run into the same issue, so it might not make
sense to upgrade the existing filter mechanism to always
attempt
to do MR filtering. We could either create a
parallel set of MR-aware filter mechanisms (such as the
previously
suggested new method on Toolkit, or a new MR-aware
version of FilteredImageSource for instance) and leave the
existing mechanism as clearly documented as MR-unaware.
Another idea is to tag a filter with an interface that
indicates
that it is MR aware? In any case, some thought needs
to be given to feeding an MR image to a filter that
(potentially
or demonstrably) cannot deal with MR images.
Alternately, we could then recommend that the old image
filtering
code isn't combined with multi-resolution images.
It seems to me that the programmer is mostly in control over
this
happening since they've either manually created the
MR image using the custiom MR image mechanism or they've
supplied
media with multiple resolution files (i.e. "@2x").
Is that really the case?
Whether it is a new filtering mechanism that must be adopted or
simply declaring the old filtering mechanism as
"obsolete with respect to MR images"...
That recommendation then "restricts forward" in that, for
example,
since Swing relies on the old mechanism, Swing then
becomes "not recommended for use with MR images", or "not MR
aware". That's probably not a restriction we want to
promote so it should be viewed as a temporary restriction
reality
and a bug that we'll fix soon, whether by using some
other mechanism to achieve the desired affects, or creating
a new
MR-aware filtering mechanism and using it in Swing.
Similarly, other 3rd party libraries that accept images and do
anything more than display them will have to be
upgraded as well before they become "MR aware" or "MR
accepting"
or whatever term applies here...
...jim
On 3/21/16 8:54 AM, Alexander Scherbatiy wrote:
The one more thing is that image filters should also be
updated
to use
them with multi-resolution images.
For example, consider the case:
----------
Image mrImage = getMultiResolutionImage();
ImageProducer mriProducer = new
FilteredImageSource(mrImage.getSource(), new
CropImageFilter(0,
0, w, h));
Toolkit.getDefaultToolkit().createImage(mriProducer);
----------
The crop image filter applied to each resolution variant just
cuts
images with the same size.
It seems that there should be added API which allows to set a
scale for
a provided image filter to be properly used with the given
resolution
variant.
I have created a separated issue for multi-resolution images
filtering
support:
JDK-8152309 Seamless way of using image filters with
multi-resolution
images
https://bugs.openjdk.java.net/browse/JDK-8152309
Thanks,
Alexandr.
On 15/03/16 20:35, Alexander Scherbatiy wrote:
On 15/03/16 18:06, Sergey Bylokhov wrote:
On 15.03.16 17:01, Alexander Scherbatiy wrote:
One update will be that FilteredImageSource should
implement
MultiResolutionImageProducer even it is used for non
multi-resolution
images.
The MRI can be created using two general ways: using
fixed
number of
resolution variants or generating a resolution variant with
necessary
quality on demand.
The current implementation is rely on that MRToolkitImage
contains a
fixed number of resolution variants. In this case
MediaTracker
can
iterate over resolution variants and load them all.
Using MultiResolutionImageProducer leads that
MRToolkitImage
will not
know about number of resolution variants in case when they
are
generated
on the fly and there will be no way to load all of them by
MediaTracker.
Just an idea to thinking about, can we save this filter to
the MRI
itself and apply it after some resolution variant will be
created on
the fly?
Do you mean that it helps to leave the code in the AquaUtils
class
unchanged:
---------------
124 static Image generateLightenedImage(final Image
image,
final
int percent) {
125 final GrayFilter filter = new GrayFilter(true,
percent);
126 final ImageProducer prod = new
FilteredImageSource(image.getSource(), filter);
127 return
Toolkit.getDefaultToolkit().createImage(prod);
128 }
---------------
or it is just an a better way for a filtered
multi-resolution
image
generation?
Thanks,
Alexandr.
As I see, the way to solve it is only using
MRI.getResolutionVariants()
method for the MultiResolutionImageProducer creation. So
the
result of
the call
toolkit.createImage(new
FilteredImageSource(mrImage.getSource(),
filter));
will be a MRToolkitImage which is based on fixed
number of
filtered
resolution variants from the original MRI.
Thanks,
Alexandr.
...jim
On 3/11/16 5:42 AM, Alexander Scherbatiy wrote:
On 09/03/16 16:58, Sergey Bylokhov wrote:
Probably we should enhance
ImageProducer/Tk.createImage/ImageFilter to
support this functionality? It seems that the number of
usage of
this
check "image instanceof MultiResolutionImage" will grow
over time.
ImageProducer produces pixels for an image and is not
able to
take
an information about the image resolution variants.
May be we can add Toolkit.createImage(Image image,
ImageFilter
imageFilter) method which takes MultiResolutionImage into
account to
cover the common case where filtered image is created.
Thanks,
Alexandr.
I think that at least our own API should support
MultiResolutionImage
w/o such checks, otherwise the user will need to do the
same.
cc 2d-dev
On 09.03.16 15:30, Alexander Scherbatiy wrote:
Hello,
Could you review the fix:
bug:
https://bugs.openjdk.java.net/browse/JDK-8151303
webrev:
http://cr.openjdk.java.net/~alexsch/8151303/webrev.00
The AquaUtils does not take into account
MultiResolutionImage
for
selected/disabled/lightened images generation.
The fix also leaves the MultiResolutionCachedImage
check because
the
base system icon size can be differ from the requested
painted
size.
Thanks,
Alexandr.
