<Swing Dev> [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
Thu Aug 11 19:10:01 UTC 2016
Hi Alexandr,
Should something be done to transfer the array of sizes to the new
instance if the source is an MRCI? Perhaps a special case for MRCI as
well that calls mrciInstance.map(mapper) instead of constructing a brand
new object from scratch?
...jim
On 08/11/2016 01:32 AM, Alexander Scherbatiy wrote:
>
> Could you review the updated fix:
> http://cr.openjdk.java.net/~alexsch/8151303/webrev.03
>
> MultiResolutionToolkitImage handing is added to the
> MultiResolutionCachedImage.map() method.
>
> Thanks,
> Alexandr.
>
> On 11/08/16 01:46, Jim Graham wrote:
>> Ah, yes, only ToolkitImages can be "not yet loaded" in that manner.
>>
>> A quick look suggests that a MRCI should not be an instance of MRTI,
>> but MRCI.map() does not force its argument to be an instance of MRCI,
>> just MRI, so it would be possible for someone to pass an MRTI to
>> MRCI.map() and then it would have this problem.
>>
>> Should we change the argument of MRCI.map() to MRCI? (And then you
>> don't need to cast to Image and use getWidth/Height(Observer) to get
>> its dimensions.)
>>
>> If that is not feasible, we should have it do something different for
>> a non-MRCI...
>>
>> ...jim
>>
>> On 8/10/16 5:35 AM, Alexander Scherbatiy wrote:
>>> 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