internal/packing: refactoring

Now pages are automatically extended by Alloc.

Updates #2327

internal/atlas/image.go

@@ -139,33 +139,14 @@ type backend struct {
 }
 
 func (b *backend) tryAlloc(width, height int) (*packing.Node, bool) {
-	// If the region is allocated without any extension, that's fine.
+	n := b.page.Alloc(width, height)
-	if n := b.page.Alloc(width, height); n != nil {
-		return n, true
-	}
-
-	nExtended := 1
-	var n *packing.Node
-	for {
-		if !b.page.Extend(nExtended) {
-			// The page can't be extended any more. Return as failure.
-			return nil, false
-		}
-		nExtended++
-		n = b.page.Alloc(width, height)
-		if n != nil {
-			b.page.CommitExtension()
-			break
-		}
-		b.page.RollbackExtension()
-	}
-
-	s := b.page.Size()
-	b.restorable = b.restorable.Extend(s, s)
-
 	if n == nil {
-		panic("atlas: Alloc result must not be nil at TryAlloc")
+		// The page can't be extended any more. Return as failure.
+		return nil, false
 	}
+
+	b.restorable = b.restorable.Extend(b.page.Size())
+
 	return n, true
 }
 

internal/packing/packing.go

@@ -84,7 +84,33 @@ func square(width, height int) float64 {
 	return float64(height) / float64(width)
 }
 
-func (p *Page) alloc(n *Node, width, height int) *Node {
+func (p *Page) canAlloc(n *Node, width, height int) bool {
+	if p.root == nil {
+		return p.size >= width && p.size >= height
+	}
+	return canAlloc(p.root, width, height)
+}
+
+func canAlloc(n *Node, width, height int) bool {
+	if n.width < width || n.height < height {
+		return false
+	}
+	if n.used {
+		return false
+	}
+	if n.child0 == nil && n.child1 == nil {
+		return true
+	}
+	if canAlloc(n.child0, width, height) {
+		return true
+	}
+	if canAlloc(n.child1, width, height) {
+		return true
+	}
+	return false
+}
+
+func alloc(n *Node, width, height int) *Node {
 	if n.width < width || n.height < height {
 		return nil
 	}
@@ -129,22 +155,22 @@ func (p *Page) alloc(n *Node, width, height int) *Node {
 				parent: n,
 			}
 		}
-		return p.alloc(n.child0, width, height)
+		return alloc(n.child0, width, height)
 	}
 	if n.child0 == nil || n.child1 == nil {
 		panic("packing: both two children must not be nil at alloc")
 	}
