Merging in more AWT stuff

[Bryce McKinlay]

"Rolf W. Rasmussen" wrote:

> This patch merges in most of the AWT code I've been working on.  With
> this patch, and the Xlib-based toolkit implementation I've also been
> working on I've been able to get my AWT test programs up and running,
> The patch implements missing parts of some AWT classes, and also fixes
> some bugs that became apparent when the windows and components
> actually started appearing on the screen.

Cool!! Assuming Tom has no objections, please go ahead and check this in, after you've
addressed my comments below.

thanks

  [ bryce ]

> +
> +  /**
> +   * 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)?

>
> +    if (comp == null) return DIM_0;

All of the "if" statements like this should  be split across two lines. For example:

if (comp == null)
  return DIM_0;

>    public void addNotify ()
>    {
>      if (peer == null)
>        peer = (ComponentPeer) getToolkit ().createButton (this);
> +    super.addNotify();
>    }

Why call super.addNotify() here? It doesnt seem to be useful.

>    public void setBounds(int x, int y, int w, int h)
>    {
> +    if ((this.x == x) && (this.y == y) &&
> +       (this.width == w) && (this.height == h)) return;

Where conditions are larger than one line, the operators should go at the start of the
line, and there shouldn't be more than one condition (of the same nesting level) per line.
This statement should look something like:

if (this.x == x
    && this.y == y
    && this.width == w
    && this.height == h)
  return;

>
>    /** Forward event to the appropriate processXXXEvent method based on the
>      * event type.
>      */
>    protected void processEvent(AWTEvent e)
>    {
> -    if (e instanceof ComponentEvent)
> -      processComponentEvent((ComponentEvent) e);
> -    else if (e instanceof FocusEvent)
> +
> +    /* Note: the order of these if statements are
> +       important. Subclasses must be checked first. Eg. MouseEvent
> +       must be checked before ComponentEvent, since a MouseEvent
> +       object is also an instance of a ComponentEvent. */

Doh! ;-) Good catch!

> +  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?

> 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?

>  }
> 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;


> Index: libjava/java/awt/Window.java
> ===================================================================
>    public void addNotify()
> @@ -78,6 +103,7 @@
>      if (peer == null)
>        // FIXME: This cast should NOT be required. ??? Compiler bug ???
>        peer = (ComponentPeer) getToolkit ().createWindow (this);
> +    super.addNotify ();
>    }

Again, why the call into super?

regards

  [ bryce ]