internal/packing: enable non-square page size

Closes #2327

internal/packing/packing.go

@@ -17,6 +17,7 @@ package packing
 
 import (
 	"errors"
+	"fmt"
 )
 
 const (
@@ -25,15 +26,15 @@ const (
 
 type Page struct {
 	root    *Node
-	size    int
+	width   int
+	height  int
 	maxSize int
-
-	rollbackExtension func()
 }
 
 func NewPage(initSize int, maxSize int) *Page {
 	return &Page{
-		size:    initSize,
+		width:   initSize,
+		height:  initSize,
 		maxSize: maxSize,
 	}
 }
@@ -86,7 +87,7 @@ func square(width, height int) float64 {
 
 func (p *Page) canAlloc(n *Node, width, height int) bool {
 	if p.root == nil {
-		return p.size >= width && p.size >= height
+		return p.width >= width && p.height >= height
 	}
 	return canAlloc(p.root, width, height)
 }
@@ -170,7 +171,7 @@ func alloc(n *Node, width, height int) *Node {
 }
 
 func (p *Page) Size() (int, int) {
-	return p.size, p.size
+	return p.width, p.height
 }
 
 func (p *Page) SetMaxSize(size int) {
@@ -191,8 +192,8 @@ func (p *Page) Alloc(width, height int) *Node {
 
 	if p.root == nil {
 		p.root = &Node{
-			width:  p.size,
+			width:  p.width,
-			height: p.size,
+			height: p.height,
 		}
 	}
 	if width < minSize {
@@ -244,47 +245,50 @@ func (p *Page) extendFor(width, height int) bool {
 		return true
 	}
 
-	nExtended := 1
+	if p.width >= p.maxSize && p.height >= p.maxSize {
-	for {
+		return false
-		if !p.extend(nExtended) {
+	}
-			// The page can't be extended any more. Return as failure.
+
-			return false
+	// (1, 0), (0, 1), (2, 0), (1, 1), (0, 2), (3, 0), (2, 1), (1, 2), (0, 3), ...
-		}
+	for i := 1; ; i++ {
-		nExtended++
+		for j := 0; j <= i; j++ {
-		if p.canAlloc(p.root, width, height) {
+			newWidth := p.width
-			p.rollbackExtension = nil
+			for k := 0; k < i-j; k++ {
-			break
+				newWidth *= 2
-		}
+			}
-		p.rollbackExtension()
+			newHeight := p.height
-		p.rollbackExtension = nil
+			for k := 0; k < j; k++ {
+				newHeight *= 2
+			}
+
+			if newWidth > p.maxSize || newHeight > p.maxSize {
+				if newWidth > p.maxSize && newHeight > p.maxSize {
+					panic(fmt.Sprintf("packing: too big extension: (%d, %d)", newWidth, newHeight))
+				}
+				continue
+			}
+
+			rollback := p.extend(newWidth, newHeight)
+			if p.canAlloc(p.root, width, height) {
+				return true
+			}
+			rollback()
+
+			// If the allocation failed even with a maximized page, give up the allocation.
+			if newWidth >= p.maxSize && newHeight >= p.maxSize {
+				return false
+			}
+		}
 	}
-	return true
 }
 
-func (p *Page) extend(count int) bool {
+func (p *Page) extend(newWidth int, newHeight int) func() {
-	if p.rollbackExtension != nil {
-		panic("packing: Extend cannot be called without rolling back or committing")
-	}
-
-	if p.size >= p.maxSize {
-		return false
-	}
-
-	newSize := p.size
-	for i := 0; i < count; i++ {
-		newSize *= 2
-	}
-
-	if newSize > p.maxSize {
-		return false
-	}
-
 	edgeNodes := []*Node{}
 	abort := errors.New("abort")
 	aborted := false
 	if p.root != nil {
 		_ = walk(p.root, func(n *Node) error {
-			if n.x+n.width < p.size && n.y+n.height < p.size {
+			if n.x+n.width < p.width && n.y+n.height < p.height {
 				return nil
 			}
 			if n.used {
@@ -296,21 +300,23 @@ func (p *Page) extend(count int) bool {
 		})
 	}
 
+	var rollback func()
+
 	if aborted {
 		origRoot := *p.root
 
 		leftUpper := p.root
 		leftLower := &Node{
 			x:      0,
-			y:      p.size,
+			y:      p.height,
-			width:  p.size,
+			width:  p.width,
-			height: newSize - p.size,
+			height: newHeight - p.height,
 		}
 		left := &Node{
 			x:      0,
 			y:      0,
-			width:  p.size,
+			width:  p.width,
-			height: p.size,
+			height: p.height,
 			child0: leftUpper,
 			child1: leftLower,
 		}
@@ -318,45 +324,47 @@ func (p *Page) extend(count int) bool {
 		leftLower.parent = left
 
 		right := &Node{
-			x:      p.size,
+			x:      p.width,
 			y:      0,
-			width:  newSize - p.size,
+			width:  newWidth - p.width,
-			height: newSize,
+			height: newHeight,
 		}
 		p.root = &Node{
 			x:      0,
 			y:      0,
-			width:  newSize,
+			width:  newWidth,
-			height: newSize,
+			height: newHeight,
 			child0: left,
 			child1: right,
 		}
 		left.parent = p.root
 		right.parent = p.root
 
-		origSize := p.size
+		origWidth, origHeight := p.width, p.height
-		p.rollbackExtension = func() {
+		rollback = func() {
-			p.size = origSize
+			p.width = origWidth
+			p.height = origHeight
 			p.root = &origRoot
 		}
 	} else {
-		origSize := p.size
+		origWidth, origHeight := p.width, p.height
 		origWidths := map[*Node]int{}
 		origHeights := map[*Node]int{}
 
 		for _, n := range edgeNodes {
-			if n.x+n.width == p.size {
+			if n.x+n.width == p.width {
 				origWidths[n] = n.width
-				n.width += newSize - p.size
+				n.width += newWidth - p.width
 			}
-			if n.y+n.height == p.size {
+			if n.y+n.height == p.height {
 				origHeights[n] = n.height
-				n.height += newSize - p.size
+				n.height += newHeight - p.height
 			}
 		}
 
-		p.rollbackExtension = func() {
+		rollback = func() {
-			p.size = origSize
+			p.width = origWidth
+			p.height = origHeight
 			for n, w := range origWidths {
 				n.width = w
 			}
@@ -366,7 +374,8 @@ func (p *Page) extend(count int) bool {
 		}
 	}
 
-	p.size = newSize
+	p.width = newWidth
+	p.height = newHeight
 
-	return true
+	return rollback
 }

internal/packing/packing_test.go

@@ -313,3 +313,17 @@ func TestAllocTooMuch(t *testing.T) {
 		t.Errorf("got: non-nil, want: nil")
 	}
 }
+
+func TestNonSquareAlloc(t *testing.T) {
+	p := packing.NewPage(1024, 16384)
+	n0 := p.Alloc(16384, 1)
+	if _, h := p.Size(); h != 1024 {
+		t.Errorf("got: %d, want: 1024", h)
+	}
+	n1 := p.Alloc(16384, 1)
+	if _, h := p.Size(); h != 1024 {
+		t.Errorf("got: %d, want: 1024", h)
+	}
+	p.Free(n0)
+	p.Free(n1)
+}