<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