Re: [OpenJDK 2D-Dev] Review request for 8024767: [TEST] need test to cover JDK-7189452
Hello Alexander, the test looks fine to me. Regarding explicit activation of xrender pipeline, there are three tests which does the same thing: test/sun/java2d/XRenderBlitsTest.java test/sun/java2d/pipe/InterpolationQualityTest.java test/sun/java2d/AcceleratedXORModeTest.java Probably, they have to be modified in order to avoid false alarms. Thanks, Andrew On 10/29/2013 6:37 PM, alexander stepanov wrote: Please see the updated webrev: http://cr.openjdk.java.net/~bae/8024767/webrev.01/ Regards, Alexander On 29.10.2013 16:48, alexander stepanov wrote: Hello Phil, OK, thanks, I'll remove the redundant options. Regards, Alexander On 29.10.2013 2:03, Phil Race wrote: Sorry for the delay in replying to this but I don't think you should specify these options * @run main/othervm -Dsun.java2d.xrender=True TextRenderingTest 1 - othervm is frowned on these days as it slows down testing 2. If the reason you are using it is to pass in the VM option, then its not right here. You are forcing the (platform specific) pipeline to be on against the better judgement of the pipeline. The test is only valid if the pipeline decides its valid. -phil. On 9/13/2013 5:58 AM, Andrew Brygin wrote: Hi Alexander, the test looks fine to me. Thanks, Andrew On 9/13/2013 1:20 PM, alexander stepanov wrote: Hello, could you please review the fix for the following [TEST] bug: https://bugs.openjdk.java.net/browse/JDK-8024767 (sorry, the bug info isn't available at bugs.sun.com yet). The webrev corresponding: http://cr.openjdk.java.net/~bae/8024767/webrev.00/ Just a new test added; no other code affected. The patch contains a simple automated test (check if source offset for text rendering is handled correctly). Thanks. Regards, Alexander
Re: [OpenJDK 2D-Dev] Review request for 8024767: [TEST] need test to cover JDK-7189452
Hi Alexander, > Please see the updated webrev: > http://cr.openjdk.java.net/~bae/8024767/webrev.01/ The fix looks fine to me. Thanks for creating the test-case :) - Clemens
Re: [OpenJDK 2D-Dev] Review request for 8024767: [TEST] need test to cover JDK-7189452
Please see the updated webrev: http://cr.openjdk.java.net/~bae/8024767/webrev.01/ Regards, Alexander On 29.10.2013 16:48, alexander stepanov wrote: Hello Phil, OK, thanks, I'll remove the redundant options. Regards, Alexander On 29.10.2013 2:03, Phil Race wrote: Sorry for the delay in replying to this but I don't think you should specify these options * @run main/othervm -Dsun.java2d.xrender=True TextRenderingTest 1 - othervm is frowned on these days as it slows down testing 2. If the reason you are using it is to pass in the VM option, then its not right here. You are forcing the (platform specific) pipeline to be on against the better judgement of the pipeline. The test is only valid if the pipeline decides its valid. -phil. On 9/13/2013 5:58 AM, Andrew Brygin wrote: Hi Alexander, the test looks fine to me. Thanks, Andrew On 9/13/2013 1:20 PM, alexander stepanov wrote: Hello, could you please review the fix for the following [TEST] bug: https://bugs.openjdk.java.net/browse/JDK-8024767 (sorry, the bug info isn't available at bugs.sun.com yet). The webrev corresponding: http://cr.openjdk.java.net/~bae/8024767/webrev.00/ Just a new test added; no other code affected. The patch contains a simple automated test (check if source offset for text rendering is handled correctly). Thanks. Regards, Alexander
Re: [OpenJDK 2D-Dev] Review request for 8024767: [TEST] need test to cover JDK-7189452
Hello Phil, OK, thanks, I'll remove the redundant options. Regards, Alexander On 29.10.2013 2:03, Phil Race wrote: Sorry for the delay in replying to this but I don't think you should specify these options * @run main/othervm -Dsun.java2d.xrender=True TextRenderingTest 1 - othervm is frowned on these days as it slows down testing 2. If the reason you are using it is to pass in the VM option, then its not right here. You are forcing the (platform specific) pipeline to be on against the better judgement of the pipeline. The test is only valid if the pipeline decides its valid. -phil. On 9/13/2013 5:58 AM, Andrew Brygin wrote: Hi Alexander, the test looks fine to me. Thanks, Andrew On 9/13/2013 1:20 PM, alexander stepanov wrote: Hello, could you please review the fix for the following [TEST] bug: https://bugs.openjdk.java.net/browse/JDK-8024767 (sorry, the bug info isn't available at bugs.sun.com yet). The webrev corresponding: http://cr.openjdk.java.net/~bae/8024767/webrev.00/ Just a new test added; no other code affected. The patch contains a simple automated test (check if source offset for text rendering is handled correctly). Thanks. Regards, Alexander
Re: [OpenJDK 2D-Dev] Review request for 8024767: [TEST] need test to cover JDK-7189452
Sorry for the delay in replying to this but I don't think you should specify these options * @run main/othervm -Dsun.java2d.xrender=True TextRenderingTest 1 - othervm is frowned on these days as it slows down testing 2. If the reason you are using it is to pass in the VM option, then its not right here. You are forcing the (platform specific) pipeline to be on against the better judgement of the pipeline. The test is only valid if the pipeline decides its valid. -phil. On 9/13/2013 5:58 AM, Andrew Brygin wrote: Hi Alexander, the test looks fine to me. Thanks, Andrew On 9/13/2013 1:20 PM, alexander stepanov wrote: Hello, could you please review the fix for the following [TEST] bug: https://bugs.openjdk.java.net/browse/JDK-8024767 (sorry, the bug info isn't available at bugs.sun.com yet). The webrev corresponding: http://cr.openjdk.java.net/~bae/8024767/webrev.00/ Just a new test added; no other code affected. The patch contains a simple automated test (check if source offset for text rendering is handled correctly). Thanks. Regards, Alexander
Re: [OpenJDK 2D-Dev] Review request for 8024767: [TEST] need test to cover JDK-7189452
Hi Alexander, the test looks fine to me. Thanks, Andrew On 9/13/2013 1:20 PM, alexander stepanov wrote: Hello, could you please review the fix for the following [TEST] bug: https://bugs.openjdk.java.net/browse/JDK-8024767 (sorry, the bug info isn't available at bugs.sun.com yet). The webrev corresponding: http://cr.openjdk.java.net/~bae/8024767/webrev.00/ Just a new test added; no other code affected. The patch contains a simple automated test (check if source offset for text rendering is handled correctly). Thanks. Regards, Alexander
