<Swing Dev> [OpenJDK 2D-Dev] [9] Review request for 8151303 [macosx] [hidpi] JButton's low-res. icon is visible when clicking on it
Alexander Scherbatiy
alexandr.scherbatiy at oracle.com
Thu Aug 11 08:32:32 UTC 2016
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.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>
>>>>>
>>>>
>>
More information about the swing-dev
mailing list