Request for review: 7124530 What is background color of AWT component? (And foreground, for that matter) (original) (raw)

Alexander Potochkin Alexander.Potochkin at oracle.com
Wed Jan 11 08:30:23 PST 2012

Hello Sergey

okay, makes sense approved

Thanks alexp

Hi Alexander 29.12.2011 22:32, Alexander Potochkin wrote:

Hello Sergey

Hi Everyone, This is a fix for some glitches in the code for background/foreground/font properties. 1. SystemColor.window was changed to color which is used by default in swing l&f(Aqua). Changes in AquaImageFactory.java and CSystemColors.m. Now most of the awt components use this color as default. 2. set** methods were moved from LWWindowPeer to LWComponentPeer, because there is an issues when these methods has no effect, because repaint of the component does not happen.For example: - call Label.setbackground which set component background property - call Peer.setBackground - call Delegate.setBackgound - compare passed color with color from Label property, which was set in the first step: changes in LWWindowPeer,LWContainerPeer,LWComponentPeer. could you give more details on that? By default delegates uses colors from DelegateContainer which take color from targets. So if we set color for target(Label) -> then set color for peer ->then set color for delegete. this code does not repaint our component: public void setBackground(Color bg) { Color oldBg = getBackground(); super.setBackground(bg); if ((oldBg != null) ? !oldBg.equals(bg) : ((bg != null) && !bg.equals(oldBg))) { // background already bound in AWT1.2 repaint(); } } Because bg and oldBg will be the same color.

the pattern where every peer has its delegate which is responsible for storing/repainting for colors and fonts looks more laconic and consistent to me. This is true if all our components had delegates. But we has LWWindow which has no delegate and LWCanvas which the delegate serves only for storage of colors(By default LWComponentPeer has not delegates so we cannot simplify old code and eliminate null checks).Probably delegate from LWPanelPeer can be removed too. And i think it would be better not to have similar code in different classes. For example I believe this is better protected final Color getBackground() { synchronized (getStateLock()) { return background; } } Than 2 different methods in different classes: protected Color getBackground() { synchronized (getDelegateLock()) { D delegate = getDelegate(); if (delegate != null) { return delegate.getBackground(); } } return null; } protected Color getBackground() { synchronized (getStateLock()) { return background; } } same for setters public void setBackground(final Color c) { final Color oldBg = getBackground(); if (oldBg == c || (oldBg != null&& oldBg.equals(c))) { return; } synchronized (getStateLock()) { background = c; } final D delegate = getDelegate(); if (delegate != null) { synchronized (getDelegateLock()) { // delegate will repaint the target delegate.setBackground(c); } } else { repaintPeer(); } } against: public void setBackground(Color c) { final boolean changed; synchronized (getStateLock()) { changed = ((background != null) ? !background.equals(c) : ((c != null)&& !c.equals(background))); background = c; } if (changed) { repaintPeer(); } } public void setBackground(Color c) { synchronized (getDelegateLock()) { D delegate = getDelegate(); if (delegate != null) { // delegate will repaint the target delegate.setBackground(c); } } }

3. List default color was changed to SystemColor.text: changes in LWListPeer.java. LWListPeer set the font and colors to its view component, with the proposed fix it seems to update the scrollPane instead But in resetColorsAndFont() we clear default colors for view and it use colors from its parent ScrollableJList.

4. LWWIndow target color initialization was moved from CPlatformIndow to LWWindowPeer, so it can be reused later by CPlatformEmbeddedFrame . 5. unnecessary peers set** methods and unnecessary delegate in LWCanvasPeer were removed: changes in LWCanvasPeer.java, LWListPeer.java, LWPanelPeer.java. Bug: http://bugs.sun.com/bugdatabase/viewbug.do?bugid=7124530 Webrev can be found at: http://cr.openjdk.java.net/~serb/7124530/webrev.00/ Thanks alexp

More information about the macosx-port-dev mailing list