Reviewers: pengzhuang,
Description:
Deferring updating column widths until the redraw loop executes in a
finally command. Currently, every time the user adds or removes a
column, we refresh the widths of all columns. This leads to an O(n^2)
creation time for the table. Resetting the column widths in the redraw
loop reduces that to O(n), and is consistent with the rendering
implementation of the table itself. We use a dirty bit to indicate that
the column widths need to be updated, to avoid unnecessary work in the
redraw loop. When setting or clearing column widths, we update the
affectected col element synchronously.
Please review this at http://gwt-code-reviews.appspot.com/1509808/
Affected files:
M user/src/com/google/gwt/user/cellview/client/AbstractCellTable.java
M user/test/com/google/gwt/user/cellview/client/CellTableTest.java
Index: user/src/com/google/gwt/user/cellview/client/AbstractCellTable.java
===================================================================
--- user/src/com/google/gwt/user/cellview/client/AbstractCellTable.java
(revision 10508)
+++ user/src/com/google/gwt/user/cellview/client/AbstractCellTable.java
(working copy)
@@ -943,6 +943,7 @@
private boolean cellIsEditing;
private final List<Column<T, ?>> columns = new ArrayList<Column<T, ?>>();
private final Map<Column<T, ?>, String> columnWidths = new
HashMap<Column<T, ?>, String>();
+ private boolean columnWidthsDirty;
private final Map<Integer, String> columnWidthsByIndex = new
HashMap<Integer, String>();
/**
@@ -1125,7 +1126,7 @@
*/
public void clearColumnWidth(Column<T, ?> column) {
columnWidths.remove(column);
- refreshColumnWidths();
+ updateColumnWidthImpl(column, null);
}
/**
@@ -1135,7 +1136,10 @@
*/
public void clearColumnWidth(Integer column) {
columnWidthsByIndex.remove(column);
- refreshColumnWidths();
+ // TODO(jlabanca): Compare to realColumnCount when headerBuilder lands.
+ if (column < getColumnCount()) {
+ doSetColumnWidth(column, null);
+ }
}
/**
@@ -1350,7 +1354,7 @@
}
CellBasedWidgetImpl.get().sinkEvents(this, consumedEvents);
- redraw();
+ refreshColumnsAndRedraw();
}
/**
@@ -1405,12 +1409,6 @@
insertColumn(beforeIndex, col, new SafeHtmlHeader(headerHtml), new
SafeHtmlHeader(footerHtml));
}
- @Override
- public void redraw() {
- refreshColumnWidths();
- super.redraw();
- }
-
/**
* Redraw the table's footers.
*/
@@ -1457,7 +1455,7 @@
}
// Redraw the table asynchronously.
- redraw();
+ refreshColumnsAndRedraw();
// We don't unsink events because other handlers or user code may have
sunk
// them intentionally.
@@ -1481,7 +1479,7 @@
*/
public void setColumnWidth(Column<T, ?> column, String width) {
columnWidths.put(column, width);
- refreshColumnWidths();
+ updateColumnWidthImpl(column, width);
}
/**
@@ -1516,7 +1514,10 @@
*/
public void setColumnWidth(int column, String width) {
columnWidthsByIndex.put(column, width);
- refreshColumnWidths();
+ // TODO(jlabanca): Compare to realColumnCount when headerBuilder lands.
+ if (column < getColumnCount()) {
+ doSetColumnWidth(column, width);
+ }
}
/**
@@ -1994,8 +1995,7 @@
@Override
protected void replaceAllChildren(List<T> values, SafeHtml html) {
- // Render the headers and footers.
- createHeadersAndFooters();
+ refreshHeadersAndColumnsImpl();
/*
* If html is not null, then the user overrode renderRowValues() and
@@ -2015,7 +2015,7 @@
@SuppressWarnings("deprecation")
@Override
protected void replaceChildren(List<T> values, int start, SafeHtml html)
{
- createHeadersAndFooters();
+ refreshHeadersAndColumnsImpl();
/*
* If html is not null, then the user override renderRowValues() and
@@ -2561,6 +2561,28 @@
return (cellId == null) || (cellId.length() == 0) ? null : cellId;
}
+ /**
+ * Mark the column widths as dirty and redraw the table.
+ */
+ private void refreshColumnsAndRedraw() {
+ columnWidthsDirty = true;
+ redraw();
+ }
+
+ /**
+ * Refresh the headers and column widths.
+ */
+ private void refreshHeadersAndColumnsImpl() {
+ // Refresh the column widths if needed.
+ if (columnWidthsDirty) {
+ columnWidthsDirty = false;
+ refreshColumnWidths();
+ }
+
+ // Render the headers and footers.
+ createHeadersAndFooters();
+ }
+
private <C> boolean resetFocusOnCellImpl(int row, int col, HasCell<T, C>
column,
Element cellParent) {
T value = getVisibleItem(row);
@@ -2586,4 +2608,21 @@
setStyleName(cells.getItem(i), cellStyle, add);
}
}
+
+ /**
+ * Update the width of all instances of the specified column. A column
+ * instance may appear multiple times in the table.
+ *
+ * @param column the column to update
+ * @param width the width of the column, or null to clear the width
+ */
+ private void updateColumnWidthImpl(Column<T, ?> column, String width) {
+ // TODO(jlabanca): Use realColumnCount when headerBuilder lands.
+ int columnCount = getColumnCount();
+ for (int i = 0; i < columnCount; i++) {
+ if (columns.get(i) == column) {
+ doSetColumnWidth(i, width);
+ }
+ }
+ }
}
Index: user/test/com/google/gwt/user/cellview/client/CellTableTest.java
===================================================================
--- user/test/com/google/gwt/user/cellview/client/CellTableTest.java
(revision 10508)
+++ user/test/com/google/gwt/user/cellview/client/CellTableTest.java
(working copy)
@@ -192,9 +192,11 @@
// Remove column 1.
table.removeColumn(column1);
+ table.getPresenter().flush();
assertEquals("0px", col1.getStyle().getWidth());
}
+ @Override
public void testSetColumnWidth() {
CellTable<String> table = createAbstractHasData(new TextCell());
Column<String, ?> column0 = table.getColumn(0);
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
