Hello, Alexander. The fix look good to me.
With best regards. Petr. 26 февр. 2014 г., в 6:40 после полудня, Alexander Scherbatiy <alexandr.scherba...@oracle.com> написал(а): > > Hello, > > Could you review the updated fix: > http://cr.openjdk.java.net/~alexsch/8033534/webrev.03/ > > On 2/26/2014 4:54 PM, Petr Pchelko wrote: >> Hello, Alexander. >> >> I have a couple of comments: >> >> 1. You could replace the first loop with indexOfObjectPassingTest method.. >> Not sure if this would look cleaner, up to you. > Updated. There is one more way to use one loop instead of two. All of them > does not look simple. > >> 2. I suppose JNFNewObjectArray could throw the OOM and we would get a >> parfait warning, could you please add CHECK_NULL_RETURN after it. > CHECK_NULL_RETURN is added. >> 3. In CImage.java you are setting the currentImageIndex to the biggest image >> representation smaller that the one requested and this representation >> would be used as a base one in the MultiResolutionBufferedImage. However in >> MultiResolutionBufferedImage getResolutionVariant you are returning >> the smallest variant bigger than the requested one. Is this correct? > I think that it is correct. Assume that an image with size 300x300 is > requested but there are only resolution variants with sizes [250x250] and > [350x350]. > The resolution variant with [350x350] size is used as the base image. > Now we need to draw the image to region [300x300]. The resolution variant > with size [350x350] will be used from the MultiResolution image. > > Thanks, > Alexandr. > > >> >> Thank you. >> With best regards. Petr. >> >> On 26.02.2014, at 16:08, Alexander Scherbatiy >> <alexandr.scherba...@oracle.com> wrote: >> >>> Hello, >>> >>> Could you review the updated fix: >>> http://cr.openjdk.java.net/~alexsch/8033534/webrev.02/ >>> >>> This is the same fix. The only difference is that the >>> MultiResolutionBufferedImage class is used from the fix JDK-8035069. >>> >>> Thanks, >>> Alexandr. >>> >>> >>> On 2/10/2014 7:05 PM, Scott Palmer wrote: >>>> Just to be clear, "the image representations are chosen to be closest to >>>> the requested size" is not accurate. >>>> This change returns the smallest image representation that is greater than >>>> or equal to the requested size. (Which I believe is the correct thing to >>>> do.) >>>> A smaller image representation may be closer to the requested size, but it >>>> makes more sense to return a larger image since scaling down to size >>>> should produce better results than scaling up. >>>> >>>> Scott >>>> >>>> >>>> On Mon, Feb 10, 2014 at 8:53 AM, Alexander Scherbatiy >>>> <alexandr.scherba...@oracle.com <mailto:alexandr.scherba...@oracle.com>> >>>> wrote: >>>> >>>> >>>> Could you review the updated fix: >>>> http://cr.openjdk.java.net/~alexsch/8033534/webrev.01 >>>> <http://cr.openjdk.java.net/%7Ealexsch/8033534/webrev.01> >>>> >>>> - The image representations are chosen to be closest to the >>>> requested size. >>>> >>>> Thanks, >>>> Alexandr. >>>> >>>> >>>> On 2/4/2014 5:00 PM, Sergey Bylokhov wrote: >>>> >>>> Hi, Alexander. >>>> I think that getResolutionVariant should return an image which >>>> is close as much as possible to the requested size. >>>> >>>> On 04.02.2014 16:42, Alexander Scherbatiy wrote: >>>> >>>> >>>> Hello, >>>> >>>> Could you review the fix: >>>> bug: https://bugs.openjdk.java.net/browse/JDK-8033534 >>>> webrev: >>>> http://cr.openjdk.java.net/~alexsch/8033534/webrev.00 >>>> <http://cr.openjdk.java.net/%7Ealexsch/8033534/webrev.00> >>>> >>>> - The method that gets a sorted array of NSImage >>>> representaion pixel sizes for the given image size is added >>>> - A MultiResolution image is created if an NSImage has >>>> several representations for the given image size >>>> >>>> Thanks, >>>> Alexandr. >>>> >>>> >>>> >>>> >>>> >
