Hello, Alexander. The fix looks good.
With best regards. Petr. On 21 мая 2014 г., at 17:47, Alexander Scherbatiy <alexandr.scherba...@oracle.com> wrote: > > Could you review the updated fix: > http://cr.openjdk.java.net/~alexsch/8040291/webrev.03/ > > - The test checks that the resolution-variant observer is called at least > once. > > Thanks, > Alexandr. > > On 5/21/2014 2:50 PM, Alexander Scherbatiy wrote: >> >> Could you review the updated fix: >> http://cr.openjdk.java.net/~alexsch/8040291/webrev.02/ >> >> - The isRVObserevr typo is fixed >> >> On 5/20/2014 7:32 PM, Petr Pchelko wrote: >>>> Could you review the updated fix: >>>> http://cr.openjdk.java.net/~alexsch/8040291/webrev.01 >>>> >>>> - The getRVSize() method from the SunToolkit is fixed >>> Thank you for the update, but I still have questions about the test: >>> 1. There’s a typo in isRVObserevr. >>> 2. Do we need the isRVObserevr method? How could it be that this >>> method return false in this test and it’s a correct behavior and not a bug? >>> I see it’s only possible from SunToolkit.prepareImage if bits are ERROR | >>> ABORT. >>> Couldn’t we get rid of this method? Walking up the stack doesn’t look like >>> the most >>> reliable solution.. >> >> The ImageObserver is used for two purposes in the Toolkit prepareImage() >> method: >> - to query which part of the image should be loaded >> - to notify an object about which image information is available >> >> To prepare a multi-resolution image it needs to prepare its resolution >> variant as well. >> The only way to know which part of the resolution variant should be loaded >> is requesting >> the base image observer. It leads that there will be a notification for the >> object that contains >> the resolution variant instead of the base image. To improve it the >> MultiResolutionToolkitImage >> wraps the base image observer for the resolution variant. The object gets >> notification >> which contains only the base image and updated sizes after that. >> >> The another case which is fixed in this issue is that the resolution >> variant can be loaded first >> and it notifies the object that the image is loaded. The fix reduces the >> resolution variant info >> flags so the object should wait for image loading notification from the >> base image. >> >> The test needs to check that the wrapped observer from the resolution >> variant does not send >> more info than the original image has already had. The isRVObserver() just >> checks >> that the observer is called from the resolution variant and not is from the >> base image. >> >> Thanks, >> Alexandr. >> >>> >>> With best regards. Petr. >>> >>> On May 20, 2014, at 7:07 PM, Alexander Scherbatiy >>> <alexandr.scherba...@oracle.com> wrote: >>> >>>> Could you review the updated fix: >>>> http://cr.openjdk.java.net/~alexsch/8040291/webrev.01 >>>> >>>> - The getRVSize() method from the SunToolkit is fixed >>>> >>>> On 5/20/2014 6:46 PM, Petr Pchelko wrote: >>>>> Hello, Alexander. >>>>> >>>>> SunToolkit:876 size == -1 ? -1 : size >>>>> >>>>> What’s going on here? Isn’t this equal to just size? >>>>> I believe is’t wrong and the size should be multiplied by 2 somewhere? >>>>> If the method is wrong how does the test pass? >>>> The test passes because it uses SunToolkit.prepareImage() method with >>>> the -1 size. >>>> It also uses the image observer that requires to load all image bits. >>>> >>>> Thanks, >>>> Alexandr. >>>> >>>>> With best regards. Petr. >>>>> >>>>> On May 20, 2014, at 6:32 PM, Alexander Scherbatiy >>>>> <alexandr.scherba...@oracle.com> wrote: >>>>> >>>>>> Hello, >>>>>> >>>>>> Could you look at the fix? >>>>>> >>>>>> Thanks, >>>>>> Alexandr. >>>>>> >>>>>> On 4/30/2014 6:34 PM, Alexander Scherbatiy wrote: >>>>>>> Hello, >>>>>>> >>>>>>> Could you review the fix: >>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8040291 >>>>>>> webrev: http://cr.openjdk.java.net/~alexsch/8040291/webrev.00 >>>>>>> >>>>>>> >>>>>>> The SunToolkit.prepareResolutionVariant() method wraps the base image >>>>>>> observer and >>>>>>> passes it to the resolution variant. It leads that the resolution >>>>>>> variant notifies >>>>>>> the base image about info changing. >>>>>>> >>>>>>> When the base image is loaded by the MediaTracker and the resolution >>>>>>> variant is loaded >>>>>>> first it calls the base image observer and wrongly finishes the base >>>>>>> image loading. >>>>>>> >>>>>>> >>>>>>> The fix passes the reduced resolution variant info flags to the base >>>>>>> image >>>>>>> so the base image loading is finished only after notifiing by the >>>>>>> original base image observer. >>>>>>> >>>>>>> >>>>>>> Thanks, >>>>>>> Alexandr. >>>>>>> >> >
