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

List:       openjdk-macosx-port-dev
Subject:    Re: <AWT Dev> [9] Review request for 8033534 Get MultiResolution image from native system
From:       Petr Pchelko <petr.pchelko () oracle ! com>
Date:       2014-02-26 16:27:37
Message-ID: 63F14163-B0A7-4E5C-8F4C-F1A0C52E177D () oracle ! com
[Download RAW message or body]

Hello, Alexander.

The fix look good to me.

With best regards. Petr.

26 февр. 2014 г., в 6:40 после полудня, Alexander Scherbatiy \
<alexandr.scherbatiy@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.scherbatiy@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.scherbatiy@oracle.com <mailto:alexandr.scherbatiy@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.
> > > > 
> > > > 
> > > > 
> > > > 
> > > > 
> 


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