On Wed, Jul 18, 2018 at 11:28 PM, Phil Race <philip.r...@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 -- Mario Torre Associate Manager, Software Engineering Red Hat GmbH <https://www.redhat.com> 9704 A60C B4BE A8B8 0F30 9205 5D7E 4952 3F65 7898
