Hi,

I've started to look deeper into AWT and how it should work and how it
actually works in Classpath. There are some issues that are plain wrong.
This patch fixes some of them and also contains some improvements with
the caching of layouts.

I have made some tests with the update() and paint() methods and found
that they actually work different than is documented in the spec. The
correct behaviour (according to my tests) is:

Component.update(): Clears the background only when it is a toplevel
component or a lightweight component. Heavyweights are 100%
selfresponsible for that (as for everything other as we will see).

Container.update(): Calls super.update() only when we are a top-level
container, otherwise directly jump into paint().

Container.visitChildren(): All children must be visited, even
Containers. The reason for not visiting the Containers has been that it
avoided double-painting heavyweights. However, heavyweight component
must not paint themselves when paint() is called. This is implemented
wrong here. It is actually the other way around: If the toolkit thinks
it has to paint a component, then it calls paint() _after_ the component
was painted, giving subclasses a chance to paint over the component. So,
paint() only acts as a callback here. This must also be fixed later. I
don't touch this for now since I think this could break the AWT.

The other stuff is in preferred/minimum/maximumSize(): There I replaced
direct access to layoutMgr with getLayout() so that components that
override this method still work correctly. I also added a kind of
caching. When the component is valid and we have a size stored in the
private fields, then we return this instead of asking the layout
manager. This should highly increase efficiency.

If you don't agree with one of the points above please let me know. But
I am pretty sure (did test it the last couple of hours) that all this is
correct.

2005-08-05  Roman Kennke  <[EMAIL PROTECTED]>

        * java/awt/Container.java:
        (preferredSize): Call getLayout() instead of directly
referencing
        the private field. This makes components work that override
        getLayout().
        (minimumSize): Call getLayout() instead of directly referencing
        the private field. This makes components work that override
        getLayout(). Use cached size if component is still valid.
        (maximumSize): Call getLayout() instead of directly referencing
        the private field. This makes components work that override
        getLayout(). Use cached size if component is still valid.
        (update): If we are a top-level-container, call super.update(),
        otherwise directly call paint().
        (visitChildren): Also visit children that are itself Containers.
        * java/awt/Component.java
        (update): Clear the background only for lightweight and
top-level
        components.

Index: java/awt/Container.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/awt/Container.java,v
retrieving revision 1.56
diff -u -r1.56 Container.java
--- java/awt/Container.java	25 Jul 2005 16:33:15 -0000	1.56
+++ java/awt/Container.java	5 Aug 2005 15:25:24 -0000
@@ -654,10 +654,10 @@
       {  
         if(valid && prefSize != null)
           return new Dimension(prefSize);
-
-        if (layoutMgr != null)
+        LayoutManager layout = getLayout();
+        if (layout != null)
           {
-            Dimension layoutSize = layoutMgr.preferredLayoutSize (this);
+            Dimension layoutSize = layout.preferredLayoutSize(this);
             if(valid)
               prefSize = layoutSize;
             return new Dimension(layoutSize);
@@ -686,8 +686,15 @@
    */
   public Dimension minimumSize()
   {
-    if (layoutMgr != null)
-      return layoutMgr.minimumLayoutSize (this);
+    if(valid && minSize != null)
+      return new Dimension(minSize);
+
+    LayoutManager layout = getLayout();
+    if (layout != null)
+      {
+        minSize = layout.minimumLayoutSize (this);
+        return minSize;
+      }    
     else
       return super.minimumSize ();
   }
@@ -699,10 +706,15 @@
    */
   public Dimension getMaximumSize()
   {
-    if (layoutMgr != null && layoutMgr instanceof LayoutManager2)
+    if (valid && maxSize != null)
+      return new Dimension(maxSize);
+
+    LayoutManager layout = getLayout();
+    if (layout != null && layout instanceof LayoutManager2)
       {
-        LayoutManager2 lm2 = (LayoutManager2) layoutMgr;
-        return lm2.maximumLayoutSize(this);
+        LayoutManager2 lm2 = (LayoutManager2) layout;
+        maxSize = lm2.maximumLayoutSize(this);
+        return maxSize;
       }
     else
       return super.getMaximumSize();
@@ -760,10 +772,21 @@
    * drawn.
    *
    * @param g The graphics context for this update.
+   *
+   * @specnote The specification suggests that this method forwards the
+   *           update() call to all its lightweight children. Tests show
+   *           that this is not done either in the JDK. The exact behaviour
+   *           seems to be that top-level container call super.update()
+   *           (causing the background to be cleared), and all other containers
+   *           directly call paint(), causing the (lightweight) children to
+   *           be painted.
    */
   public void update(Graphics g)
   {
-    super.update(g);
+    if (getParent() == null)
+      super.update(g);
+    
+    paint(g);
   }
 
   /**
@@ -1492,12 +1515,8 @@
         for (int i = ncomponents - 1; i >= 0; --i)
           {
             Component comp = component[i];
-            // If we're visiting heavyweights as well,
-            // don't recurse into Containers here. This avoids
-            // painting the same nested child multiple times.
             boolean applicable = comp.isVisible()
-              && (comp.isLightweight()
-                  || !lightweightOnly && ! (comp instanceof Container));
+              && (comp.isLightweight() || !lightweightOnly);
 
             if (applicable)
               visitChild(gfx, visitor, comp);
Index: java/awt/Component.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/awt/Component.java,v
retrieving revision 1.68
diff -u -r1.68 Component.java
--- java/awt/Component.java	2 Aug 2005 20:36:37 -0000	1.68
+++ java/awt/Component.java	5 Aug 2005 15:25:24 -0000
@@ -1848,10 +1848,19 @@
    *
    * @see #paint(Graphics)
    * @see #repaint()
+   *
+   * @specnote In contrast to what the spec says, tests show that the exact
+   *           behaviour is to clear the background on lightweight and
+   *           top-level components only. Heavyweight components are not
+   *           affected by this method and only call paint().
    */
   public void update(Graphics g)
   {
-    if (!isLightweight())
+    // Tests show that the clearing of the background is only done in
+    // two cases:
+    // - If the component is lightwight (yes this is in contrast to the spec).
+    // - If the component is a toplevel container.
+    if (isLightweight() || getParent() == null)
       {
         Rectangle clip = g.getClipBounds();
         if (clip == null)
@@ -1859,7 +1868,6 @@
         else
           g.clearRect(clip.x, clip.y, clip.width, clip.height);
       }
-
     paint(g);
   }
 
_______________________________________________
Classpath-patches mailing list
[email protected]
http://lists.gnu.org/mailman/listinfo/classpath-patches

Reply via email to