[OpenJDK 2D-Dev] Request for re-reviewl: Backport of JDK-8150954: Taking screenshots on x11 composite desktop produce wrong result

Phil Race philip.race at oracle.com
Thu Jul 19 20:43:44 UTC 2018


+1

-phil.

On 07/19/2018 05:33 AM, Mario Torre wrote:
> On Wed, Jul 18, 2018 at 11:28 PM, Phil Race <philip.race at oracle.com> wrote:
>> The review for the original fix was actually on awt-dev which probably was
>> correct
>> and so this should be there too.
> Hi Phil,
>
> Thanks for the review, and I apologise for posting it to 2d-dev, that
> was out of habit :)
>
>> I hadn't seen the thread so had to go read it .. but it was so long ago I'd
>> probably have had to re-read it anyway. But it was not so easy to find
>> since it did not have the bug ID in the subject !
>> http://mail.openjdk.java.net/pipermail/awt-dev/2016-March/010742.html
>> http://mail.openjdk.java.net/pipermail/awt-dev/2016-April/010996.html
>> http://mail.openjdk.java.net/pipermail/awt-dev/2016-May/011370.html
> Right, thanks again for linking the relevant thread information, I
> didn't post it as it was in the bug description, but indeed is a nice
> courtesy for the reviewer, I'll make sure to have it linked next time
> in the email request too.
>
>> Since it is changed, yes, being able to point to this review thread is
>> likely
>> something the 8u-dev maintainers would ask you for.
>>
>> It looks OK to me although I don't grok why the order here in 8u
>>
>> 302     if (isXCompositeDisplay(awt_display, adata->awt_visInfo.screen) &&
>>   303         hasXCompositeOverlayExtension(awt_display))
>>
>> is reversed from what you had in 9 :
>> 309         if (hasXCompositeOverlayExtension(awt_display) &&
>>   310             isXCompositeDisplay(awt_display,
>> adata->awt_visInfo.screen))
>>
> Eh.. This is because the current patch is based on what I had on the
> RPM, which is an older version of the patch I pushed for 9, before the
> additional gtk code, for some reason the line was inverted initially,
> I changed this in the 9 patch as it is indeed more logical. Thanks for
> spotting this as I completely missed it (good that we have reviews for
> backports too!).
>
> It's fixed in this new webrev:
>
> http://cr.openjdk.java.net/~neugens/8150954/webrev-jdk8u.01/jdk.patch
>
> Cheers,
> Mario
>



More information about the 2d-dev mailing list