internal/restorable: optimize removeDuplicatedRegions

Updates #2375
Updates #2626
Updates #3083

internal/restorable/export_test.go

@@ -15,8 +15,6 @@
 package restorable
 
 import (
-	"image"
-
 	"github.com/hajimehoshi/ebiten/v2/internal/graphicsdriver"
 )
 
@@ -28,7 +26,3 @@ func EnableRestoringForTesting() {
 func ResolveStaleImages(graphicsDriver graphicsdriver.Graphics) error {
 	return resolveStaleImages(graphicsDriver, false)
 }
-
-func AppendRegionRemovingDuplicates(regions *[]image.Rectangle, region image.Rectangle) {
-	appendRegionRemovingDuplicates(regions, region)
-}

internal/restorable/image.go

@@ -17,6 +17,7 @@ package restorable
 import (
 	"fmt"
 	"image"
+	"sort"
 
 	"github.com/hajimehoshi/ebiten/v2/internal/graphics"
 	"github.com/hajimehoshi/ebiten/v2/internal/graphicscommand"
@@ -215,20 +216,16 @@ func (i *Image) makeStale(rect image.Rectangle) {
 		return
 	}
 
-	var addedRegions []image.Rectangle
+	origSize := len(i.staleRegions)
-	i.appendRegionsForDrawTriangles(&addedRegions)
+	i.staleRegions = i.appendRegionsForDrawTriangles(i.staleRegions)
 	if !rect.Empty() {
-		appendRegionRemovingDuplicates(&addedRegions, rect)
+		i.staleRegions = append(i.staleRegions, rect)
-	}
-
-	for _, rect := range addedRegions {
-		appendRegionRemovingDuplicates(&i.staleRegions, rect)
 	}
 
 	i.clearDrawTrianglesHistory()
 
 	// Clear pixels to save memory.
-	for _, r := range addedRegions {
+	for _, r := range i.staleRegions[origSize:] {
 		i.basePixels.Clear(r)
 	}
 
@@ -450,13 +447,16 @@ func (i *Image) readPixelsFromGPU(graphicsDriver graphicsdriver.Graphics) error
 	if i.stale {
 		rs = i.staleRegions
 	} else {
-		i.appendRegionsForDrawTriangles(&i.regionsCache)
+		i.regionsCache = i.appendRegionsForDrawTriangles(i.regionsCache)
 		defer func() {
 			i.regionsCache = i.regionsCache[:0]
 		}()
 		rs = i.regionsCache
 	}
 
+	// Remove duplications. Is this heavy?
+	rs = rs[:removeDuplicatedRegions(rs)]
+
 	args := make([]graphicsdriver.PixelsArgs, 0, len(rs))
 	for _, r := range rs {
 		if r.Empty() {
@@ -607,7 +607,7 @@ func (i *Image) restore(graphicsDriver graphicsdriver.Graphics) error {
 
 	// In order to clear the draw-triangles history, read pixels from GPU.
 	if len(i.drawTrianglesHistory) > 0 {
-		i.appendRegionsForDrawTriangles(&i.regionsCache)
+		i.regionsCache = i.appendRegionsForDrawTriangles(i.regionsCache)
 		defer func() {
 			i.regionsCache = i.regionsCache[:0]
 		}()
@@ -683,38 +683,54 @@ func (i *Image) InternalSize() (int, int) {
 	return i.image.InternalSize()
 }
 
-func (i *Image) appendRegionsForDrawTriangles(regions *[]image.Rectangle) {
+func (i *Image) appendRegionsForDrawTriangles(regions []image.Rectangle) []image.Rectangle {
 	for _, d := range i.drawTrianglesHistory {
 		if d.dstRegion.Empty() {
 			continue
 		}
-		appendRegionRemovingDuplicates(regions, d.dstRegion)
+		regions = append(regions, d.dstRegion)
 	}
+	return regions
 }
 
-// appendRegionRemovingDuplicates adds a region to a given list of regions,
+// removeDuplicatedRegions removes duplicated regions and returns a shrunk slice.
-// but removes any duplicate between the newly added region and any existing regions.
+// If a region covers preceding regions, the covered regions are removed.
-//
+func removeDuplicatedRegions(regions []image.Rectangle) int {
-// In case the newly added region is fully contained in any pre-existing region, this function does nothing.
+	// Sweep and prune algorithm
-// Otherwise, any pre-existing regions that are fully contained in the newly added region are removed.
+
-//
+	sort.Slice(regions, func(i, j int) bool {
-// This is done to avoid unnecessary reading pixels from GPU.
+		return regions[i].Min.X < regions[j].Min.X
-func appendRegionRemovingDuplicates(regions *[]image.Rectangle, region image.Rectangle) {
+	})
-	for _, r := range *regions {
+
-		if region.In(r) {
+	for i, r := range regions {
-			// The newly added rectangle is fully contained in one of the input regions.
+		if r.Empty() {
-			// Nothing to add.
-			return
-		}
-	}
-	// Separate loop, as regions must not get mutated before above return.
-	n := 0
-	for _, r := range *regions {
-		if r.In(region) {
 			continue
 		}
-		(*regions)[n] = r
+		for j := i + 1; j < len(regions); j++ {
+			rr := regions[j]
+			if rr.Empty() {
+				continue
+			}
+			if r.Max.X <= rr.Min.X {
+				break
+			}
+			if rr.In(r) {
+				regions[j] = image.Rectangle{}
+			} else if r.In(rr) {
+				regions[i] = image.Rectangle{}
+				break
+			}
+		}
+	}
+
+	var n int
+	for _, r := range regions {
+		if r.Empty() {
+			continue
+		}
+		regions[n] = r
 		n++
 	}
-	*regions = append((*regions)[:n], region)
+
+	return n
 }

internal/restorable/rect_test.go

@@ -1,165 +0,0 @@
-// Copyright 2023 The Ebitengine Authors
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package restorable_test
-
-import (
-	"image"
-	"testing"
-
-	"github.com/hajimehoshi/ebiten/v2/internal/restorable"
-)
-
-func areEqualRectangles(a, b []image.Rectangle) bool {
-	if len(a) != len(b) {
-		return false
-	}
-
-	for i := range a {
-		if a[i] != b[i] {
-			return false
-		}
-	}
-
-	return true
-}
-
-func TestRemoveDuplicatedRegions(t *testing.T) {
-	cases := []struct {
-		Regions    []image.Rectangle
-		NewRegions []image.Rectangle
-		Expected   []image.Rectangle
-	}{
-		{
-			Regions:    nil,
-			NewRegions: nil,
-			Expected:   nil,
-		},
-		{
-			NewRegions: []image.Rectangle{
-				image.Rect(0, 0, 2, 2),
-			},
-			Expected: []image.Rectangle{
-				image.Rect(0, 0, 2, 2),
-			},
-		},
-		{
-			NewRegions: []image.Rectangle{
-				image.Rect(0, 0, 2, 2),
-				image.Rect(0, 0, 1, 1),
-			},
-			Expected: []image.Rectangle{
-				image.Rect(0, 0, 2, 2),
-			},
-		},
-		{
-			NewRegions: []image.Rectangle{
-				image.Rect(0, 0, 1, 1),
-				image.Rect(0, 0, 2, 2),
-			},
-			Expected: []image.Rectangle{
-				image.Rect(0, 0, 2, 2),
-			},
-		},
-		{
-			NewRegions: []image.Rectangle{
-				image.Rect(0, 0, 1, 3),
-				image.Rect(0, 0, 2, 2),
-				image.Rect(0, 0, 3, 1),
-			},
-			Expected: []image.Rectangle{
-				image.Rect(0, 0, 1, 3),
-				image.Rect(0, 0, 2, 2),
-				image.Rect(0, 0, 3, 1),
-			},
-		},
-		{
-			NewRegions: []image.Rectangle{
-				image.Rect(0, 0, 1, 3),
-				image.Rect(0, 0, 2, 2),
-				image.Rect(0, 0, 3, 1),
-				image.Rect(0, 0, 4, 4),
-			},
-			Expected: []image.Rectangle{
-				image.Rect(0, 0, 4, 4),
-			},
-		},
-		{
-			NewRegions: []image.Rectangle{
-				image.Rect(0, 0, 1, 3),
-				image.Rect(0, 0, 2, 2),
-				image.Rect(0, 0, 3, 1),
-				image.Rect(0, 0, 4, 4),
-				image.Rect(1, 1, 2, 2),
-			},
-			Expected: []image.Rectangle{
-				image.Rect(0, 0, 4, 4),
-			},
-		},
-		{
-			NewRegions: []image.Rectangle{
-				image.Rect(0, 0, 1, 3),
-				image.Rect(0, 0, 2, 2),
-				image.Rect(0, 0, 3, 1),
-				image.Rect(0, 0, 4, 4),
-				image.Rect(0, 0, 5, 5),
-			},
-			Expected: []image.Rectangle{
-				image.Rect(0, 0, 5, 5),
-			},
-		},
-		{
-			Regions: []image.Rectangle{
-				image.Rect(0, 0, 1, 3),
-				image.Rect(0, 0, 2, 2),
-				image.Rect(0, 0, 3, 1),
-				image.Rect(0, 0, 4, 4),
-			},
-			NewRegions: []image.Rectangle{
-				image.Rect(0, 0, 5, 5),
-			},
-			Expected: []image.Rectangle{
-				image.Rect(0, 0, 5, 5),
-			},
-		},
-		{
-			Regions: []image.Rectangle{
-				image.Rect(0, 0, 2, 2),
-				image.Rect(0, 0, 3, 1),
-				image.Rect(0, 0, 4, 4),
-				image.Rect(0, 0, 5, 5),
-			},
-			NewRegions: []image.Rectangle{
-				image.Rect(0, 0, 1, 3),
-			},
-			Expected: []image.Rectangle{
-				image.Rect(0, 0, 2, 2),
-				image.Rect(0, 0, 3, 1),
-				image.Rect(0, 0, 4, 4),
-				image.Rect(0, 0, 5, 5),
-			},
-		},
-	}
-
-	for _, c := range cases {
-		got := c.Regions
-		for _, r := range c.NewRegions {
-			restorable.AppendRegionRemovingDuplicates(&got, r)
-		}
-		want := c.Expected
-		if !areEqualRectangles(got, want) {
-			t.Errorf("restorable.RemoveDuplicatedRegions(%#v): got: %#v, want: %#v", c.NewRegions, got, want)
-		}
-	}
-}