Changeset 3588a2fad in vaadin

Timestamp:

02/10/12 09:35:43 (15 months ago)

Author:

Henri Sara <hesara@…>

Branches:

master, 7.0, 7.1

Children:

eba294b5

Parents:

81540696

git-author:

Henri Sara <hesara@…> (02/10/12 09:35:43)

git-committer:

Henri Sara <hesara@…> (02/10/12 09:35:43)

Message:

Defer nested components to paint them as top-level changes (#8304).

Refactoring and changes to paint nested component contents after the
component in which they are nested. The client side can create
uninitialized components immediately but defer configuring them.

Location:

src/com/vaadin

Files:

• terminal/PaintTarget.java (modified) (4 diffs)
• terminal/gwt/client/ApplicationConnection.java (modified) (1 diff)
• terminal/gwt/client/ui/VAbstractPaintableWidget.java (modified) (1 diff)
• terminal/gwt/client/ui/layout/CellBasedLayoutPaintable.java (modified) (3 diffs)
• terminal/gwt/server/AbstractCommunicationManager.java (modified) (1 diff)
• terminal/gwt/server/JsonPaintTarget.java (modified) (4 diffs)
• ui/AbstractComponent.java (modified) (3 diffs)

src/com/vaadin/terminal/PaintTarget.java

@@ -36 +36 @@
     public void addSection(String sectionTagName, String sectionData)
             throws PaintException;
+
+    /**
+     * Result of starting to paint a Paintable (
+     * {@link PaintTarget#startPaintable(Paintable, String)}).
+     * 
+     * @since 7.0
+     */
+    public enum PaintStatus {
+        /**
+         * Painting started, addVariable() and addAttribute() etc. methods may
+         * be called.
+         */
+        PAINTING,
+        /**
+         * Paintable is cached on the client side and unmodified - only an
+         * indication of that should be painted.
+         */
+        CACHED,
+        /**
+         * A previously unpainted or painted {@link Paintable} has been queued
+         * be created/update later in a separate change in the same set of
+         * changes.
+         */
+        DEFER
+    };
 
     /**
@@ -53 +78 @@
      * <p>
      * Each paintable being painted should be closed by a matching
-     * {@link #endPaintable(Paintable)}.
+     * {@link #endPaintable(Paintable)} regardless of the {@link PaintStatus}
+     * returned.
      * </p>
      * 
@@ -60 +86 @@
      * @param tag
      *            the name of the start tag.
-     * @return <code>true</code> if paintable found in cache, <code>false</code>
-     *         otherwise.
+     * @return {@link PaintStatus} - ready to paint, already cached on the
+     *         client or defer painting to another change
      * @throws PaintException
      *             if the paint operation failed.
@@ -67 +93 @@
      * @since 7.0 (previously using startTag(Paintable, String))
      */
-    public boolean startPaintable(Paintable paintable, String tag)
+    public PaintStatus startPaintable(Paintable paintable, String tag)
             throws PaintException;
 

src/com/vaadin/terminal/gwt/client/ApplicationConnection.java

@@ -1866 +1866 @@
     public VPaintableWidget getPaintable(UIDL uidl) {
         final String pid = uidl.getId();
+        // the actual content UIDL may be deferred, but it always contains
+        // enough information to create a paintable instance
         if (!paintableMap.hasPaintable(pid)) {
             // Create and register a new paintable if no old was found

src/com/vaadin/terminal/gwt/client/ui/VAbstractPaintableWidget.java

@@ -107 +107 @@
 
     protected static boolean isRealUpdate(UIDL uidl) {
-        return !isCachedUpdate(uidl) && !uidl.getBooleanAttribute("invisible");
+        return !isCachedUpdate(uidl) && !uidl.getBooleanAttribute("invisible")
+                && !uidl.hasAttribute("deferred");
     }
 

src/com/vaadin/terminal/gwt/client/ui/layout/CellBasedLayoutPaintable.java

@@ -16 +16 @@
         getWidgetForPaintable().client = client;
 
-        // Only non-cached UIDL:s can introduce changes
-        if (isCachedUpdate(uidl)) {
-            return;
+        if (isRealUpdate(uidl)) {
+            /**
+             * Margin and spacing detection depends on classNames and must be
+             * set before setting size. Here just update the details from UIDL
+             * and from overridden setStyleName run actual margin detections.
+             */
+            updateMarginAndSpacingInfo(uidl);
         }
-
-        /**
-         * Margin and spacind detection depends on classNames and must be set
-         * before setting size. Here just update the details from UIDL and from
-         * overridden setStyleName run actual margin detections.
-         */
-        updateMarginAndSpacingInfo(uidl);
 
         /*
@@ -34 +31 @@
         super.updateFromUIDL(uidl, client);
 
-        handleDynamicDimensions(uidl);
+        if (isRealUpdate(uidl)) {
+            handleDynamicDimensions(uidl);
+        }
     }
 
@@ -59 +58 @@
 
     void updateMarginAndSpacingInfo(UIDL uidl) {
-        if (!uidl.hasAttribute("invisible")) {
-            int bitMask = uidl.getIntAttribute("margins");
-            if (getWidgetForPaintable().activeMarginsInfo.getBitMask() != bitMask) {
-                getWidgetForPaintable().activeMarginsInfo = new VMarginInfo(
-                        bitMask);
-                getWidgetForPaintable().marginsNeedsRecalculation = true;
-            }
-            boolean spacing = uidl.getBooleanAttribute("spacing");
-            if (spacing != getWidgetForPaintable().spacingEnabled) {
-                getWidgetForPaintable().marginsNeedsRecalculation = true;
-                getWidgetForPaintable().spacingEnabled = spacing;
-            }
+        int bitMask = uidl.getIntAttribute("margins");
+        if (getWidgetForPaintable().activeMarginsInfo.getBitMask() != bitMask) {
+            getWidgetForPaintable().activeMarginsInfo = new VMarginInfo(bitMask);
+            getWidgetForPaintable().marginsNeedsRecalculation = true;
+        }
+        boolean spacing = uidl.getBooleanAttribute("spacing");
+        if (spacing != getWidgetForPaintable().spacingEnabled) {
+            getWidgetForPaintable().marginsNeedsRecalculation = true;
+            getWidgetForPaintable().spacingEnabled = spacing;
         }
     }

src/com/vaadin/terminal/gwt/server/AbstractCommunicationManager.java

@@ -857 +857 @@
                 if (paintTarget.needsToBePainted(p)) {
                     paintTarget.startTag("change");
-                    paintTarget.addAttribute("format", "uidl");
                     final String pid = getPaintableId(p);
                     paintTarget.addAttribute("pid", pid);
 
-                    // TODO this should paint subcomponents as references and
-                    // defer painting their contents to another top-level change
+                    // paints subcomponents as references (via
+                    // JsonPaintTarget.startPaintable()) and defers painting
+                    // their contents to another top-level change (via
+                    // queuePaintable())
                     p.paint(paintTarget);
 

src/com/vaadin/terminal/gwt/server/JsonPaintTarget.java

@@ -12 +12 @@
 import java.io.Serializable;
 import java.io.StringWriter;
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
@@ -91 +92 @@
     private Collection<Paintable> identifiersCreatedDueRefPaint;
 
+    private Collection<Paintable> deferredPaintables;
+
     private final Collection<Class<? extends Paintable>> usedPaintableTypes = new LinkedList<Class<? extends Paintable>>();
 
@@ -120 +123 @@
         openPaintables = new Stack<Paintable>();
         openPaintableTags = new Stack<String>();
+
+        deferredPaintables = new ArrayList<Paintable>();
 
         cacheEnabled = cachingRequired;
@@ -678 +683 @@
      * (non-Javadoc)
      * 
-     * @see com.vaadin.terminal.PaintTarget#startTag(com.vaadin.terminal
+     * @see com.vaadin.terminal.PaintTarget#startPaintable(com.vaadin.terminal
      * .Paintable, java.lang.String)
      */
-    public boolean startPaintable(Paintable paintable, String tagName)
+    public PaintStatus startPaintable(Paintable paintable, String tagName)
             throws PaintException {
         startTag(tagName, true);
         final boolean isPreviouslyPainted = manager.hasPaintableId(paintable)
                 && (identifiersCreatedDueRefPaint == null || !identifiersCreatedDueRefPaint
-                        .contains(paintable));
+                        .contains(paintable))
+                && !deferredPaintables.contains(paintable);
         final String id = manager.getPaintableId(paintable);
         paintable.addListener(manager);
         addAttribute("id", id);
 
-        // TODO if to queue if already painting a paintable
-        // if (openPaintables.isEmpty()) {
+        // queue for painting later if already painting a paintable
+        boolean topLevelPaintableTag = openPaintables.isEmpty();
+
         openPaintables.push(paintable);
         openPaintableTags.push(tagName);
 
-        paintedComponents.add(paintable);
-        // } else {
-        // // notify manager: add to paint queue instead of painting now
-        // manager.queuePaintable(paintable);
-        // // TODO return suitable value to defer painting and close tag if a
-        // // paintable tag is already open
-        // }
-
-        if (paintable instanceof CustomLayout) {
-            customLayoutArgumentsOpen = true;
-        }
-
-        return cacheEnabled && isPreviouslyPainted;
+        if (cacheEnabled && isPreviouslyPainted) {
+            // cached (unmodified) paintable, paint the it now
+            paintedComponents.add(paintable);
+            deferredPaintables.remove(paintable);
+            return PaintStatus.CACHED;
+        } else if (!topLevelPaintableTag) {
+            // notify manager: add to paint queue instead of painting now
+            manager.queuePaintable(paintable);
+            deferredPaintables.add(paintable);
+            return PaintStatus.DEFER;
+        } else {
+            // not a nested paintable, paint the it now
+            paintedComponents.add(paintable);
+            deferredPaintables.remove(paintable);
+
+            if (paintable instanceof CustomLayout) {
+                customLayoutArgumentsOpen = true;
+            }
+            return PaintStatus.PAINTING;
+        }
     }
 

src/com/vaadin/ui/AbstractComponent.java

@@ -28 +28 @@
 import com.vaadin.terminal.PaintException;
 import com.vaadin.terminal.PaintTarget;
+import com.vaadin.terminal.PaintTarget.PaintStatus;
 import com.vaadin.terminal.Resource;
 import com.vaadin.terminal.Terminal;
@@ -754 +755 @@
     public void paint(PaintTarget target) throws PaintException {
         final String tag = target.getTag(this);
-        if (!target.startPaintable(this, tag)
-                || repaintRequestListenersNotified) {
-
+        final PaintStatus status = target.startPaintable(this, tag);
+        if (PaintStatus.DEFER == status) {
+            // nothing to do but flag as deferred and close the paintable tag
+            // paint() will be called again later to paint the contents
+            target.addAttribute("deferred", true);
+        } else if (PaintStatus.CACHED == status
+                && !repaintRequestListenersNotified) {
+            // Contents have not changed, only cached presentation can be used
+            target.addAttribute("cached", true);
+        } else {
             // Paint the contents of the component
 
@@ -810 +818 @@
                 target.addAttribute("invisible", true);
             }
-        } else {
-
-            // Contents have not changed, only cached presentation can be used
-            target.addAttribute("cached", true);
         }
         target.endPaintable(this);