-	if node := p.alloc(n.child0, width, height); node != nil {
+	if node := alloc(n.child0, width, height); node != nil {
 		return node
 	}
-	if node := p.alloc(n.child1, width, height); node != nil {
+	if node := alloc(n.child1, width, height); node != nil {
 		return node
 	}
 	return nil
 }
 
-func (p *Page) Size() int {
+func (p *Page) Size() (int, int) {
-	return p.size
+	return p.size, p.size
 }
 
 func (p *Page) SetMaxSize(size int) {
@@ -158,6 +184,11 @@ func (p *Page) Alloc(width, height int) *Node {
 	if width <= 0 || height <= 0 {
 		panic("packing: width and height must > 0")
 	}
+
+	if !p.extendFor(width, height) {
+		return nil
+	}
+
 	if p.root == nil {
 		p.root = &Node{
 			width:  p.size,
@@ -170,8 +201,7 @@ func (p *Page) Alloc(width, height int) *Node {
 	if height < minSize {
 		height = minSize
 	}
-	n := p.alloc(p.root, width, height)
+	return alloc(p.root, width, height)
-	return n
 }
 
 func (p *Page) Free(node *Node) {
@@ -209,7 +239,29 @@ func walk(n *Node, f func(n *Node) error) error {
 	return nil
 }
 
-func (p *Page) Extend(count int) bool {
+func (p *Page) extendFor(width, height int) bool {
+	if p.canAlloc(p.root, width, height) {
+		return true
+	}
+
+	nExtended := 1
+	for {
+		if !p.extend(nExtended) {
+			// The page can't be extended any more. Return as failure.
+			return false
+		}
+		nExtended++
+		if p.canAlloc(p.root, width, height) {
+			p.rollbackExtension = nil
+			break
+		}
+		p.rollbackExtension()
+		p.rollbackExtension = nil
+	}
+	return true
+}
+
+func (p *Page) extend(count int) bool {
 	if p.rollbackExtension != nil {
 		panic("packing: Extend cannot be called without rolling back or committing")
 	}
@@ -318,20 +370,3 @@ func (p *Page) Extend(count int) bool {
 
 	return true
 }
-
-// RollbackExtension rollbacks Extend call once.
-func (p *Page) RollbackExtension() {
-	if p.rollbackExtension == nil {
-		panic("packing: RollbackExtension cannot be called without Extend")
-	}
-	p.rollbackExtension()
-	p.rollbackExtension = nil
-}
-
-// CommitExtension commits Extend call.
-func (p *Page) CommitExtension() {
-	if p.rollbackExtension == nil {
-		panic("packing: RollbackExtension cannot be called without Extend")
-	}
-	p.rollbackExtension = nil
-}

internal/packing/packing_test.go

@@ -253,109 +253,57 @@ func TestPage(t *testing.T) {
 	}
 }
 
-func TestExtend(t *testing.T) {
+func TestAlloc(t *testing.T) {
 	p := packing.NewPage(1024, 4096)
-	s := p.Size()
+	w, h := p.Size()
-	p.Alloc(s/2, s/2)
+	p.Alloc(w/2, h/2)
-	p.Extend(1)
+
-	if p.Size() != s*2 {
+	n0 := p.Alloc(w*3/2, h*2)
-		t.Errorf("p.Size(): got: %d, want: %d", p.Size(), s*2)
-	}
-	n0 := p.Alloc(s*3/2, s*2)
 	if n0 == nil {
-		t.Errorf("p.Alloc failed: width: %d, height: %d", s*3/2, s*2)
+		t.Errorf("p.Alloc failed: width: %d, height: %d", w*3/2, h*2)
 	}
-	n1 := p.Alloc(s/2, s*3/2)
+	n1 := p.Alloc(w/2, h*3/2)
 	if n1 == nil {
-		t.Errorf("p.Alloc failed: width: %d, height: %d", s/2, s*3/2)
+		t.Errorf("p.Alloc failed: width: %d, height: %d", w/2, h*3/2)
-	}
-	if p.Alloc(1, 1) != nil {
-		t.Errorf("p.Alloc must fail: width: %d, height: %d", 1, 1)
 	}
 	p.Free(n1)
 	p.Free(n0)
-
-	p.RollbackExtension()
-
-	if got, want := p.Size(), s; got != want {
-		t.Errorf("p.Size(): got: %d, want: %d", got, want)
-	}
-	if p.Alloc(s*3/2, s*2) != nil {
-		t.Errorf("p.Alloc(%d, %d) must fail but not", s*3/2, s*2)
-	}
-	if p.Alloc(s/2, s*3/2) != nil {
-		t.Errorf("p.Alloc(%d, %d) must fail but not", s/2, s*3/2)
-	}
 }
 
-func TestExtend2(t *testing.T) {
+func TestAlloc2(t *testing.T) {
 	p := packing.NewPage(1024, 4096)
-	s := p.Size()
+	w, h := p.Size()
-	p.Alloc(s/2, s/2)
+	p.Alloc(w/2, h/2)
-	n1 := p.Alloc(s/2, s/2)
+	n1 := p.Alloc(w/2, h/2)
-	n2 := p.Alloc(s/2, s/2)
+	n2 := p.Alloc(w/2, h/2)
-	p.Alloc(s/2, s/2)
+	p.Alloc(w/2, h/2)
 	p.Free(n1)
 	p.Free(n2)
-	p.Extend(1)
-	if p.Size() != s*2 {
-		t.Errorf("p.Size(): got: %d, want: %d", p.Size(), s*2)
-	}
 
-	n3 := p.Alloc(s, s*2)
+	n3 := p.Alloc(w, h*2)
 	if n3 == nil {
-		t.Errorf("p.Alloc failed: width: %d, height: %d", s, s*2)
+		t.Errorf("p.Alloc failed: width: %d, height: %d", w, h*2)
 	}
-	n4 := p.Alloc(s, s)
+	n4 := p.Alloc(w, h)
 	if n4 == nil {
-		t.Errorf("p.Alloc failed: width: %d, height: %d", s, s)
+		t.Errorf("p.Alloc failed: width: %d, height: %d", w, h)
-	}
-	if p.Alloc(s, s) != nil {
-		t.Errorf("p.Alloc must fail: width: %d, height: %d", s, s)
 	}
 	p.Free(n4)
 	p.Free(n3)
+}
 
-	p.RollbackExtension()
+func TestAllocJustSize(t *testing.T) {
-
+	p := packing.NewPage(1024, 4096)
-	if got, want := p.Size(), s; got != want {
+	if p.Alloc(4096, 4096) == nil {
-		t.Errorf("p.Size(): got: %d, want: %d", got, want)
+		t.Errorf("got: nil, want: non-nil")
-	}
-	if p.Alloc(s, s*2) != nil {
-		t.Errorf("p.Alloc(%d, %d) must fail but not", s, s*2)
-	}
-	if p.Alloc(s, s) != nil {
-		t.Errorf("p.Alloc(%d, %d) must fail but not", s, s)
 	}
 }
 
 // Issue #1454
-func TestExtendTooMuch(t *testing.T) {
+func TestAllocTooMuch(t *testing.T) {
 	p := packing.NewPage(1024, 4096)
 	p.Alloc(1, 1)
-	if got, want := p.Extend(3), false; got != want {
+	if p.Alloc(4096, 4096) != nil {
-		t.Errorf("got: %t, want: %t", got, want)
+		t.Errorf("got: non-nil, want: nil")
-	}
-}
-
-func TestExtendWithoutAllocation(t *testing.T) {
-	p := packing.NewPage(1024, 4096)
-
-	if got, want := p.Extend(2), true; got != want {
-		t.Errorf("got: %t, want: %t", got, want)
-	}
-
-	p.RollbackExtension()
-	if got, want := p.Size(), 1024; got != want {
-		t.Errorf("p.Size(): got: %d, want: %d", got, want)
-	}
-
-	if got, want := p.Extend(2), true; got != want {
-		t.Errorf("got: %t, want: %t", got, want)
-	}
-
-	p.CommitExtension()
-	if got, want := p.Size(), 4096; got != want {
-		t.Errorf("p.Size(): got: %d, want: %d", got, want)
 	}
 }