[OpenJDK 2D-Dev] [9] Review request for 8151303 [macosx] [hidpi] JButton's low-res. icon is visible when clicking on it

Jim Graham james.graham at oracle.com
Fri May 13 22:54:10 UTC 2016 

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 2d-dev mailing list