Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rect.Intersect(Rect) and Rect.Intersects(Rect) have conflicting behaviour #277

Closed
btanner opened this issue Mar 14, 2021 · 1 comment
Closed
Labels

Comments

@btanner
Copy link

btanner commented Mar 14, 2021

Rect.Intersect specifically calculates the ZR intersection using inclusive operators:

if t.Min.X >= t.Max.X || t.Min.Y >= t.Max.Y {
		return ZR
}

Rect.Intersects uses exclusive operators:

return !(s.Max.X < r.Min.X ||
	s.Min.X > r.Max.X ||
	s.Max.Y < r.Min.Y ||
	s.Min.Y > r.Max.Y)

I think the former is more convenient, because having 2 rectangles with a perfectly aligned edge is has 0 intersection area. I think I would argue that if the area of intersection is 0, intersects should return false.

We can see with a simple example, this behaviour is a bit inconsistent. There are no tests for either right now.

I learned this making a small game with my son, and the overhead of changing and adding tests and creating a PR felt a bit high. But, can do if you need.

Program to repro:

package main

import (
	"fmt"
	"github.com/faiface/pixel"
)

func main() {
	r1 := pixel.R(0, 0, 10, 10)
	r2 := pixel.R(10, 10, 20, 20) 	// No real overlap, just corners touching.
	fmt.Println(r1.Intersect(r2)) 	// Output Rect(0, 0, 0, 0)
	fmt.Println(r1.Intersects(r2)) 	// Output: True
}
dusk125 added a commit that referenced this issue Aug 19, 2021
Fixing #277: Rect.Intersect and Rect.Intersects now have consistent behavior
@dusk125
Copy link
Collaborator

dusk125 commented Aug 19, 2021

Thanks for bringing this to our attention, this should now be fixed! :) Closing

@dusk125 dusk125 closed this as completed Aug 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants