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

[Alexander Potochkin]

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/view_bug.do?bug_id=7124530
>>> Webrev can be found at: 
>>> http://cr.openjdk.java.net/~serb/7124530/webrev.00/
>>>
>>
>> Thanks
>> alexp
>
>