Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732

2019-02-14 Thread Shashidhara Veerabhadraiah
Hi Prasanta, The parameters for the function does not tells the order and is not implied by the way function definition is and more over this function is called in an interval of minimum refresh time(4 mins). Rather than adding overhead of confusion since it is not implied by the function I

Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732

2019-02-14 Thread Prasanta Sadhukhan
Hi Shashi, In this case, we know doCompare() is called from one location only [and also is not a public method to be called by user] and the check I mentioned is redundant. It's in while(true) loop so any optimzation we can do to avoid unnecessary checks will be good in my opinion. If

Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732

2019-02-14 Thread Shashidhara Veerabhadraiah
Hi Prasanta, doCompare(str1, str2) is a different function and one should not define a function based on how it is going to be called!! What if someone changed the caller to this: doCompare(currentRemotePrinters, prevRemotePrinters). There is no restriction if one calls like this. Here the

Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732

2019-02-14 Thread Prasanta Sadhukhan
Hi Shashi, I have one comment: doCompare(prevRemotePrinters, currentRemotePrinters) is only called from run() method when "prevRemotePrinters" is already checked to be not null [if (prevRemotePrinters != null && prevRemotePrinters.length > 0) ] so I see no point in checking "str1" [which is

Re: [OpenJDK 2D-Dev] [12] Review Request: 8213110 Remove the use of applets in automatic tests

2019-02-14 Thread Sergey Bylokhov
On 30/01/2019 14:53, Phil Race wrote: Oh .. and I meant to ask what probably is an obvious question but needs to be asked anyway. You say you did not try to fix tests that already fail - this is just a conversion - but did you confirm there are no NEW failures on any platform ? Yes, there

Re: [OpenJDK 2D-Dev] [12] Review Request: 8213110 Remove the use of applets in automatic tests

2019-02-14 Thread Sergey Bylokhov
Hi, Phil. Thank you for review, see my comments inline: You discuss the life cycle of an applet in jtreg ---   - Call init() and start() methods of the Applet   - Waits for 2 seconds   - Call stop() and destroy() methods of the Applet   - Dispose the frame -- I see init() and

Re: [OpenJDK 2D-Dev] RFR: JDK-8218682 + JDK-8198411: [TEST_BUG] DashOffset fails in mach5 + Two java2d tests are unstable in mach5

2019-02-14 Thread Sergey Bylokhov
On 14/02/2019 13:02, Alexey Ivanov wrote: BTW do we sure that the usage of IndexColorModel is not a bug? I don't think it's a bug. Likely Windows Server OS is installed in Server Code mode which has limited GUI support. I was able to get IndexColorModel on my desktop by something like this:

Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732

2019-02-14 Thread Phil Race
+1 .. remember to use the current bug synopsis in the push comment ie : "[Windows] Exception if no printers are installed." -phil. On 2/11/19 12:39 AM, Shashidhara Veerabhadraiah wrote: Hi Phil, Here is the new Webrev fixing those comments:

Re: [OpenJDK 2D-Dev] RFR: JDK-8218682 + JDK-8198411: [TEST_BUG] DashOffset fails in mach5 + Two java2d tests are unstable in mach5

2019-02-14 Thread Alexey Ivanov
On 14/02/2019 16:04, Sergey Bylokhov wrote: It looks fine. Thank you, Sergey! BTW do we sure that the usage of IndexColorModel is not a bug? I don't think it's a bug. Likely Windows Server OS is installed in Server Code mode which has limited GUI support. Regards, Alexey On

Re: [OpenJDK 2D-Dev] RFR 8218854: FontMetrics.getMaxAdvance may be less than the maximum FontMetrics.charWidth

2019-02-14 Thread Martin Balao
Hi Phil, Thanks for your feedback. I proposed the font in the test case because it's licensed under GNU Affero General Public License, Version 3 (AGPL) with an exemption [2]. My understanding was that we were able to ship it. Anyways, I removed the font and use system installed fonts. On newer

Re: [OpenJDK 2D-Dev] RFR 8218854: FontMetrics.getMaxAdvance may be less than the maximum FontMetrics.charWidth

2019-02-14 Thread Philip Race
Unfortunately we can't check in the font. That would require us to obtain a 3rd party code license and to update the font regularly as new versions are created. Way too onerous. Perhaps instead you can just run through installed fonts ? Can you also add a comment that explains that the values

Re: [OpenJDK 2D-Dev] RFR: JDK-8218682 + JDK-8198411: [TEST_BUG] DashOffset fails in mach5 + Two java2d tests are unstable in mach5

2019-02-14 Thread Sergey Bylokhov
It looks fine. BTW do we sure that the usage of IndexColorModel is not a bug? On 14/02/2019 06:07, Alexey Ivanov wrote: Hi Sergey, Do you have any comments for the latest webrev: http://cr.openjdk.java.net/~aivanov/8218682-8198411/webrev.01/ Do I push the fix? Regards, Alexey On 12/02/2019

Re: [OpenJDK 2D-Dev] RFR: JDK-8218682 + JDK-8198411: [TEST_BUG] DashOffset fails in mach5 + Two java2d tests are unstable in mach5

2019-02-14 Thread Alexey Ivanov
Hi Sergey, Do you have any comments for the latest webrev: http://cr.openjdk.java.net/~aivanov/8218682-8198411/webrev.01/ Do I push the fix? Regards, Alexey On 12/02/2019 18:33, Phil Race wrote: +1 -phil. On 2/12/19 6:24 AM, Alexey Ivanov wrote: Hi Phil, On 11/02/2019 18:32, Phil Race