[OpenJDK 2D-Dev] [14] Review request for 8227392: Colors with alpha are painted incorrectly on Linux, after JDK-8214579
Phil Race
philip.race at oracle.com
Mon Jul 15 19:31:30 UTC 2019
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