From a24fcb25eff21920d3e2785d7bad040e7f9a2bc4 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Leif=20=C3=85strand?= Date: Mon, 13 Feb 2012 16:49:32 +0200 Subject: [PATCH] Refactor to calculate heights and widths separately (#8313) --- .../terminal/gwt/client/FastStringSet.java | 5 + .../terminal/gwt/client/MeasureManager.java | 134 ++++++++++-------- .../terminal/gwt/client/MeasuredSize.java | 80 +++++++---- .../client/ui/VAbstractPaintableWidget.java | 20 ++- .../ui/VMeasuringOrderedLayoutPaintable.java | 4 +- 5 files changed, 149 insertions(+), 94 deletions(-) diff --git a/src/com/vaadin/terminal/gwt/client/FastStringSet.java b/src/com/vaadin/terminal/gwt/client/FastStringSet.java index e5ba40da0d..918c44d38a 100644 --- a/src/com/vaadin/terminal/gwt/client/FastStringSet.java +++ b/src/com/vaadin/terminal/gwt/client/FastStringSet.java @@ -34,6 +34,11 @@ public final class FastStringSet extends JavaScriptObject { return array; }-*/; + public native void remove(String string) + /*-{ + delete this[string]; + }-*/; + public static FastStringSet create() { return JavaScriptObject.createObject().cast(); } diff --git a/src/com/vaadin/terminal/gwt/client/MeasureManager.java b/src/com/vaadin/terminal/gwt/client/MeasureManager.java index 26f922ca5f..516eb61c08 100644 --- a/src/com/vaadin/terminal/gwt/client/MeasureManager.java +++ b/src/com/vaadin/terminal/gwt/client/MeasureManager.java @@ -1,5 +1,6 @@ package com.vaadin.terminal.gwt.client; +import com.google.gwt.core.client.Duration; import com.google.gwt.core.client.JsArrayString; import com.vaadin.terminal.gwt.client.ui.LayoutPhaseListener; import com.vaadin.terminal.gwt.client.ui.ResizeRequired; @@ -18,76 +19,88 @@ public class MeasureManager { } int passes = 0; - long start = System.currentTimeMillis(); + Duration totalDuration = new Duration(); + while (true) { - long passStart = System.currentTimeMillis(); + Duration passDuration = new Duration(); passes++; - long measureStart = System.currentTimeMillis(); - FastStringSet changedSet = findChangedWidgets(paintableWidgets); - JsArrayString changed = changedSet.dump(); - long measureEnd = System.currentTimeMillis(); + measureWidgets(paintableWidgets); - VConsole.log("Measure in " + (measureEnd - measureStart) + " ms"); + FastStringSet needsHeightUpdate = FastStringSet.create(); + FastStringSet needsWidthUpdate = FastStringSet.create(); - if (changed.length() == 0) { - VConsole.log("No more changes in pass " + passes); - break; - } + for (VPaintableWidget paintable : paintableWidgets) { + MeasuredSize measuredSize = paintable.getMeasuredSize(); + boolean notifiableType = isNotifiableType(paintable); - if (passes > 100) { - VConsole.log("Aborting layout"); - break; - } + VPaintableWidgetContainer parent = paintable.getParent(); + boolean parentNotifiable = parent != null + && isNotifiableType(parent); - FastStringSet affectedContainers = FastStringSet.create(); - for (int i = 0; i < changed.length(); i++) { - VPaintableWidget paintable = (VPaintableWidget) paintableMap - .getPaintable(changed.get(i)); - VPaintableWidget parentPaintable = paintable.getParent(); - if (parentPaintable instanceof CalculatingLayout) { - affectedContainers.add(parentPaintable.getId()); + if (measuredSize.isHeightNeedsUpdate()) { + if (notifiableType) { + needsHeightUpdate.add(paintable.getId()); + } + if (!paintable.isRelativeHeight() && parentNotifiable) { + needsHeightUpdate.add(parent.getId()); + } + } + if (measuredSize.isWidthNeedsUpdate()) { + if (notifiableType) { + needsWidthUpdate.add(paintable.getId()); + } + if (!paintable.isRelativeWidth() && parentNotifiable) { + needsWidthUpdate.add(parent.getId()); + } } + measuredSize.clearDirtyState(); } - long layoutStart = System.currentTimeMillis(); - for (int i = 0; i < changed.length(); i++) { - String pid = changed.get(i); + int measureTime = passDuration.elapsedMillis(); + VConsole.log("Measure in " + measureTime + " ms"); + + FastStringSet updatedSet = FastStringSet.create(); + + JsArrayString needsHeightUpdateArray = needsHeightUpdate.dump(); + for (int i = 0; i < needsHeightUpdateArray.length(); i++) { + String pid = needsHeightUpdateArray.get(i); + VPaintableWidget paintable = (VPaintableWidget) paintableMap .getPaintable(pid); - if (!affectedContainers.contains(pid)) { - if (paintable instanceof CalculatingLayout) { - CalculatingLayout calculating = (CalculatingLayout) paintable; - calculating.updateHorizontalSizes(); - calculating.updateVerticalSizes(); - } else if (paintable instanceof ResizeRequired) { - ((ResizeRequired) paintable).onResize(); - } + if (paintable instanceof CalculatingLayout) { + CalculatingLayout cl = (CalculatingLayout) paintable; + cl.updateVerticalSizes(); + + } else if (paintable instanceof ResizeRequired) { + ResizeRequired rr = (ResizeRequired) paintable; + rr.onResize(); + needsWidthUpdate.remove(pid); } + updatedSet.add(pid); } - JsArrayString affectedPids = affectedContainers.dump(); - for (int i = 0; i < affectedPids.length(); i++) { - // Find all changed children - String containerPid = affectedPids.get(i); - CalculatingLayout container = (CalculatingLayout) paintableMap - .getPaintable(containerPid); + JsArrayString needsWidthUpdateArray = needsWidthUpdate.dump(); + for (int i = 0; i < needsWidthUpdateArray.length(); i++) { + String pid = needsWidthUpdateArray.get(i); - container.updateHorizontalSizes(); - container.updateVerticalSizes(); + VPaintable paintable = paintableMap.getPaintable(pid); + if (paintable instanceof CalculatingLayout) { + CalculatingLayout cl = (CalculatingLayout) paintable; + cl.updateHorizontalSizes(); + } + updatedSet.add(pid); } - long layoutEnd = System.currentTimeMillis(); - VConsole.log(affectedPids.length() - + " requestLayout invocations in " - + (layoutEnd - layoutStart) + "ms"); + JsArrayString changed = updatedSet.dump(); + VConsole.log(changed.length() + " requestLayout invocations in " + + (passDuration.elapsedMillis() - measureTime) + "ms"); - long passEnd = System.currentTimeMillis(); StringBuilder b = new StringBuilder(); b.append(changed.length()); b.append(" changed widgets in pass "); b.append(passes); b.append(" in "); - b.append((passEnd - passStart)); + b.append(passDuration.elapsedMillis()); b.append(" ms: "); if (changed.length() < 10) { for (int i = 0; i < changed.length(); i++) { @@ -98,6 +111,16 @@ public class MeasureManager { } } VConsole.log(b.toString()); + + if (changed.length() == 0) { + VConsole.log("No more changes in pass " + passes); + break; + } + + if (passes > 100) { + VConsole.log("Aborting layout"); + break; + } } for (VPaintableWidget vPaintableWidget : paintableWidgets) { @@ -106,23 +129,20 @@ public class MeasureManager { } } - long end = System.currentTimeMillis(); - VConsole.log("Total layout time: " + (end - start) + "ms"); + VConsole.log("Total layout time: " + totalDuration.elapsedMillis() + + "ms"); } - private FastStringSet findChangedWidgets(VPaintableWidget[] paintableWidgets) { + private void measureWidgets(VPaintableWidget[] paintableWidgets) { - FastStringSet changed = FastStringSet.create(); for (VPaintableWidget paintableWidget : paintableWidgets) { MeasuredSize measuredSize = paintableWidget.getMeasuredSize(); measuredSize.measure(); - - if (measuredSize.isDirty()) { - changed.add(paintableWidget.getId()); - measuredSize.setDirty(false); - } } + } - return changed; + private static boolean isNotifiableType(VPaintableWidget paintable) { + return paintable instanceof ResizeRequired + || paintable instanceof CalculatingLayout; } } diff --git a/src/com/vaadin/terminal/gwt/client/MeasuredSize.java b/src/com/vaadin/terminal/gwt/client/MeasuredSize.java index 85ca915e6d..93642df7ec 100644 --- a/src/com/vaadin/terminal/gwt/client/MeasuredSize.java +++ b/src/com/vaadin/terminal/gwt/client/MeasuredSize.java @@ -1,6 +1,5 @@ package com.vaadin.terminal.gwt.client; -import java.util.Arrays; import java.util.HashMap; import java.util.Map; import java.util.Map.Entry; @@ -18,7 +17,8 @@ public final class MeasuredSize { private final VPaintableWidget paintable; - private boolean isDirty = true; + private boolean heightChanged = true; + private boolean widthChanged = true; private final Map dependencySizes = new HashMap(); @@ -54,30 +54,21 @@ public final class MeasuredSize { public void setOuterHeight(int height) { if (this.height != height) { - isDirty = true; + heightChanged = true; this.height = height; } } public void setOuterWidth(int width) { if (this.width != width) { - isDirty = true; + widthChanged = true; this.width = width; } } - public boolean isDirty() { - return isDirty; - } - - public void setDirty(boolean isDirty) { - this.isDirty = isDirty; - } - public void registerDependency(Element element) { if (!dependencySizes.containsKey(element)) { dependencySizes.put(element, new int[] { -1, -1 }); - isDirty = true; } } @@ -175,28 +166,35 @@ public final class MeasuredSize { } void measure() { - boolean changed = isDirty; - Widget widget = paintable.getWidgetForPaintable(); ComputedStyle computedStyle = new ComputedStyle(widget.getElement()); int[] paddings = computedStyle.getPadding(); - if (!changed && !Arrays.equals(this.paddings, paddings)) { - changed = true; - this.paddings = paddings; + if (!heightChanged && hasHeightChanged(this.paddings, paddings)) { + heightChanged = true; + } + if (!widthChanged && hasWidthChanged(this.paddings, paddings)) { + widthChanged = true; } + this.paddings = paddings; int[] margins = computedStyle.getMargin(); - if (!changed && !Arrays.equals(this.margins, margins)) { - changed = true; - this.margins = margins; + if (!heightChanged && hasHeightChanged(this.margins, margins)) { + heightChanged = true; } + if (!widthChanged && hasWidthChanged(this.margins, margins)) { + widthChanged = true; + } + this.margins = margins; int[] borders = computedStyle.getBorder(); - if (!changed && !Arrays.equals(this.borders, borders)) { - changed = true; - this.borders = borders; + if (!heightChanged && hasHeightChanged(this.borders, borders)) { + heightChanged = true; + } + if (!widthChanged && hasWidthChanged(this.borders, borders)) { + widthChanged = true; } + this.borders = borders; int offsetHeight = widget.getOffsetHeight(); int marginHeight = sumHeights(margins); @@ -219,7 +217,7 @@ public final class MeasuredSize { // + " width changed from " + sizes[0] + " to " // + elementWidth); sizes[0] = elementWidth; - changed = true; + widthChanged = true; } int elementHeight = element.getOffsetHeight(); @@ -231,13 +229,37 @@ public final class MeasuredSize { // + " height changed from " + sizes[1] + " to " // + elementHeight); sizes[1] = elementHeight; - changed = true; + heightChanged = true; } // i++; } + } - if (changed) { - setDirty(true); - } + void clearDirtyState() { + heightChanged = widthChanged = false; + } + + public boolean isHeightNeedsUpdate() { + return heightChanged; + } + + public boolean isWidthNeedsUpdate() { + return widthChanged; + } + + private static boolean hasWidthChanged(int[] sizes1, int[] sizes2) { + return sizes1[1] != sizes2[1] || sizes1[3] != sizes2[3]; + } + + private static boolean hasHeightChanged(int[] sizes1, int[] sizes2) { + return sizes1[0] != sizes2[0] || sizes1[2] != sizes2[2]; + } + + public void setWidthNeedsUpdate() { + widthChanged = true; + } + + public void setHeightNeedsUpdate() { + heightChanged = true; } } \ No newline at end of file diff --git a/src/com/vaadin/terminal/gwt/client/ui/VAbstractPaintableWidget.java b/src/com/vaadin/terminal/gwt/client/ui/VAbstractPaintableWidget.java index e02e3f4a44..9c8853bd88 100644 --- a/src/com/vaadin/terminal/gwt/client/ui/VAbstractPaintableWidget.java +++ b/src/com/vaadin/terminal/gwt/client/ui/VAbstractPaintableWidget.java @@ -196,12 +196,20 @@ public abstract class VAbstractPaintableWidget implements VPaintableWidget { String h = uidl.hasAttribute("height") ? uidl .getStringAttribute("height") : ""; - // Dirty if either dimension changed between relative and non-relative - MeasuredSize measuredSize = getMeasuredSize(); - if (!measuredSize.isDirty() - && (w.endsWith("%") != definedWidth.endsWith("%") || h - .endsWith("%") != definedHeight.endsWith("%"))) { - measuredSize.setDirty(true); + // Parent should be updated if either dimension changed between relative + // and non-relative + if (w.endsWith("%") != definedWidth.endsWith("%")) { + VPaintableWidgetContainer parent = getParent(); + if (parent != null) { + parent.getMeasuredSize().setWidthNeedsUpdate(); + } + } + + if (h.endsWith("%") != definedHeight.endsWith("%")) { + VPaintableWidgetContainer parent = getParent(); + if (parent != null) { + parent.getMeasuredSize().setHeightNeedsUpdate(); + } } definedWidth = w; diff --git a/src/com/vaadin/terminal/gwt/client/ui/VMeasuringOrderedLayoutPaintable.java b/src/com/vaadin/terminal/gwt/client/ui/VMeasuringOrderedLayoutPaintable.java index c995e681d1..0583be5ada 100644 --- a/src/com/vaadin/terminal/gwt/client/ui/VMeasuringOrderedLayoutPaintable.java +++ b/src/com/vaadin/terminal/gwt/client/ui/VMeasuringOrderedLayoutPaintable.java @@ -96,7 +96,6 @@ public abstract class VMeasuringOrderedLayoutPaintable extends if (!childUIDL.getBooleanAttribute("cached")) { child.updateFromUIDL(childUIDL, client); - child.getMeasuredSize().setDirty(true); } previousChildren.remove(child); @@ -115,7 +114,8 @@ public abstract class VMeasuringOrderedLayoutPaintable extends layout.updateSpacingStyleName(uidl.getBooleanAttribute("spacing")); - getMeasuredSize().setDirty(true); + getMeasuredSize().setHeightNeedsUpdate(); + getMeasuredSize().setWidthNeedsUpdate(); } private int getSizeForInnerSize(int size, boolean isVertical) { -- 2.39.5