From 36c4451a7b39e0503b11218903624e1ea8ee46c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Verschelde?= Date: Thu, 11 Nov 2021 11:31:30 +0100 Subject: [PATCH] Rect2: Clarify docs for `has_point` excluding bottom and right borders Improve tests, as well as documentation for `expand`. --- doc/classes/AABB.xml | 17 ++++- doc/classes/Rect2.xml | 20 +++++- doc/classes/Rect2i.xml | 19 ++++- tests/core/math/test_rect2.h | 136 +++++++++++++++++++++++++++++------ 4 files changed, 167 insertions(+), 25 deletions(-) diff --git a/doc/classes/AABB.xml b/doc/classes/AABB.xml index f353cd19b8a..15d146fba1b 100644 --- a/doc/classes/AABB.xml +++ b/doc/classes/AABB.xml @@ -54,7 +54,22 @@ - Returns this [AABB] expanded to include a given point. + Returns a copy of this [AABB] expanded to include a given point. + [b]Example:[/b] + [codeblocks] + [gdscript] + # position (-3, 2, 0), size (1, 1, 1) + var box = AABB(Vector3(-3, 2, 0), Vector3(1, 1, 1)) + # position (-3, -1, 0), size (3, 4, 2), so we fit both the original AABB and Vector3(0, -1, 2) + var box2 = box.expand(Vector3(0, -1, 2)) + [/gdscript] + [csharp] + // position (-3, 2, 0), size (1, 1, 1) + var box = new AABB(new Vector3(-3, 2, 0), new Vector3(1, 1, 1)); + // position (-3, -1, 0), size (3, 4, 2), so we fit both the original AABB and Vector3(0, -1, 2) + var box2 = box.Expand(new Vector3(0, -1, 2)); + [/csharp] + [/codeblocks] diff --git a/doc/classes/Rect2.xml b/doc/classes/Rect2.xml index 01bec10ed80..ad88551d9d2 100644 --- a/doc/classes/Rect2.xml +++ b/doc/classes/Rect2.xml @@ -71,7 +71,22 @@ - Returns this [Rect2] expanded to include a given point. + Returns a copy of this [Rect2] expanded to include a given point. + [b]Example:[/b] + [codeblocks] + [gdscript] + # position (-3, 2), size (1, 1) + var rect = Rect2(Vector2(-3, 2), Vector2(1, 1)) + # position (-3, -1), size (3, 4), so we fit both rect and Vector2(0, -1) + var rect2 = rect.expand(Vector2(0, -1)) + [/gdscript] + [csharp] + # position (-3, 2), size (1, 1) + var rect = new Rect2(new Vector2(-3, 2), new Vector2(1, 1)); + # position (-3, -1), size (3, 4), so we fit both rect and Vector2(0, -1) + var rect2 = rect.Expand(new Vector2(0, -1)); + [/csharp] + [/codeblocks] @@ -121,7 +136,8 @@ - Returns [code]true[/code] if the [Rect2] contains a point. + Returns [code]true[/code] if the [Rect2] contains a point. By convention, the right and bottom edges of the [Rect2] are considered exclusive, so points on these edges are [b]not[/b] included. + [b]Note:[/b] This method is not reliable for [Rect2] with a [i]negative size[/i]. Use [method abs] to get a positive sized equivalent rectangle to check for contained points. diff --git a/doc/classes/Rect2i.xml b/doc/classes/Rect2i.xml index fc27c64fa5a..f5ea6a6c87d 100644 --- a/doc/classes/Rect2i.xml +++ b/doc/classes/Rect2i.xml @@ -69,7 +69,21 @@ - Returns this [Rect2i] expanded to include a given point. + Returns a copy of this [Rect2i] expanded to include a given point. + [codeblocks] + [gdscript] + # position (-3, 2), size (1, 1) + var rect = Rect2i(Vector2i(-3, 2), Vector2i(1, 1)) + # position (-3, -1), size (3, 4), so we fit both rect and Vector2i(0, -1) + var rect2 = rect.expand(Vector2i(0, -1)) + [/gdscript] + [csharp] + # position (-3, 2), size (1, 1) + var rect = new Rect2i(new Vector2i(-3, 2), new Vector2i(1, 1)); + # position (-3, -1), size (3, 4), so we fit both rect and Vector2i(0, -1) + var rect2 = rect.Expand(new Vector2i(0, -1)); + [/csharp] + [/codeblocks] @@ -120,7 +134,8 @@ - Returns [code]true[/code] if the [Rect2i] contains a point. + Returns [code]true[/code] if the [Rect2i] contains a point. By convention, the right and bottom edges of the [Rect2i] are considered exclusive, so points on these edges are [b]not[/b] included. + [b]Note:[/b] This method is not reliable for [Rect2i] with a [i]negative size[/i]. Use [method abs] to get a positive sized equivalent rectangle to check for contained points. diff --git a/tests/core/math/test_rect2.h b/tests/core/math/test_rect2.h index 3d9fe5a32ed..aabb9504618 100644 --- a/tests/core/math/test_rect2.h +++ b/tests/core/math/test_rect2.h @@ -211,26 +211,74 @@ TEST_CASE("[Rect2] Growing") { } TEST_CASE("[Rect2] Has point") { + Rect2 rect = Rect2(0, 100, 1280, 720); CHECK_MESSAGE( - Rect2(0, 100, 1280, 720).has_point(Vector2(500, 600)), + rect.has_point(Vector2(500, 600)), "has_point() with contained Vector2 should return the expected result."); CHECK_MESSAGE( - !Rect2(0, 100, 1280, 720).has_point(Vector2(0, 0)), + !rect.has_point(Vector2(0, 0)), "has_point() with non-contained Vector2 should return the expected result."); CHECK_MESSAGE( - Rect2(0, 100, 1280, 720).has_point(Vector2(0, 110)), - "has_point() with positive Vector2 on left edge should return the expected result."); + rect.has_point(rect.position), + "has_point() with positive size should include `position`."); CHECK_MESSAGE( - !Rect2(0, 100, 1280, 720).has_point(Vector2(1280, 110)), - "has_point() with positive Vector2 on right edge should return the expected result."); + rect.has_point(rect.position + Vector2(1, 1)), + "has_point() with positive size should include `position + (1, 1)`."); + CHECK_MESSAGE( + !rect.has_point(rect.position + Vector2(1, -1)), + "has_point() with positive size should not include `position + (1, -1)`."); + CHECK_MESSAGE( + !rect.has_point(rect.position + rect.size), + "has_point() with positive size should not include `position + size`."); + CHECK_MESSAGE( + !rect.has_point(rect.position + rect.size + Vector2(1, 1)), + "has_point() with positive size should not include `position + size + (1, 1)`."); + CHECK_MESSAGE( + rect.has_point(rect.position + rect.size + Vector2(-1, -1)), + "has_point() with positive size should include `position + size + (-1, -1)`."); + CHECK_MESSAGE( + !rect.has_point(rect.position + rect.size + Vector2(-1, 1)), + "has_point() with positive size should not include `position + size + (-1, 1)`."); CHECK_MESSAGE( - Rect2(-4000, 100, 1280, 720).has_point(Vector2(-4000, 110)), - "has_point() with negative Vector2 on left edge should return the expected result."); + rect.has_point(rect.position + Vector2(0, 10)), + "has_point() with point located on left edge should return true."); CHECK_MESSAGE( - !Rect2(-4000, 100, 1280, 720).has_point(Vector2(-2720, 110)), - "has_point() with negative Vector2 on right edge should return the expected result."); + !rect.has_point(rect.position + Vector2(rect.size.x, 10)), + "has_point() with point located on right edge should return false."); + CHECK_MESSAGE( + rect.has_point(rect.position + Vector2(10, 0)), + "has_point() with point located on top edge should return true."); + CHECK_MESSAGE( + !rect.has_point(rect.position + Vector2(10, rect.size.y)), + "has_point() with point located on bottom edge should return false."); + + /* + // FIXME: Disabled for now until GH-37617 is fixed one way or another. + // More tests should then be written like for the positive size case. + rect = Rect2(0, 100, -1280, -720); + CHECK_MESSAGE( + rect.has_point(rect.position), + "has_point() with negative size should include `position`."); + CHECK_MESSAGE( + !rect.has_point(rect.position + rect.size), + "has_point() with negative size should not include `position + size`."); + */ + + rect = Rect2(-4000, -200, 1280, 720); + CHECK_MESSAGE( + rect.has_point(rect.position + Vector2(0, 10)), + "has_point() with negative position and point located on left edge should return true."); + CHECK_MESSAGE( + !rect.has_point(rect.position + Vector2(rect.size.x, 10)), + "has_point() with negative position and point located on right edge should return false."); + CHECK_MESSAGE( + rect.has_point(rect.position + Vector2(10, 0)), + "has_point() with negative position and point located on top edge should return true."); + CHECK_MESSAGE( + !rect.has_point(rect.position + Vector2(10, rect.size.y)), + "has_point() with negative position and point located on bottom edge should return false."); } TEST_CASE("[Rect2] Intersection") { @@ -429,26 +477,74 @@ TEST_CASE("[Rect2i] Growing") { } TEST_CASE("[Rect2i] Has point") { + Rect2i rect = Rect2i(0, 100, 1280, 720); CHECK_MESSAGE( - Rect2i(0, 100, 1280, 720).has_point(Vector2i(500, 600)), + rect.has_point(Vector2i(500, 600)), "has_point() with contained Vector2i should return the expected result."); CHECK_MESSAGE( - !Rect2i(0, 100, 1280, 720).has_point(Vector2i(0, 0)), + !rect.has_point(Vector2i(0, 0)), "has_point() with non-contained Vector2i should return the expected result."); CHECK_MESSAGE( - Rect2i(0, 100, 1280, 720).has_point(Vector2(0, 110)), - "has_point() with positive Vector2 on left edge should return the expected result."); + rect.has_point(rect.position), + "has_point() with positive size should include `position`."); CHECK_MESSAGE( - !Rect2i(0, 100, 1280, 720).has_point(Vector2(1280, 110)), - "has_point() with positive Vector2 on right edge should return the expected result."); + rect.has_point(rect.position + Vector2i(1, 1)), + "has_point() with positive size should include `position + (1, 1)`."); + CHECK_MESSAGE( + !rect.has_point(rect.position + Vector2i(1, -1)), + "has_point() with positive size should not include `position + (1, -1)`."); + CHECK_MESSAGE( + !rect.has_point(rect.position + rect.size), + "has_point() with positive size should not include `position + size`."); + CHECK_MESSAGE( + !rect.has_point(rect.position + rect.size + Vector2i(1, 1)), + "has_point() with positive size should not include `position + size + (1, 1)`."); + CHECK_MESSAGE( + rect.has_point(rect.position + rect.size + Vector2i(-1, -1)), + "has_point() with positive size should include `position + size + (-1, -1)`."); + CHECK_MESSAGE( + !rect.has_point(rect.position + rect.size + Vector2i(-1, 1)), + "has_point() with positive size should not include `position + size + (-1, 1)`."); CHECK_MESSAGE( - Rect2i(-4000, 100, 1280, 720).has_point(Vector2(-4000, 110)), - "has_point() with negative Vector2 on left edge should return the expected result."); + rect.has_point(rect.position + Vector2i(0, 10)), + "has_point() with point located on left edge should return true."); CHECK_MESSAGE( - !Rect2i(-4000, 100, 1280, 720).has_point(Vector2(-2720, 110)), - "has_point() with negative Vector2 on right edge should return the expected result."); + !rect.has_point(rect.position + Vector2i(rect.size.x, 10)), + "has_point() with point located on right edge should return false."); + CHECK_MESSAGE( + rect.has_point(rect.position + Vector2i(10, 0)), + "has_point() with point located on top edge should return true."); + CHECK_MESSAGE( + !rect.has_point(rect.position + Vector2i(10, rect.size.y)), + "has_point() with point located on bottom edge should return false."); + + /* + // FIXME: Disabled for now until GH-37617 is fixed one way or another. + // More tests should then be written like for the positive size case. + rect = Rect2i(0, 100, -1280, -720); + CHECK_MESSAGE( + rect.has_point(rect.position), + "has_point() with negative size should include `position`."); + CHECK_MESSAGE( + !rect.has_point(rect.position + rect.size), + "has_point() with negative size should not include `position + size`."); + */ + + rect = Rect2i(-4000, -200, 1280, 720); + CHECK_MESSAGE( + rect.has_point(rect.position + Vector2i(0, 10)), + "has_point() with negative position and point located on left edge should return true."); + CHECK_MESSAGE( + !rect.has_point(rect.position + Vector2i(rect.size.x, 10)), + "has_point() with negative position and point located on right edge should return false."); + CHECK_MESSAGE( + rect.has_point(rect.position + Vector2i(10, 0)), + "has_point() with negative position and point located on top edge should return true."); + CHECK_MESSAGE( + !rect.has_point(rect.position + Vector2i(10, rect.size.y)), + "has_point() with negative position and point located on bottom edge should return false."); } TEST_CASE("[Rect2i] Intersection") {