[OpenJDK 2D-Dev] [14] Review request for 8227392: Colors with alpha are painted incorrectly on Linux, after JDK-8214579
Prasanta Sadhukhan
prasanta.sadhukhan at oracle.com
Tue Jul 16 09:10:55 UTC 2019
+1
Regards
Prasanta
On 16-Jul-19 1:01 AM, Phil Race wrote:
>
> Looks good!
>
> -phil.
>
> On 7/15/19 10:26 AM, Anton Litvinov wrote:
>> Hello Phil,
>>
>> Thank you for the reply. The second version of the fix with the
>> described in the previous e-mail workaround for the regression test
>> is created. Could you please look at the second version of the fix.
>>
>> The 2nd version of the fix:
>> http://cr.openjdk.java.net/~alitvinov/8227392/jdk13/webrev.01
>>
>> Thank you,
>> Anton
>>
>> On 12/07/2019 19:42, Phil Race wrote:
>>> > Would such a workaround be acceptable as part of this back out fix?
>>>
>>> Yes, very much so !
>>>
>>> -phil.
>>>
>>> On 7/12/19 11:34 AM, Anton Litvinov wrote:
>>>> Hello Phil,
>>>>
>>>> I tried to apply your code and can confirm that value of "color" is
>>>> always the same for the case, when the test passes and fails. For
>>>> each test run the "paint(Graphics)" method is called either 2 or 3
>>>> times and this tracing output is always the same and following:
>>>> "color=java.awt.Color[r=255,g=255,b=255] alpha=127"
>>>>
>>>> My today's experiments show that this intermittent failure is
>>>> reproducible with builds prior to the builds with the fix for
>>>> 8214579 and this is also reproducible on Oracle Linux 7.4 x64. I
>>>> was able to reproduce in the following environment configurations:
>>>>
>>>> 1. Oracle Linux 7.4 x64 with JDK 13+23 (the build without the fix
>>>> 8214579 which was integrated in JDK 13+24)
>>>> 2. openSUSE Leap 15.0 with JDK 13+23, JDK 11+25 (build in which the
>>>> test itself was introduced), JDK 11+28 (GA build).
>>>>
>>>> I attached a screenshot with the test failure and wrote a comment
>>>> in the bug (https://bugs.openjdk.java.net/browse/JDK-8224825).
>>>>
>>>> MY OBSERVATION:
>>>>
>>>> Although I have not been able to identify the root cause of the
>>>> intermittent test failure, I think that the wrong color is got,
>>>> when the black color, which is set as the background for the frame,
>>>> is not participating in color blending.
>>>>
>>>> THE DISCOVERED WORKAROUND:
>>>>
>>>> I discovered that if the following workaround is applied to the
>>>> regression test "test/jdk/java/awt/Color/AlphaColorTest.java", then
>>>> the intermittent test failure disappears (is not reproducible, when
>>>> test is run 20 times in a row). Also I verified with the workaround
>>>> the test fails as it should with JDK 11+24 (the build without the
>>>> fix 8204931 which was integrated in JDK 11+25 and introduced the
>>>> test). The workaround is following.
>>>>
>>>> Substitute the next 2 code lines:
>>>>
>>>> 77 frame = new Frame("Alpha Color Test");
>>>> 78 frame.setBackground(Color.black);
>>>>
>>>> for the next 8 code lines:
>>>>
>>>> frame = new Frame("Alpha Color Test") {
>>>> @Override
>>>> public void paint(Graphics g) {
>>>> g.setColor(Color.black);
>>>> g.fillRect(0, 0, getWidth(), getHeight());
>>>> super.paint(g);
>>>> }
>>>> };
>>>>
>>>> Would such a workaround be acceptable as part of this back out fix?
>>>>
>>>> Thank you,
>>>> Anton
>>>>
>>>> On 11/07/2019 19:48, Phil Race wrote:
>>>>> I don't think we ever saw it fail until 8214579 was pushed.
>>>>> I don't know the reason for the color you are sporadically getting
>>>>> in VB
>>>>> Maybe it is slow and the paint event is being missed ?
>>>>> But then I'd expect black .. not light grey.
>>>>>
>>>>> Still, it might be informative to put a print statement in here
>>>>> (in the test)
>>>>> public void paint(Graphics g) {
>>>>> System.out.println("color=" + color + " alpha=" +
>>>>> color.getAlpha()); // << add this
>>>>> g.setColor(color);
>>>>> g.fillRect(0, 0, getSize().width, getSize().height);
>>>>> }
>>>>>
>>>>> and compare what is printed for failing and passing runs.
>>>>> If you can't figure it out quickly, then update the test to not
>>>>> exit so quickly and grab a screen
>>>>> shot of the failure and add it to the bug.
>>>>>
>>>>>
>>>>> -phil.
>>>>>
>>>>> On 7/11/19 11:13 AM, Anton Litvinov wrote:
>>>>>> Hello Phil,
>>>>>>
>>>>>> Thank you for these additional details. I have tried to run the
>>>>>> test "open/test/jdk/java/awt/Color/AlphaColorTest.java" with JDK
>>>>>> 13 compiled from "jdk/jdk13" with/without this back out fix and
>>>>>> the results are not so stable on my VirtualBox host with openSUSE
>>>>>> Leap 15.0 OS (GNOME 3.26.2):
>>>>>>
>>>>>> 1. "AlphaColorTest.java" stably fails with JDK 13 without this
>>>>>> back out fix and the test failure message is stably:
>>>>>> "Color is not as expected. Got
>>>>>> java.awt.Color[r=255,g=255,b=255]"
>>>>>>
>>>>>> 2. "AlphaColorTest.java" starts passing with JDK 13 with the back
>>>>>> out fix, but continues to intermittently fail with the failure
>>>>>> message:
>>>>>> "Color is not as expected. Got
>>>>>> java.awt.Color[r=189,g=189,b=189]"
>>>>>>
>>>>>> It looks that either this regression test had been failing
>>>>>> intermittently before the fix 8214579, which we are backing out,
>>>>>> was integrated, or some other fix causes this intermittent failure.
>>>>>>
>>>>>> Maybe in this case it is better not to change this
>>>>>> "open/test/jdk/ProblemList.txt" as well as the test itself
>>>>>> "open/test/jdk/java/awt/Color/AlphaColorTest.java" as part of
>>>>>> this back out fix for JCK test failure?
>>>>>>
>>>>>> Thank you,
>>>>>> Anton
>>>>>>
>>>>>> On 11/07/2019 17:38, Philip Race wrote:
>>>>>>> There is a regression test that is supposed to catch this exact
>>>>>>> problem.
>>>>>>>
>>>>>>> So I had looked into how we did not catch this earlier and found
>>>>>>> that in fact we did.
>>>>>>> This was originally found and filed as
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8224825
>>>>>>> The test java/awt/Color/AlphaColorTest.java was then problem
>>>>>>> listed on Linux.
>>>>>>>
>>>>>>> $ grep AlphaColorTest test/jdk/ProblemList.txt
>>>>>>> java/awt/Color/AlphaColorTest.java 8224825 linux-all
>>>>>>>
>>>>>>> So I think we can close JDK-8224825 as a duplicate of this as
>>>>>>> well as updating the problem list
>>>>>>> and the test - after confirming that this backout resolves that
>>>>>>> as I expect it will.
>>>>>>>
>>>>>>> -phil.
>>>>>>>
>>>>>>> On 7/11/19, 9:20 AM, Anton Litvinov wrote:
>>>>>>>> By your request regenerated the webrev specifically against
>>>>>>>> (http://hg.openjdk.java.net/jdk/jdk13) repository.
>>>>>>>> JDK 13 specific webrev:
>>>>>>>> http://cr.openjdk.java.net/~alitvinov/8227392/jdk13/webrev.00
>>>>>>>>
>>>>>>>> I confirm again, that 2 failing manual tests: "ColorTest0003",
>>>>>>>> "ColorTest0004" from the test
>>>>>>>> "api/java_awt/interactive/ColorTests.html" in JCK 13 do not
>>>>>>>> fail anymore after this fix is applied to JDK 13 compiled from
>>>>>>>> "jdk/jdk13" repository.
>>>>>>>>
>>>>>>>> "jdk/jdk13" repository does not contain any problem list
>>>>>>>> mentioning "api/java_awt/interactive/ColorTests.html" test from
>>>>>>>> JCK 13, thus I cannot de-problem list it and have no idea,
>>>>>>>> where it is problem-listed. I also doubt that it is
>>>>>>>> problem-listed, since it is manual. I added "noreg-jck" label
>>>>>>>> to the bug in JBS deliberately according to OpenJDK process
>>>>>>>> (Section #6 from the web page
>>>>>>>> (http://openjdk.java.net/guide/changePlanning.html)), because
>>>>>>>> this back out fix does not contain a new separate regression
>>>>>>>> test, while this regression can be checked by running existing
>>>>>>>> mentioned above test from JCK package.
>>>>>>>>
>>>>>>>> If this "noreg-jck" label creates problems, I can remove it at
>>>>>>>> all.
>>>>>>>>
>>>>>>>> Thank you,
>>>>>>>> Anton
>>>>>>>>
>>>>>>>> On 11/07/2019 15:51, Philip Race wrote:
>>>>>>>>> One more thing I just realised you should do here is
>>>>>>>>> de-problem list
>>>>>>>>> the regression test that fails ... after verifying it now works,
>>>>>>>>> updating it with this bug ID.
>>>>>>>>> And you can remove that noreg-jck label as a consequence.
>>>>>>>>>
>>>>>>>>> -phil.
>>>>>>>>>
>>>>>>>>> On 7/11/19, 7:35 AM, Philip Race wrote:
>>>>>>>>>> In such cases I like more than to be told "it would apply
>>>>>>>>>> cleanly" but also to see
>>>>>>>>>> that you actually prepared the webrev against 13. This is
>>>>>>>>>> more certain
>>>>>>>>>> and ensures that when the fix is approved you don't
>>>>>>>>>> accidentally push it
>>>>>>>>>> to the wrong repo. You have to go clone 13 and apply the
>>>>>>>>>> patch there anyway ...
>>>>>>>>>>
>>>>>>>>>> -phil.
>>>>>>>>>>
>>>>>>>>>> On 7/11/19, 5:44 AM, Anton Litvinov wrote:
>>>>>>>>>>> Hello Phil,
>>>>>>>>>>>
>>>>>>>>>>> Thank you for review and the important remark about the need
>>>>>>>>>>> to work with "jdk/jdk13" stabilization repository, I forgot
>>>>>>>>>>> about this feature of post RDP 1 phase. Today I checked the
>>>>>>>>>>> fix against "jdk/jdk13" repository and confirm that it
>>>>>>>>>>> resolves this bug for JDK 13.
>>>>>>>>>>>
>>>>>>>>>>> Will wait for feedback or approval from any other second
>>>>>>>>>>> code reviewer.
>>>>>>>>>>>
>>>>>>>>>>> Thank you,
>>>>>>>>>>> Anton
>>>>>>>>>>>
>>>>>>>>>>> On 10/07/2019 19:55, Phil Race wrote:
>>>>>>>>>>>> Anton,
>>>>>>>>>>>>
>>>>>>>>>>>> This looks fine except that it needs to be prepared against
>>>>>>>>>>>> 13, and then pushed there, not 14.
>>>>>>>>>>>> And it will get forwarded synced from 13 to 14. That is the
>>>>>>>>>>>> RDP process ..
>>>>>>>>>>>>
>>>>>>>>>>>> -phil.
>>>>>>>>>>>>
>>>>>>>>>>>> On 7/10/19 5:00 AM, Anton Litvinov wrote:
>>>>>>>>>>>>> Hello,
>>>>>>>>>>>>>
>>>>>>>>>>>>> Could you please review the following fix for the bug. The
>>>>>>>>>>>>> fix is backing out of the fix for the bug JDK-8214579
>>>>>>>>>>>>> which caused this JCK test failure. If this fix is
>>>>>>>>>>>>> accepted, then a new separate bug for readdressing the bug
>>>>>>>>>>>>> reported in JDK-8214579 will be filed.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8227392
>>>>>>>>>>>>> Webrev:
>>>>>>>>>>>>> http://cr.openjdk.java.net/~alitvinov/8227392/jdk14/webrev.00
>>>>>>>>>>>>> Changeset of JDK-8214579:
>>>>>>>>>>>>> http://hg.openjdk.java.net/jdk/client/rev/c53905e7dc57
>>>>>>>>>>>>>
>>>>>>>>>>>>> Thank you,
>>>>>>>>>>>>> Anton
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
More information about the 2d-dev
mailing list