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