This closes #2226, optimize overlap merged cell ranges check (#2228)

- Update unit tests
- Remove empty row on saving, reduce generated workbook file size

Author: wangacc

merge.go

@@ -112,20 +112,12 @@ func (f *File) UnmergeCell(sheet, topLeftCell, bottomRightCell string) error {
 	if ws.MergeCells == nil {
 		return nil
 	}
-	if err = f.mergeOverlapCells(ws); err != nil {
+	if err = ws.mergeOverlapCells(); err != nil {
 		return err
 	}
 	i := 0
 	for _, mergeCell := range ws.MergeCells.Cells {
-		if mergeCell == nil {
-			continue
-		}
-		mergedCellsRef := mergeCell.Ref
-		if !strings.Contains(mergedCellsRef, ":") {
-			mergedCellsRef += ":" + mergedCellsRef
-		}
-		rect2, _ := rangeRefToCoordinates(mergedCellsRef)
-		if isOverlap(rect1, rect2) {
+		if rect2, _ := rangeRefToCoordinates(mergeCell.Ref); isOverlap(rect1, rect2) {
 			continue
 		}
 		ws.MergeCells.Cells[i] = mergeCell
@@ -163,7 +155,7 @@ func (f *File) GetMergeCells(sheet string, withoutValues ...bool) ([]MergeCell,
 		return mergeCells, err
 	}
 	if ws.MergeCells != nil {
-		if err = f.mergeOverlapCells(ws); err != nil {
+		if err = ws.mergeOverlapCells(); err != nil {
 			return mergeCells, err
 		}
 		mergeCells = make([]MergeCell, 0, len(ws.MergeCells.Cells))
@@ -179,123 +171,56 @@ func (f *File) GetMergeCells(sheet string, withoutValues ...bool) ([]MergeCell,
 	return mergeCells, err
 }
 
-// overlapRange calculate overlap range of merged cells, and returns max
-// column and rows of the range.
-func overlapRange(ws *xlsxWorksheet) (row, col int, err error) {
-	var rect []int
-	for _, mergeCell := range ws.MergeCells.Cells {
-		if mergeCell == nil {
+// mergeOverlapCells merge overlap cells.
+func (ws *xlsxWorksheet) mergeOverlapCells() error {
+	var (
+		err      error
+		rectList [][]int
+		merged   = true
+	)
+	for _, cell := range ws.MergeCells.Cells {
+		if cell == nil {
 			continue
 		}
-		if rect, err = mergeCell.Rect(); err != nil {
-			return
-		}
-		x1, y1, x2, y2 := rect[0], rect[1], rect[2], rect[3]
-		if x1 > col {
-			col = x1
-		}
-		if x2 > col {
-			col = x2
-		}
-		if y1 > row {
-			row = y1
-		}
-		if y2 > row {
-			row = y2
-		}
-	}
-	return
-}
-
-// flatMergedCells convert merged cells range reference to cell-matrix.
-func flatMergedCells(ws *xlsxWorksheet, matrix [][]*xlsxMergeCell) error {
-	for i, cell := range ws.MergeCells.Cells {
-		rect, err := cell.Rect()
-		if err != nil {
+		if cell.rect, err = cell.Rect(); err != nil {
 			return err
 		}
-		x1, y1, x2, y2 := rect[0]-1, rect[1]-1, rect[2]-1, rect[3]-1
-		var overlapCells []*xlsxMergeCell
-		for x := x1; x <= x2; x++ {
-			for y := y1; y <= y2; y++ {
-				if matrix[x][y] != nil {
-					overlapCells = append(overlapCells, matrix[x][y])
-				}
-				matrix[x][y] = cell
-			}
-		}
-		if len(overlapCells) != 0 {
-			newCell := cell
-			for _, overlapCell := range overlapCells {
-				newCell = mergeCell(cell, overlapCell)
+		rectList = append(rectList, cell.rect)
+	}
+	for merged {
+		merged = false
+		var mergedRectList [][]int
+		used := make([]bool, len(rectList))
+		for i := 0; i < len(rectList); i++ {
+			if used[i] {
+				continue
 			}
-			newRect, _ := newCell.Rect()
-			x1, y1, x2, y2 := newRect[0]-1, newRect[1]-1, newRect[2]-1, newRect[3]-1
-			for x := x1; x <= x2; x++ {
-				for y := y1; y <= y2; y++ {
-					matrix[x][y] = newCell
+			r1 := rectList[i]
+			for j := i + 1; j < len(rectList); j++ {
+				if used[j] {
+					continue
 				}
-			}
-			ws.MergeCells.Cells[i] = newCell
-		}
-	}
-	return nil
-}
-
-// mergeOverlapCells merge overlap cells.
-func (f *File) mergeOverlapCells(ws *xlsxWorksheet) error {
-	rows, cols, err := overlapRange(ws)
-	if err != nil {
-		return err
-	}
-	if rows == 0 || cols == 0 {
-		return nil
-	}
-	matrix := make([][]*xlsxMergeCell, cols)
-	for i := range matrix {
-		matrix[i] = make([]*xlsxMergeCell, rows)
-	}
-	_ = flatMergedCells(ws, matrix)
-	mergeCells := ws.MergeCells.Cells[:0]
-	for _, cell := range ws.MergeCells.Cells {
-		rect, _ := cell.Rect()
-		x1, y1, x2, y2 := rect[0]-1, rect[1]-1, rect[2]-1, rect[3]-1
-		if matrix[x1][y1] == cell {
-			mergeCells = append(mergeCells, cell)
-			for x := x1; x <= x2; x++ {
-				for y := y1; y <= y2; y++ {
-					matrix[x][y] = nil
+				if r2 := rectList[j]; isOverlap(r1, r2) {
+					r1 = []int{
+						min(r1[0], r2[0]),
+						min(r1[1], r2[1]),
+						max(r1[2], r2[2]),
+						max(r1[3], r2[3]),
+					}
+					merged, used[j] = true, true
 				}
 			}
+			mergedRectList = append(mergedRectList, r1)
 		}
+		rectList = mergedRectList
 	}
-	ws.MergeCells.Count, ws.MergeCells.Cells = len(mergeCells), mergeCells
-	return nil
-}
-
-// mergeCell merge two cells.
-func mergeCell(cell1, cell2 *xlsxMergeCell) *xlsxMergeCell {
-	rect1, _ := cell1.Rect()
-	rect2, _ := cell2.Rect()
-
-	if rect1[0] > rect2[0] {
-		rect1[0], rect2[0] = rect2[0], rect1[0]
-	}
-
-	if rect1[2] < rect2[2] {
-		rect1[2], rect2[2] = rect2[2], rect1[2]
-	}
-
-	if rect1[1] > rect2[1] {
-		rect1[1], rect2[1] = rect2[1], rect1[1]
-	}
-
-	if rect1[3] < rect2[3] {
-		rect1[3], rect2[3] = rect2[3], rect1[3]
+	ws.MergeCells.Cells = make([]*xlsxMergeCell, 0, len(rectList))
+	for _, r := range rectList {
+		ref, _ := coordinatesToRangeRef(r)
+		ws.MergeCells.Cells = append(ws.MergeCells.Cells, &xlsxMergeCell{Ref: ref, rect: r})
 	}
-	topLeftCell, _ := CoordinatesToCellName(rect1[0], rect1[1])
-	bottomRightCell, _ := CoordinatesToCellName(rect1[2], rect1[3])
-	return &xlsxMergeCell{rect: rect1, Ref: topLeftCell + ":" + bottomRightCell}
+	ws.MergeCells.Count = len(ws.MergeCells.Cells)
+	return err
 }
 
 // MergeCell define a merged cell data.
@@ -317,9 +242,5 @@ func (m *MergeCell) GetStartAxis() string {
 // GetEndAxis returns the bottom right cell reference of merged range, for
 // example: "D4".
 func (m *MergeCell) GetEndAxis() string {
-	coordinates := strings.Split((*m)[0], ":")
-	if len(coordinates) == 2 {
-		return coordinates[1]
-	}
-	return coordinates[0]
+	return strings.Split((*m)[0], ":")[1]
 }

merge_test.go

@@ -217,11 +217,6 @@ func TestUnmergeCell(t *testing.T) {
 	assert.EqualError(t, f.UnmergeCell("Sheet1", "A2", "B3"), newCellNameToCoordinatesError("A", newInvalidCellNameError("A")).Error())
 }
 
-func TestFlatMergedCells(t *testing.T) {
-	ws := &xlsxWorksheet{MergeCells: &xlsxMergeCells{Cells: []*xlsxMergeCell{{Ref: ""}}}}
-	assert.EqualError(t, flatMergedCells(ws, [][]*xlsxMergeCell{}), "cannot convert cell \"\" to coordinates: invalid cell name \"\"")
-}
-
 func TestMergeCellsParser(t *testing.T) {
 	ws := &xlsxWorksheet{MergeCells: &xlsxMergeCells{Cells: []*xlsxMergeCell{nil}}}
 	_, err := ws.mergeCellsParser("A1")

sheet.go

@@ -160,7 +160,7 @@ func (f *File) workSheetWriter() {
 		if ws != nil {
 			sheet := ws.(*xlsxWorksheet)
 			if sheet.MergeCells != nil && len(sheet.MergeCells.Cells) > 0 {
-				_ = f.mergeOverlapCells(sheet)
+				_ = sheet.mergeOverlapCells()
 			}
 			if sheet.Cols != nil && len(sheet.Cols.Col) > 0 {
 				f.mergeExpandedCols(sheet)
@@ -197,12 +197,15 @@ func trimRow(sheetData *xlsxSheetData) []xlsxRow {
 		i   int
 	)
 
-	for k := range sheetData.Row {
+	for k := 0; k < len(sheetData.Row); k++ {
 		row = sheetData.Row[k]
 		if row = trimCell(row); len(row.C) != 0 || row.hasAttr() {
 			sheetData.Row[i] = row
+			i++
+			continue
 		}
-		i++
+		sheetData.Row = append(sheetData.Row[:k], sheetData.Row[k+1:]...)
+		k--
 	}
 	return sheetData.Row[:i]
 }