Commit 3a209281 authored by Nigel Tao's avatar Nigel Tao

image: fix the overlap check in Rectangle.Intersect.

This is a re-roll of a previous commit,
a855da29, which was rolled back in
14347ee4.

It was rolled back because it broke a unit test in image/gif. The
image/gif code was fixed by 9ef65dbe
"image/gif: fix frame-inside-image bounds checking".

The original commit message:

image: fix the overlap check in Rectangle.Intersect.

The doc comment for Rectangle.Intersect clearly states, "If the two
rectangles do not overlap then the zero rectangle will be returned."
Prior to this fix, calling Intersect on adjacent but non-overlapping
rectangles would return an empty but non-zero rectangle.

The fix essentially changes
if r.Min.X > r.Max.X || r.Min.Y > r.Max.Y { etc }
to
if r.Min.X >= r.Max.X || r.Min.Y >= r.Max.Y { etc }
(note that the > signs have become >= signs), but changing that line to:
if r.Empty() { etc }
seems clearer (and equivalent).

Change-Id: I2e3af1f1686064a573b2e513b39246fe60c03631
Reviewed-on: https://go-review.googlesource.com/36734Reviewed-by: 's avatarRob Pike <r@golang.org>
Run-TryBot: Nigel Tao <nigeltao@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
parent 5faba305
...@@ -161,7 +161,11 @@ func (r Rectangle) Intersect(s Rectangle) Rectangle { ...@@ -161,7 +161,11 @@ func (r Rectangle) Intersect(s Rectangle) Rectangle {
if r.Max.Y > s.Max.Y { if r.Max.Y > s.Max.Y {
r.Max.Y = s.Max.Y r.Max.Y = s.Max.Y
} }
if r.Min.X > r.Max.X || r.Min.Y > r.Max.Y { // Letting r0 and s0 be the values of r and s at the time that the method
// is called, this next line is equivalent to:
//
// if max(r0.Min.X, s0.Min.X) >= min(r0.Max.X, s0.Max.X) || likewiseForY { etc }
if r.Empty() {
return ZR return ZR
} }
return r return r
......
...@@ -28,6 +28,7 @@ func TestRectangle(t *testing.T) { ...@@ -28,6 +28,7 @@ func TestRectangle(t *testing.T) {
rects := []Rectangle{ rects := []Rectangle{
Rect(0, 0, 10, 10), Rect(0, 0, 10, 10),
Rect(10, 0, 20, 10),
Rect(1, 2, 3, 4), Rect(1, 2, 3, 4),
Rect(4, 6, 10, 10), Rect(4, 6, 10, 10),
Rect(2, 3, 12, 5), Rect(2, 3, 12, 5),
...@@ -62,9 +63,9 @@ func TestRectangle(t *testing.T) { ...@@ -62,9 +63,9 @@ func TestRectangle(t *testing.T) {
if err := in(a, s); err != nil { if err := in(a, s); err != nil {
t.Errorf("Intersect: r=%s, s=%s, a=%s, a not in s: %v", r, s, a, err) t.Errorf("Intersect: r=%s, s=%s, a=%s, a not in s: %v", r, s, a, err)
} }
if a.Empty() == r.Overlaps(s) { if isZero, overlaps := a == (Rectangle{}), r.Overlaps(s); isZero == overlaps {
t.Errorf("Intersect: r=%s, s=%s, a=%s: empty=%t same as overlaps=%t", t.Errorf("Intersect: r=%s, s=%s, a=%s: isZero=%t same as overlaps=%t",
r, s, a, a.Empty(), r.Overlaps(s)) r, s, a, isZero, overlaps)
} }
largerThanA := [4]Rectangle{a, a, a, a} largerThanA := [4]Rectangle{a, a, a, a}
largerThanA[0].Min.X-- largerThanA[0].Min.X--
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment