<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 Aug 8 19:48:06 UTC 2016


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