+1
-phil.
On 07/19/2018 05:33 AM, Mario Torre wrote:
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
