From 3588a2fad586f1ec05b4381e66ddc4a1446483a7 Mon Sep 17 00:00:00 2001 From: Henri Sara Date: Fri, 10 Feb 2012 11:35:43 +0200 Subject: [PATCH] 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. --- src/com/vaadin/terminal/PaintTarget.java | 34 ++++++++++++-- .../gwt/client/ApplicationConnection.java | 2 + .../client/ui/VAbstractPaintableWidget.java | 3 +- .../ui/layout/CellBasedLayoutPaintable.java | 42 ++++++++--------- .../server/AbstractCommunicationManager.java | 7 +-- .../terminal/gwt/server/JsonPaintTarget.java | 46 ++++++++++++------- src/com/vaadin/ui/AbstractComponent.java | 18 +++++--- 7 files changed, 98 insertions(+), 54 deletions(-) diff --git a/src/com/vaadin/terminal/PaintTarget.java b/src/com/vaadin/terminal/PaintTarget.java index 78cc9e0af9..f5de7bf136 100644 --- a/src/com/vaadin/terminal/PaintTarget.java +++ b/src/com/vaadin/terminal/PaintTarget.java @@ -36,6 +36,31 @@ public interface PaintTarget extends Serializable { 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 + }; + /** * Prints element start tag of a paintable section. Starts a paintable * section using the given tag. The PaintTarget may implement a caching @@ -52,21 +77,22 @@ public interface PaintTarget extends Serializable { *

*

* Each paintable being painted should be closed by a matching - * {@link #endPaintable(Paintable)}. + * {@link #endPaintable(Paintable)} regardless of the {@link PaintStatus} + * returned. *

* * @param paintable * the paintable to start. * @param tag * the name of the start tag. - * @return true if paintable found in cache, false - * 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. * @see #startTag(String) * @since 7.0 (previously using startTag(Paintable, String)) */ - public boolean startPaintable(Paintable paintable, String tag) + public PaintStatus startPaintable(Paintable paintable, String tag) throws PaintException; /** diff --git a/src/com/vaadin/terminal/gwt/client/ApplicationConnection.java b/src/com/vaadin/terminal/gwt/client/ApplicationConnection.java index 1415e8a0ff..b41b4f1b3c 100644 --- a/src/com/vaadin/terminal/gwt/client/ApplicationConnection.java +++ b/src/com/vaadin/terminal/gwt/client/ApplicationConnection.java @@ -1865,6 +1865,8 @@ public class ApplicationConnection { */ 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 VPaintableWidget p = widgetSet.createWidget(uidl.getTag(), diff --git a/src/com/vaadin/terminal/gwt/client/ui/VAbstractPaintableWidget.java b/src/com/vaadin/terminal/gwt/client/ui/VAbstractPaintableWidget.java index 355516ccd1..746bc99ba4 100644 --- a/src/com/vaadin/terminal/gwt/client/ui/VAbstractPaintableWidget.java +++ b/src/com/vaadin/terminal/gwt/client/ui/VAbstractPaintableWidget.java @@ -106,7 +106,8 @@ public abstract class VAbstractPaintableWidget implements VPaintableWidget { } protected static boolean isRealUpdate(UIDL uidl) { - return !isCachedUpdate(uidl) && !uidl.getBooleanAttribute("invisible"); + return !isCachedUpdate(uidl) && !uidl.getBooleanAttribute("invisible") + && !uidl.hasAttribute("deferred"); } protected static boolean isCachedUpdate(UIDL uidl) { diff --git a/src/com/vaadin/terminal/gwt/client/ui/layout/CellBasedLayoutPaintable.java b/src/com/vaadin/terminal/gwt/client/ui/layout/CellBasedLayoutPaintable.java index 752462dcf6..3a5030e606 100644 --- a/src/com/vaadin/terminal/gwt/client/ui/layout/CellBasedLayoutPaintable.java +++ b/src/com/vaadin/terminal/gwt/client/ui/layout/CellBasedLayoutPaintable.java @@ -15,25 +15,24 @@ public abstract class CellBasedLayoutPaintable extends public void updateFromUIDL(UIDL uidl, ApplicationConnection client) { 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); - /* * This call should be made first. Ensure correct implementation, handle * size etc. */ super.updateFromUIDL(uidl, client); - handleDynamicDimensions(uidl); + if (isRealUpdate(uidl)) { + handleDynamicDimensions(uidl); + } } private void handleDynamicDimensions(UIDL uidl) { @@ -58,18 +57,15 @@ public abstract class CellBasedLayoutPaintable extends } 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; } } diff --git a/src/com/vaadin/terminal/gwt/server/AbstractCommunicationManager.java b/src/com/vaadin/terminal/gwt/server/AbstractCommunicationManager.java index 4421001a3f..7b75592442 100644 --- a/src/com/vaadin/terminal/gwt/server/AbstractCommunicationManager.java +++ b/src/com/vaadin/terminal/gwt/server/AbstractCommunicationManager.java @@ -856,12 +856,13 @@ public abstract class AbstractCommunicationManager implements // rendered already (changes with only cached flag) 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); paintTarget.endTag("change"); diff --git a/src/com/vaadin/terminal/gwt/server/JsonPaintTarget.java b/src/com/vaadin/terminal/gwt/server/JsonPaintTarget.java index 3f68577f8c..4470970095 100644 --- a/src/com/vaadin/terminal/gwt/server/JsonPaintTarget.java +++ b/src/com/vaadin/terminal/gwt/server/JsonPaintTarget.java @@ -11,6 +11,7 @@ import java.io.InputStreamReader; import java.io.PrintWriter; import java.io.Serializable; import java.io.StringWriter; +import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; import java.util.HashSet; @@ -90,6 +91,8 @@ public class JsonPaintTarget implements PaintTarget { private Collection identifiersCreatedDueRefPaint; + private Collection deferredPaintables; + private final Collection> usedPaintableTypes = new LinkedList>(); /** @@ -120,6 +123,8 @@ public class JsonPaintTarget implements PaintTarget { openPaintables = new Stack(); openPaintableTags = new Stack(); + deferredPaintables = new ArrayList(); + cacheEnabled = cachingRequired; } @@ -677,37 +682,46 @@ public class JsonPaintTarget implements PaintTarget { /* * (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 (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; + if (paintable instanceof CustomLayout) { + customLayoutArgumentsOpen = true; + } + return PaintStatus.PAINTING; } - - return cacheEnabled && isPreviouslyPainted; } public void endPaintable(Paintable paintable) throws PaintException { diff --git a/src/com/vaadin/ui/AbstractComponent.java b/src/com/vaadin/ui/AbstractComponent.java index fa46441d06..8d9e3be932 100644 --- a/src/com/vaadin/ui/AbstractComponent.java +++ b/src/com/vaadin/ui/AbstractComponent.java @@ -27,6 +27,7 @@ import com.vaadin.event.ShortcutListener; import com.vaadin.terminal.ErrorMessage; 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; import com.vaadin.terminal.gwt.client.ComponentState; @@ -753,9 +754,16 @@ public abstract class AbstractComponent implements Component, MethodEventSource */ 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 // Only paint content of visible components. @@ -809,10 +817,6 @@ public abstract class AbstractComponent implements Component, MethodEventSource } else { target.addAttribute("invisible", true); } - } else { - - // Contents have not changed, only cached presentation can be used - target.addAttribute("cached", true); } target.endPaintable(this); -- 2.39.5