[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-2d-dev
Subject:    Re: [OpenJDK 2D-Dev] [9] Review request for 8163854 Add ToolkitImage.getImage() method which loads a
From:       Jim Graham <james.graham () oracle ! com>
Date:       2016-09-23 6:43:44
Message-ID: 35d3257b-1dc8-6c99-f749-b31b32a77aca () oracle ! com
[Download RAW message or body]

Naming aside - I think I came up with schema a few discussions ago and 
better name suggestions are always welcome...

If we can get away with a list of suffixes then a new class is less 
important, though we have the issue that an array or list of strings is 
mutable.  If any part of this process is lazy then we have to make a 
copy of the list of strings every time the method is invoked.  Wrapping 
them in a class gives us the opportunity to make the list read-only and 
also provide some utility methods to perform the substitutions.  I can 
see it going either way.

If it is API I don't think it should be an inner class, though.  There 
may be other opportunities to use it for other methods here and there 
and so they shouldn't have a Toolkit.FooList class mixed in to their 
method signatures...

			...jim

On 9/22/2016 3:06 PM, Philip Race wrote:
> Does that need a whole new API class to support a list of strings ?
> And it still not a "schema". It is simply one of a list of suffixes.
>
> -phil.
>
> On 9/22/16, 1:51 PM, Jim Graham wrote:
>> If I am developing a skinned UI and I provide a set of 20 images for
>> various parts of the controls and then hand off to my graphic designer
>> to create the DPI variants for all of the images, it is much easier
>> for me to tell them to name all of the images with suffixes for the
>> DPI variants and then load them all with:
>>
>>     getImage("buttoncorner.png", <list of suffixes>);
>>
>> than it is to do:
>>
>>     for (String suffix : mylistofsuffixes) {
>>        array[i] = "buttoncorner" + suffix+".png";
>>     }
>>     getImage("buttoncorner.png", array);
>>
>> for every image in the UI.
>>
>> So, the list of suffixes in some form has value.  As to whether or not
>> delineating the scales along with the suffixes is needed, that's
>> another question...
>>
>>             ...jim
>>
>> On 9/22/16 10:56 AM, Philip Race wrote:
>>>> The method getImageUsingNamingSchemes(String fileName,
>>>> MediaResolutionNamingScheme...
>>>>  namingSchemes) works for any scheme not only for the default one.
>>>
>>> I'm sure it does. My point is that we don't need it.
>>> No one will care or use it. They just want to list their image files.
>>>
>>> The naming scheme is only important for the cases when people
>>> are NOT supplying an explicit list of images to getImage.
>>> So public API for the naming schema - or the float scale - is
>>> completely unnecessary
>>> bloat and complication.
>>>
>>> -phil.
>>>
>>> On 9/22/16, 10:23 AM, Alexandr Scherbatiy wrote:
>>>> On 9/21/2016 9:47 PM, Philip Race wrote:
>>>>> Hi,
>>>>>
>>>>> When the application is specifying the set of images from which
>>>>> to build the MRI you ask the app to specify a "schema" (probably
>>>>> not the
>>>>> right name given that it is per-file), and a floating point scale.
>>>>>
>>>>> I don't see why we need to ask the app to name the files
>>>>> in accordance with our schema in this case .. it should just be
>>>>> able to list the set of files. It looks redundant to an app developer
>>>>> to say "150pct" is scale "1.5".
>>>>
>>>>    The method getImageUsingNamingSchemes(String fileName,
>>>> MediaResolutionNamingScheme... namingSchemes) works for any
>>>> scheme not only for the default one.
>>>>    For example call
>>>>    Image image = toolkit.getImageUsingNamingSchemes(url,
>>>>            new Toolkit.MediaResolutionNamingScheme("-144dpi", 1.5f),
>>>>            new Toolkit.MediaResolutionNamingScheme("-192dpi", 2f)
>>>>    );
>>>>    loads resolution variants image-144dpi.png and image-192dpi.png
>>>> for the base image.png image.
>>>>
>>>>    If it is necessary I can add a method like
>>>> Toolkit.loadImageWithScales(String fileName, float... scales) which
>>>> loads resolution variants for the given scales using the default
>>>> scheme.
>>>>
>>>>   Thanks,
>>>>   Alexandr.
>>>>>
>>>>> Obviously the ideal is the image is exactly what the naming convention
>>>>> implies it is, but what if it is not ?
>>>>>
>>>>> This issue does exist already even in JDK 8 .. if the
>>>>> @2x image is really 1.5X the @1 image
>>>>>
>>>>> Consider what happens if this contradicts the floating point scale ?
>>>>> It appears to me that as implemented, in practice, the app could
>>>>> call it "@XXX",
>>>>> and once @XXX has been used to find the file, the only thing that
>>>>> actually
>>>>> matters is the floating point scale.
>>>>>
>>>>> So the naming schema is not important when they provide the scale.
>>>>>
>>>>> But we still have the issue that the *actual* image size may not be
>>>>> what they said it was  - either explicitly or by convention.
>>>>>
>>>>> Supposing what is claimed to be a 1.5x1.5 scale image is actually
>>>>> 1.0x2.0 times the size of the base image ? It is not even uniform.
>>>>>
>>>>> Ultimately what needs to "win" is the w:h ratio of the base image
>>>>> and we generally would want to pick whichever image best works
>>>>> for the actual device scale, based on the *real* dimensions of
>>>>> the hi-res image, don't we ?
>>>>>
>>>>> In which case, I'd expect us to work out the scale automatically.
>>>>> It is WID_HIRES/WID_BASE x HGT_HIRES/HGT_BASE
>>>>>
>>>>> At which point why do we even need the app to tell us anything
>>>>> except the (full) names of the files where to get the set of images,
>>>>> with the first one being the base .. or perhaps it should always
>>>>> be the "smallest".
>>>>>
>>>>> Otherwise if any are in fact smaller (or the same as) BASE .. do we
>>>>> just discard them ?
>>>>>
>>>>> -phil.
>>>>>
>>>>> On 9/19/16, 12:03 PM, Alexander Scherbatiy wrote:
>>>>>> Hello,
>>>>>>
>>>>>> Could you review the updated fix:
>>>>>>   http://cr.openjdk.java.net/~alexsch/8163854/webrev.01
>>>>>>
>>>>>> The fix includes support for resolution variants loading by
>>>>>> getImage() method for built-in toolkits using the
>>>>>> following media resolution naming scheme (qualifier, scale):
>>>>>> ("@125pct", 1.25), ("@150pct", 1.5), ("@200pct" or
>>>>>> "@2x", 2), ("@250pct", 2.5), ("@300pct" or "@3x", 3).
>>>>>>
>>>>>> Thanks,
>>>>>> Alexandr.
>>>>>>
>>>>>> On 25/08/16 05:39, Philip Race wrote:
>>>>>>> FWIW I think the most important image loading use case
>>>>>>> is that some generic resource loading code - perhaps JDK code -
>>>>>>> will get a URL for where
>>>>>>> the resources are and go hunting. It is never going to call this
>>>>>>> API .. so
>>>>>>> it had better be an optimisation and not a necessity
>>>>>>>
>>>>>>> -phil.
>>>>>>>
>>>>>>>
>>>>>>> On 8/24/16, 5:24 PM, Philip Race wrote:
>>>>>>>> Alexander,
>>>>>>>>
>>>>>>>> Were  the existing Toolkit.getImage(String/URL) APIs not
>>>>>>>> enhanced to
>>>>>>>> do this for you automatically ? I suppose I thought they were but
>>>>>>>> they can't be since you are using getImage(String) here.
>>>>>>>>
>>>>>>>> IMO that would be more important than this.
>>>>>>>>
>>>>>>>> And in any case I don't see why this is solved only for local
>>>>>>>> files.
>>>>>>>>
>>>>>>>> I am *not* asking for that right now. I am asking if the
>>>>>>>> existing Toolkit APIs
>>>>>>>> can load a multi-res image and if not, why not  and can we fix
>>>>>>>> that instead ..
>>>>>>>>
>>>>>>>> -phil.
>>>>>>>>
>>>>>>>> On 8/24/16, 9:36 AM, Alexander Scherbatiy wrote:
>>>>>>>>>
>>>>>>>>> Hello,
>>>>>>>>>
>>>>>>>>> Could you review the fix:
>>>>>>>>>   bug: https://bugs.openjdk.java.net/browse/JDK-8163854
>>>>>>>>>   webrev: http://cr.openjdk.java.net/~alexsch/8163854/webrev.00
>>>>>>>>>
>>>>>>>>>   The public API which allows to load an image with resolution
>>>>>>>>> variants based on the provided media resolution
>>>>>>>>> naming scheme is added:
>>>>>>>>>   - Toolkit.MediaResolutionNamingScheme class
>>>>>>>>>   - Toolkit.getImageUsingNamingSchemes(String fileName,
>>>>>>>>> MediaResolutionNamingScheme... namingSchemes)
>>>>>>>>>
>>>>>>>>>   A simple example for images which use naming scheme @150pct
>>>>>>>>> for scale 1.5 and @2x for scale 2 is:
>>>>>>>>>     image_name.ext
>>>>>>>>>     image_name@150pct.ext
>>>>>>>>>     image_name@2x.ext
>>>>>>>>>
>>>>>>>>>     Toolkit toolkit = Toolkit.getDefaultToolkit();
>>>>>>>>>     Image image = toolkit.getImageUsingNamingSchemes(fileName,
>>>>>>>>>             new Toolkit.MediaResolutionNamingScheme("@150pct",
>>>>>>>>> 1.5f),
>>>>>>>>>             new Toolkit.MediaResolutionNamingScheme("@2x", 2f)
>>>>>>>>>     );
>>>>>>>>>
>>>>>>>>>   Thanks,
>>>>>>>>>   Alexandr.
>>>>>>>>>
>>>>>>
>>>>
[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic