<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
Mon Jun 27 18:17:44 UTC 2016
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