<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
Wed Aug 10 12:35:45 UTC 2016
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