Merging in more AWT stuff

Rolf W. Rasmussen rolfwr@ii.uib.no
Tue Aug 15 09:25:00 GMT 2000


On Mon, Aug 14, 2000 at 04:37:35PM +1200, Bryce McKinlay wrote:
> >[...]
> Cool!! Assuming Tom has no objections, please go ahead and check this in, after you've
> addressed my comments below.

I'll do that.

> > +  /**
> > +   * Completely disregards the requested maximum sizes of the
> > +   * components, and states that the container has no upper size
> > +   * limit.
> > +   *
> > +   * @return a dimension of width and height Integer.MAX_VALUE.
> > +   */
> > +  public Dimension maximumLayoutSize(Container target)
> > +  {
> > +    return (Dimension) DIM_MAX.clone();
> > +  }
> 
> Is this really how BorderLayout is supposed to behave? Shouldn't it take into account the
> getMaximumSize() values of the child components? Or is this just an interim implementation
> (in which case a FIXME should be added)?

I believe the implementation is correct.

In all the tests I've done, the Sun implementation returns
java.awt.Dimension[width=2147483647,height=2147483647].

[...formatting comments...]

I'll go through the code and make sure it's formatted properly. I 
sometimes slip up when I'm going back and forth between projects with 
different coding conventions.

>[...] 
> > +  private void visitChildren(Graphics gfx, GfxVisitor visitor,
> > +                    boolean lightweightOnly)
> >
> > +  private void visitChild(Graphics gfx, GfxVisitor visitor,
> > +                         boolean lightweightOnly, Component comp)
> 
> > +abstract class GfxVisitor
> 
> Could you add some commentry to explain the purpose of ths visitor mechanism, and how it
> works?

I'll do that.

> > Index: libjava/java/awt/Graphics.java
> > ===================================================================
> >
> > +  public void draw3DRect(int x, int y, int width, int height,
> > +                        boolean raised)
> > +  {
> > +    Color color = getColor();
> > +    Color tl = color; // FIXME, make lighter
> > +    Color br = color; // FIXME, make darker
> 
> Can't we just call Color.darker() / Color.brighter() here?

Oops, forgot about that one. The draw3DRect() code predates the 
brighter()/darker() code. I'll fix it.

> > Index: libjava/java/awt/Image.java
> > ===================================================================
> >  public abstract class Image extends Object
> >  {
> > -  public Image()
> > +  public static final Object UndefinedProperty;
> > +
> > +  public static final int SCALE_DEFAULT        =  1;
> > +  public static final int SCALE_FAST           =  2;
> > +  public static final int SCALE_SMOOTH         =  4;
> > +  public static final int SCALE_REPLICATE      =  8;
> > +  public static final int SCALE_AREA_AVERAGING = 16;
> 
> I prefer to write these masking constants using the left-shift operator (I guess it makes
> it more clear that they are in fact to be used to form a bitmask). I also like to remove
> repetitions of the "public static final int" keywords. eg:
> 
> public static final int SCALE_DEFAULT        = 1 << 0,
>                         SCALE_FAST           = 1 << 1,
>                         SCALE_SMOOTH         = 1 << 2,
>                         SCALE_REPLICATE      = 1 << 3,
>                         SCALE_AREA_AVERAGING = 1 << 4;

I agree.

>[... calling super.addNotify() ...]
> Why call super.addNotify() here? It doesnt seem to be useful.
>[...]
> Again, why the call into super?

The chaining of addNotify() and removeNotify() has the same purpose as 
chaining constructors and finalize() methods; to allow each level of 
the inheritance hierachy to act upon the state change of the object. 
Chaining methods in this manner has sort of become an idiom.

Currently, the only reason the addNotify() method of the base 
class Component should be called, is to execute the following 
statement:

peer.setEventMask(eventMask);

...which ensures that the peer knows which events to deliver. This is 
something that needs to be done regardless of whether the actual 
component is a Button, a Window, or any other type of component.

But there are probably other things the addNotify() method of Component 
should do. One thing that comes to mind is notification of popup menus. 
Calls to addNotify() should be propagated to all children of the 
component. All AWT components can have PopupMenu instances as 
children, and these instances should be notified by calling their 
addNotify() method.

-- 
Rolf W. Rasmussen


More information about the Java-patches mailing list