Eliminate rounding errors for GridLayout expand ratios (#19797)

Change-Id: Idf05dde5d6526fafee618fd3e2eb1afa63fab7bc

6 files changed, 129 insertions, 52 deletions

diff --git a/client/src/main/java/com/vaadin/client/ui/VGridLayout.java b/client/src/main/java/com/vaadin/client/ui/VGridLayout.java
index 42bcb5060a..9e13bb7fb2 100644
--- a/client/src/main/java/com/vaadin/client/ui/VGridLayout.java
+++ b/client/src/main/java/com/vaadin/client/ui/VGridLayout.java
@@ -61,10 +61,10 @@ public class VGridLayout extends ComplexPanel {
     public int[] rowHeights;
 
     /** For internal use only. May be removed or replaced in the future. */
-    public int[] colExpandRatioArray;
+    public float[] colExpandRatioArray;
 
     /** For internal use only. May be removed or replaced in the future. */
-    public int[] rowExpandRatioArray;
+    public float[] rowExpandRatioArray;
 
     int[] minColumnWidths;
 
@@ -142,7 +142,7 @@ public class VGridLayout extends ComplexPanel {
     void expandRows() {
         if (!isUndefinedHeight()) {
             int usedSpace = calcRowUsedSpace();
-            int[] actualExpandRatio = calcRowExpandRatio();
+            float[] actualExpandRatio = calcRowExpandRatio();
             // Round down to avoid problems with fractions (100.1px available ->
             // can use 100, not 101)
             int availableSpace = (int) LayoutManager.get(client)
@@ -150,13 +150,12 @@ public class VGridLayout extends ComplexPanel {
             int excessSpace = availableSpace - usedSpace;
             int distributed = 0;
             if (excessSpace > 0) {
-                int expandRatioSum = 0;
+                float expandRatioSum = 0;
                 for (int i = 0; i < rowHeights.length; i++) {
                     expandRatioSum += actualExpandRatio[i];
                 }
                 for (int i = 0; i < rowHeights.length; i++) {
-                    int ew = excessSpace * actualExpandRatio[i]
-                            / expandRatioSum;
+                    int ew = (int) (excessSpace * actualExpandRatio[i] / expandRatioSum);
                     rowHeights[i] = minRowHeights[i] + ew;
                     distributed += ew;
                 }
@@ -171,8 +170,8 @@ public class VGridLayout extends ComplexPanel {
         }
     }
 
-    private int[] calcRowExpandRatio() {
-        int[] actualExpandRatio = new int[minRowHeights.length];
+    private float[] calcRowExpandRatio() {
+        float[] actualExpandRatio = new float[minRowHeights.length];
         for (int i = 0; i < minRowHeights.length; i++) {
             if (hiddenEmptyRow(i)) {
                 actualExpandRatio[i] = 0;
@@ -224,7 +223,7 @@ public class VGridLayout extends ComplexPanel {
     void expandColumns() {
         if (!isUndefinedWidth()) {
             int usedSpace = calcColumnUsedSpace();
-            int[] actualExpandRatio = calcColumnExpandRatio();
+            float[] actualExpandRatio = calcColumnExpandRatio();
             // Round down to avoid problems with fractions (100.1px available ->
             // can use 100, not 101)
             int availableSpace = (int) LayoutManager.get(client)
@@ -232,13 +231,12 @@ public class VGridLayout extends ComplexPanel {
             int excessSpace = availableSpace - usedSpace;
             int distributed = 0;
             if (excessSpace > 0) {
-                int expandRatioSum = 0;
+                float expandRatioSum = 0;
                 for (int i = 0; i < columnWidths.length; i++) {
                     expandRatioSum += actualExpandRatio[i];
                 }
                 for (int i = 0; i < columnWidths.length; i++) {
-                    int ew = excessSpace * actualExpandRatio[i]
-                            / expandRatioSum;
+                    int ew = (int) (excessSpace * actualExpandRatio[i] / expandRatioSum);
                     columnWidths[i] = minColumnWidths[i] + ew;
                     distributed += ew;
                 }
@@ -256,8 +254,8 @@ public class VGridLayout extends ComplexPanel {
     /**
      * Calculates column expand ratio.
      */
-    private int[] calcColumnExpandRatio() {
-        int[] actualExpandRatio = new int[minColumnWidths.length];
+    private float[] calcColumnExpandRatio() {
+        float[] actualExpandRatio = new float[minColumnWidths.length];
         for (int i = 0; i < minColumnWidths.length; i++) {
             if (!hiddenEmptyColumn(i)) {
                 actualExpandRatio[i] = colExpandRatioArray[i];
@@ -537,7 +535,7 @@ public class VGridLayout extends ComplexPanel {
 
     private static void distributeSpanSize(int[] dimensions,
             int spanStartIndex, int spanSize, int spacingSize, int size,
-            int[] expansionRatios) {
+            float[] expansionRatios) {
         int allocated = dimensions[spanStartIndex];
         for (int i = 1; i < spanSize; i++) {
             allocated += spacingSize + dimensions[spanStartIndex + i];
@@ -563,8 +561,8 @@ public class VGridLayout extends ComplexPanel {
                     // expansion ratios
                     expansion = neededExtraSpace / spanSize;
                 } else {
-                    expansion = neededExtraSpace * expansionRatios[itemIndex]
-                            / totalExpansion;
+                    expansion = (int) (neededExtraSpace
+                            * expansionRatios[itemIndex] / totalExpansion);
                 }
                 dimensions[itemIndex] += expansion;
                 allocatedExtraSpace += expansion;
diff --git a/client/src/main/java/com/vaadin/client/ui/gridlayout/GridLayoutConnector.java b/client/src/main/java/com/vaadin/client/ui/gridlayout/GridLayoutConnector.java
index 4d1ce692ad..b450092d54 100644
--- a/client/src/main/java/com/vaadin/client/ui/gridlayout/GridLayoutConnector.java
+++ b/client/src/main/java/com/vaadin/client/ui/gridlayout/GridLayoutConnector.java
@@ -117,8 +117,8 @@ public class GridLayoutConnector extends AbstractComponentContainerConnector
             cell.updateCell(childComponentData);
         }
 
-        layout.colExpandRatioArray = uidl.getIntArrayAttribute("colExpand");
-        layout.rowExpandRatioArray = uidl.getIntArrayAttribute("rowExpand");
+        layout.colExpandRatioArray = getState().colExpand;
+        layout.rowExpandRatioArray = getState().rowExpand;
 
         layout.updateMarginStyleNames(new MarginInfo(getState().marginsBitmask));
         layout.updateSpacingStyleName(getState().spacing);
diff --git a/server/src/main/java/com/vaadin/ui/GridLayout.java b/server/src/main/java/com/vaadin/ui/GridLayout.java
index 3b9272b034..214d0c567f 100644
--- a/server/src/main/java/com/vaadin/ui/GridLayout.java
+++ b/server/src/main/java/com/vaadin/ui/GridLayout.java
@@ -461,58 +461,38 @@ public class GridLayout extends AbstractLayout implements
      */
     @Override
     public void paintContent(PaintTarget target) throws PaintException {
-        final Integer[] columnExpandRatioArray = new Integer[getColumns()];
-        final Integer[] rowExpandRatioArray = new Integer[getRows()];
+        // TODO Remove once LegacyComponent is no longer implemented
+    }
 
-        int realColExpandRatioSum = 0;
+    public void beforeClientResponse(boolean initial) {
+        super.beforeClientResponse(initial);
+
+        getState().colExpand = new float[getColumns()];
         float colSum = getExpandRatioSum(columnExpandRatio);
         if (colSum == 0) {
-            // no columns has been expanded, all cols have same expand
-            // rate
-            float equalSize = 1 / (float) getColumns();
-            int myRatio = Math.round(equalSize * 1000);
+            // no cols have been expanded
             for (int i = 0; i < getColumns(); i++) {
-                columnExpandRatioArray[i] = myRatio;
+                getState().colExpand[i] = 1f;
             }
-            realColExpandRatioSum = myRatio * getColumns();
         } else {
             for (int i = 0; i < getColumns(); i++) {
-                int myRatio = Math
-                        .round((getColumnExpandRatio(i) / colSum) * 1000);
-                columnExpandRatioArray[i] = myRatio;
-                realColExpandRatioSum += myRatio;
+                getState().colExpand[i] = getColumnExpandRatio(i);
             }
         }
 
-        int realRowExpandRatioSum = 0;
+        getState().rowExpand = new float[getRows()];
         float rowSum = getExpandRatioSum(rowExpandRatio);
         if (rowSum == 0) {
             // no rows have been expanded
-            float equalSize = 1 / (float) getRows();
-            int myRatio = Math.round(equalSize * 1000);
             for (int i = 0; i < getRows(); i++) {
-                rowExpandRatioArray[i] = myRatio;
+                getState().rowExpand[i] = 1f;
             }
-            realRowExpandRatioSum = myRatio * getRows();
         } else {
-            for (int cury = 0; cury < getRows(); cury++) {
-                int myRatio = Math
-                        .round((getRowExpandRatio(cury) / rowSum) * 1000);
-                rowExpandRatioArray[cury] = myRatio;
-                realRowExpandRatioSum += myRatio;
+            for (int i = 0; i < getRows(); i++) {
+                getState().rowExpand[i] = getRowExpandRatio(i);
             }
         }
 
-        // correct possible rounding error
-        if (rowExpandRatioArray.length > 0) {
-            rowExpandRatioArray[0] -= realRowExpandRatioSum - 1000;
-        }
-        if (columnExpandRatioArray.length > 0) {
-            columnExpandRatioArray[0] -= realColExpandRatioSum - 1000;
-        }
-        target.addAttribute("colExpand", columnExpandRatioArray);
-        target.addAttribute("rowExpand", rowExpandRatioArray);
-
     }
 
     private float getExpandRatioSum(Map<Integer, Float> ratioMap) {
diff --git a/shared/src/main/java/com/vaadin/shared/ui/gridlayout/GridLayoutState.java b/shared/src/main/java/com/vaadin/shared/ui/gridlayout/GridLayoutState.java
index b84c2673f4..7d637aa33f 100644
--- a/shared/src/main/java/com/vaadin/shared/ui/gridlayout/GridLayoutState.java
+++ b/shared/src/main/java/com/vaadin/shared/ui/gridlayout/GridLayoutState.java
@@ -40,6 +40,8 @@ public class GridLayoutState extends AbstractLayoutState {
     public Set<Integer> explicitColRatios = new HashSet<Integer>();
     public Map<Connector, ChildComponentData> childData = new HashMap<Connector, GridLayoutState.ChildComponentData>();
     public boolean hideEmptyRowsAndColumns = false;
+    public float[] rowExpand;
+    public float[] colExpand;
 
     public static class ChildComponentData implements Serializable {
         public int column1;
diff --git a/uitest/src/main/java/com/vaadin/tests/components/gridlayout/GridLayoutExpandWithManyRows.java b/uitest/src/main/java/com/vaadin/tests/components/gridlayout/GridLayoutExpandWithManyRows.java
new file mode 100644
index 0000000000..a901093e31
--- /dev/null
+++ b/uitest/src/main/java/com/vaadin/tests/components/gridlayout/GridLayoutExpandWithManyRows.java
@@ -0,0 +1,57 @@
+package com.vaadin.tests.components.gridlayout;
+
+import com.vaadin.annotations.Theme;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.ui.Component;
+import com.vaadin.ui.GridLayout;
+import com.vaadin.ui.Label;
+import com.vaadin.ui.Panel;
+import com.vaadin.ui.UI;
+
+@Theme("tests-valo")
+public class GridLayoutExpandWithManyRows extends UI {
+
+    static final int POPULATED_ROWS = 20;
+    static int ROW_COUNT = 58;
+
+    public static class ColoredLabel extends Label {
+        private static int colorNumber = 0;
+
+        public ColoredLabel() {
+            super();
+            addStyleName("color-label");
+            addStyleName("color-" + (colorNumber++) % 10);
+        }
+    }
+
+    @Override
+    protected void init(VaadinRequest request) {
+        for (int i = 0; i < 10; i++) {
+            getPage().getStyles().add(".color-" + i + " {" //
+                    + "background-color: hsl(" + (i * 90) + ", 60%, 70%);" //
+                    + "}");
+        }
+
+        GridLayout gridLayout = new GridLayout(6, ROW_COUNT);
+        for (int i = 0; i < ROW_COUNT; i++) {
+            gridLayout.setRowExpandRatio(i, 1);
+        }
+        gridLayout.setSizeFull();
+        for (int i = 0; i < POPULATED_ROWS; i++) {
+            int upperLeftRow = i * 2;
+            int upperLeftCol = 0;
+            int lowerRightCol = 5;
+            int lowerRightRow = upperLeftRow + 1;
+            ColoredLabel coloredLabel = new ColoredLabel();
+            coloredLabel.setSizeFull();
+            gridLayout.addComponent(coloredLabel, upperLeftCol, upperLeftRow,
+                    lowerRightCol, lowerRightRow);
+        }
+
+        gridLayout.setHeight("500%");
+        Component root = new Panel(gridLayout);
+        root.setSizeFull();
+        setContent(root);
+
+    }
+}
diff --git a/uitest/src/test/java/com/vaadin/tests/components/gridlayout/GridLayoutExpandWithManyRowsTest.java b/uitest/src/test/java/com/vaadin/tests/components/gridlayout/GridLayoutExpandWithManyRowsTest.java
new file mode 100644
index 0000000000..693fdc73db
--- /dev/null
+++ b/uitest/src/test/java/com/vaadin/tests/components/gridlayout/GridLayoutExpandWithManyRowsTest.java
@@ -0,0 +1,40 @@
+package com.vaadin.tests.components.gridlayout;
+
+import java.util.List;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.openqa.selenium.By;
+import org.openqa.selenium.WebElement;
+
+import com.vaadin.testbench.elements.GridLayoutElement;
+import com.vaadin.tests.tb3.SingleBrowserTest;
+
+public class GridLayoutExpandWithManyRowsTest extends SingleBrowserTest {
+
+    @Test
+    public void equalRowHeights() {
+        openTestURL();
+        GridLayoutElement gridlayout = $(GridLayoutElement.class).first();
+
+        // Rows are expanded using integer pixels and leftover pixels are added
+        // to the first N rows.
+        // The tests uses rowspan=2 so one row in the DOM should be max 2 pixels
+        // lower than the first row
+        List<WebElement> slots = gridlayout.findElements(By
+                .className("v-gridlayout-slot"));
+        Assert.assertEquals(GridLayoutExpandWithManyRows.POPULATED_ROWS,
+                slots.size());
+
+        int firstRowHeight = slots.get(0).getSize().height;
+        int lastRowHeight = firstRowHeight;
+        for (int i = 1; i < GridLayoutExpandWithManyRows.POPULATED_ROWS; i++) {
+            int rowHeight = slots.get(i).getSize().height;
+            Assert.assertTrue(rowHeight <= firstRowHeight);
+            Assert.assertTrue(rowHeight >= firstRowHeight - 2);
+            Assert.assertTrue(rowHeight <= lastRowHeight);
+
+            lastRowHeight = rowHeight;
+        }
+    }
+}