Hi Phil, Semyon, Didn't see this land originally. Thanks for the sponsorship.
Will try and get this applied to downstream Ubuntu builds. Thanks Michael On 14 November 2017 at 08:20, Semyon Sadetsky <semyon.sadet...@oracle.com> wrote: > +1 > > --Semyon > > > On 11/09/2017 10:50 AM, Phil Race wrote: >> >> Protocol is that there first be a bug report and then a "RFR" (request for >> review) is sent >> to the list referencing the bug + the fix >> >> I have filed https://bugs.openjdk.java.net/browse/JDK-8191041 >> >> and verified that it works. So we can perhaps short-circuit that this time >> .. but not next time .. >> >> As to the blueprints path it looks safer to leave it since I don't know >> how to test it >> so I would be happy to commit the patch as is but I think this opinion >> needs to be >> reviewed by someone else on the list first .. >> >> -phil. >> >> On 11/03/2017 09:53 PM, Michael D wrote: >>> >>> Apologies, noticed I broke the patch between testing and submission. >>> >>> Correct and re-tested version >>> >>> >>> --- >>> a/src/java.desktop/share/classes/com/sun/java/swing/plaf/gtk/GTKGraphicsUtils.java >>> +++ >>> b/src/java.desktop/share/classes/com/sun/java/swing/plaf/gtk/GTKGraphicsUtils.java >>> @@ -47,34 +47,17 @@ class GTKGraphicsUtils extends SynthGraphicsUtils { >>> return; >>> } >>> int componentState = context.getComponentState(); >>> - if ((componentState & SynthConstants.DISABLED) == >>> - SynthConstants.DISABLED){ >>> - if (!GTKLookAndFeel.is3()) { >>> - Color orgColor = g.getColor(); >>> - g.setColor(context.getStyle().getColor(context, >>> - GTKColorType.WHITE)); >>> - x += 1; >>> - y += 1; >>> - super.paintText(context, g, text, x, y, mnemonicIndex); >>> >>> - g.setColor(orgColor); >>> - x -= 1; >>> - y -= 1; >>> - } >>> - super.paintText(context, g, text, x, y, mnemonicIndex); >>> - } >>> - else { >>> - String themeName = GTKLookAndFeel.getGtkThemeName(); >>> - if (themeName != null && themeName.startsWith("blueprint") >>> && >>> - shouldShadowText(context.getRegion(), componentState)) { >>> + String themeName = GTKLookAndFeel.getGtkThemeName(); >>> + if (themeName != null && themeName.startsWith("blueprint") && >>> + shouldShadowText(context.getRegion(), componentState)) { >>> >>> - g.setColor(Color.BLACK); >>> - super.paintText(context, g, text, x+1, y+1, >>> mnemonicIndex); >>> - g.setColor(Color.WHITE); >>> - } >>> - >>> - super.paintText(context, g, text, x, y, mnemonicIndex); >>> + g.setColor(Color.BLACK); >>> + super.paintText(context, g, text, x+1, y+1, mnemonicIndex); >>> + g.setColor(Color.WHITE); >>> } >>> + >>> + super.paintText(context, g, text, x, y, mnemonicIndex); >>> } >>> >>> /** >>> >>> On 4 November 2017 at 07:13, Michael D <m...@md-5.net> wrote: >>>> >>>> My bad, >>>> >>>> Patch just looks like this (bit messy because of indent change). >>>> Thanks >>>> Michael >>>> >>>> >>>> >>>> >>>> --- >>>> a/src/java.desktop/share/classes/com/sun/java/swing/plaf/gtk/GTKGraphicsUtils.java >>>> +++ >>>> b/src/java.desktop/share/classes/com/sun/java/swing/plaf/gtk/GTKGraphicsUtils.java >>>> @@ -46,35 +46,17 @@ class GTKGraphicsUtils extends SynthGraphicsUtils { >>>> // ignore this. >>>> return; >>>> } >>>> - int componentState = context.getComponentState(); >>>> - if ((componentState & SynthConstants.DISABLED) == >>>> - SynthConstants.DISABLED){ >>>> - if (!GTKLookAndFeel.is3()) { >>>> - Color orgColor = g.getColor(); >>>> - g.setColor(context.getStyle().getColor(context, >>>> - GTKColorType.WHITE)); >>>> - x += 1; >>>> - y += 1; >>>> - super.paintText(context, g, text, x, y, mnemonicIndex); >>>> >>>> - g.setColor(orgColor); >>>> - x -= 1; >>>> - y -= 1; >>>> - } >>>> - super.paintText(context, g, text, x, y, mnemonicIndex); >>>> - } >>>> - else { >>>> - String themeName = GTKLookAndFeel.getGtkThemeName(); >>>> - if (themeName != null && themeName.startsWith("blueprint") >>>> && >>>> - shouldShadowText(context.getRegion(), componentState)) >>>> { >>>> + String themeName = GTKLookAndFeel.getGtkThemeName(); >>>> + if (themeName != null && themeName.startsWith("blueprint") && >>>> + shouldShadowText(context.getRegion(), componentState)) { >>>> >>>> - g.setColor(Color.BLACK); >>>> - super.paintText(context, g, text, x+1, y+1, >>>> mnemonicIndex); >>>> - g.setColor(Color.WHITE); >>>> - } >>>> - >>>> - super.paintText(context, g, text, x, y, mnemonicIndex); >>>> + g.setColor(Color.BLACK); >>>> + super.paintText(context, g, text, x+1, y+1, mnemonicIndex); >>>> + g.setColor(Color.WHITE); >>>> } >>>> + >>>> + super.paintText(context, g, text, x, y, mnemonicIndex); >>>> } >>>> >>>> /** >>>> >>>> On 4 November 2017 at 06:39, Philip Race <philip.r...@oracle.com> wrote: >>>>> >>>>> Yes these mailing lists strip attachements >>>>> So an in-line patch .. or send directly to one of us to host it for you >>>>> on >>>>> cr.openjdk.java.net. >>>>> >>>>> -phil. >>>>> >>>>> On 11/3/17, 12:34 PM, Sergey Bylokhov wrote: >>>>>> >>>>>> Hi, Michael. >>>>>> It seems that the attached patch was removed from the email, can you >>>>>> please provide a link to it or inline in email. >>>>>> Thank you. >>>>>> >>>>>> On 03/11/2017 03:09, Michael D wrote: >>>>>>> >>>>>>> On 3 November 2017 at 21:07, Michael D <m...@md-5.net> wrote: >>>>>>>> >>>>>>>> Hi All, >>>>>>>> >>>>>>>> Last couple of days I've been trying to root out this bug - >>>>>>>> https://bugs.launchpad.net/ubuntu/+source/openjdk-8/+bug/1729558 >>>>>>>> which >>>>>>>> has resulted in poor looking rendering on my machine. >>>>>>>> It turns out that the cause of this behavior is that Swing tries to >>>>>>>> do >>>>>>>> its own magic disabled item rendering by turning the text white and >>>>>>>> shifting it a pixel in each direction. I have no doubt there was >>>>>>>> probably a good reason for this, however the file has remained >>>>>>>> unchanged since its initial import in 2007! (Maybe someone from >>>>>>>> Oracle >>>>>>>> can find out more) >>>>>>>> Removing this code (patch attached) makes disabled menu items render >>>>>>>> correctly in both light and dark GTK themes. Its completely trivial, >>>>>>>> but I've nonetheless provided the patch for context. There is also >>>>>>>> some adjacent code that references a metacity theme called >>>>>>>> "blueprint" >>>>>>>> that also seems pre 2007 era, but I've left that in as I have no >>>>>>>> idea >>>>>>>> of its context. >>>>>>>> >>>>>>>> Appreciate if this change / bug could be reviewed & discussed. In >>>>>>>> the >>>>>>>> meantime I will also forward it downstream to Ubuntu as this is >>>>>>>> where >>>>>>>> I encountered it / affects my usage. >>>>>>>> >>>>>>>> Thanks >>>>>>>> Michael >>>>>> >>>>>> >>>>>> >> >
