Hi All, A gentle reminder, please review my code changes. With Regards, Avik Niyogi
> On 08-Jul-2016, at 4:24 pm, Yuri Nesterenko <yuri.nestere...@oracle.com> > wrote: > > It pass for me if I provide some time to process native events > after the user activity simulation. For instance, > setAutoDelay(50) for robot does that plus waitForIdle() > after every mouseMove(). In this case, the part with that additional > check and a (misleading, I think) comment about the color profiles > may be removed. The test would take much more time though, and > @run main/timeout=600 bug8057791 > line would be necessary. > > Thanks, > -yan > > On 07/08/2016 04:28 AM, Avik Niyogi wrote: >> The test does not pass if mac specific check is not done for >> backgroundcolor. >> The check is required to get the same values from BufferedImage as they >> are the same as found with Digital Color Meter. >> This test case fixes that. >> Please provide inputs if this fix will get a +1 or if not any positive >> inputs to modify the test case will be welcome. >> >> With Regards, >> Avik Niyogi >>> On 07-Jul-2016, at 9:51 pm, Semyon Sadetsky >>> <semyon.sadet...@oracle.com <mailto:semyon.sadet...@oracle.com>> wrote: >>> >>> >>> >>> On 7/7/2016 6:30 PM, Yuri Nesterenko wrote: >>>> On 07/07/2016 06:15 PM, Semyon Sadetsky wrote: >>>>> >>>>> >>>>> On 7/7/2016 5:58 PM, Yuri Nesterenko wrote: >>>>>> On 07/07/2016 05:35 PM, Yuri Nesterenko wrote: >>>>>>> On 07/07/2016 05:04 PM, Semyon Sadetsky wrote: >>>>>>>> >>>>>>>> >>>>>>>> On 07.07.2016 16:35, Avik Niyogi wrote: >>>>>>>>> Hi Semyon, >>>>>>>>> >>>>>>>>> Thank you for the review comment. >>>>>>>>> >>>>>>>>> In Mac OS X, *System Preferences > Displays > Colors > Display >>>>>>>>> Profile* section, the default value is *Color LCD*. >>>>>>>>> This causes a failure in some test cases which uses robot.The colour >>>>>>>>> configuration it expects to use is the *Generic RGB Profile*. >>>>>>>>> That is what “Non-generic display settings” means. >>>>>>>> AFAIK there are instruction that tests should be executed using color >>>>>>>> profile with no color corrections on OS X. I guess there are many >>>>>>>> other >>>>>>>> tests that fail with color correction. >>>>>>>> If this is a root cause than the bug doesn't need to be fixed. >>>>>>> >>>>>>> Well, I filed this bug and I used Generic RGB on all my >>>>>>> test machines. There may be additional precautions in the tests >>>>>>> about that but misconfiguration is not the root case here. >>>>>>> That said, I feel vary about attempts to guess Apple colors >>>>>> wary I mean >>>>>>> in non-generic profiles. >>>>> Yuri. Do you mean that the non-generic is not a root case? >>>> No: I had Generic RGB everywhere, and it failed for me. >>>> I should say that the new version of the test properly >>>> fails with b120 and pass with current PIT. And colors >>>> are visibly different in the two builds: so the test works OK now. >>> That contradicts with the description of the fix. >>> Probably the test does unnecessary care about the non-Generic profiles. >>> >>> 159 if (!foundMatch && isMac()) { >>> 160 //One more chance for Mac due to non-Generic >>> display setting >>> 161 detectedColor = new Color(img.getRGB(x, y), true); >>> 162 if (detectedColor.equals(colorCheck)) { >>> 163 foundMatch = true; >>> 164 break checkLoops; >>> 165 } >>> 166 } >>> >>> Does it mean that the code above, which is necessary to serve >>> non-Generic profiles on OS X, may be removed and the test still passes? >>> >>> --Semyon >>>> >>>> -yan >>>> >>>>> >>>>> --Semyon >>>>>>> >>>>>>> -yan >>>>>>> >>>>>>> >>>>>>>> >>>>>>>> --Semyon >>>>>>>>> >>>>>>>>> Regarding “Negative scenarios”, these include cases where >>>>>>>>> colours are >>>>>>>>> different from the expected background or foreground colors >>>>>>>>> is checked with the robot and BufferedImage respectively to >>>>>>>>> *eliminate >>>>>>>>> false positives due to coincidentally finding a match* by using >>>>>>>>> robot >>>>>>>>> or BufferedImage. >>>>>>>>> >>>>>>>>> Please find my changes appropriating the inputs provided. >>>>>>>>> I removed the variable foundMatch as I found that it is not required >>>>>>>>> at all and incorporated the suggestion to use return instead of a >>>>>>>>> labelled break. >>>>>>>>> >>>>>>>>> http://cr.openjdk.java.net/~aniyogi/8160438/webrev.01/ >>>>>>>>> <http://cr.openjdk.java.net/%7Eaniyogi/8160438/webrev.01/> >>>>>>>>> >>>>>>>>> >>>>>>>>>> On 07-Jul-2016, at 6:30 pm, Semyon Sadetsky >>>>>>>>>> <semyon.sadet...@oracle.com <mailto:semyon.sadet...@oracle.com>> >>>>>>>>>> wrote: >>>>>>>>>> >>>>>>>>>> Hi Avik, >>>>>>>>>> >>>>>>>>>> could you clarify what is "Non-generic display settings"? Is it >>>>>>>>>> known >>>>>>>>>> bug on OS X? >>>>>>>>>> And also please be more specific on "negative scenarios" why >>>>>>>>>> they are >>>>>>>>>> necessary ? >>>>>>>>>> >>>>>>>>>> Also could you replace labeled break with "return foundMatch; " >>>>>>>>>> >>>>>>>>>> --Semyon >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> On 07.07.2016 15:11, Avik Niyogi wrote: >>>>>>>>>>> Hi All, >>>>>>>>>>> >>>>>>>>>>> Kindly review the fix for JDK9. >>>>>>>>>>> >>>>>>>>>>> *Bug: >>>>>>>>>>> *<https://bugs.openjdk.java.net/browse/JDK-8160438>https://bugs.openjdk.java.net/browse/JDK-8160438 >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> *Webrev: >>>>>>>>>>> *<http://cr.openjdk.java.net/%7Eaniyogi/8160438/webrev.00/>http://cr.openjdk.java.net/~aniyogi/8160438/webrev.00/ >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> *Issue: *test javax/swing/plaf/nimbus/8057791/bug8057791.java >>>>>>>>>>> consistently fails on OS X 10.10 >>>>>>>>>>> >>>>>>>>>>> *Cause: * Due to bug in detecting color for Non-generic display >>>>>>>>>>> settings for Mac hardware, test case failed >>>>>>>>>>> >>>>>>>>>>> *Fix: *Positive and negative scenarios was added to check the >>>>>>>>>>> colour >>>>>>>>>>> for the Nimbus LAF foreground and background colours. >>>>>>>>>>> >>>>>>>>>>> With Regards, >>>>>>>>>>> Avik Niyogi >> >
