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.

Sign up for GitHub

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

Cast to number_t to avoid floating point errors #26629

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+107 −40
Open

Cast to number_t to avoid floating point errors #26629

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
81916be
Cast to number_t to avoid floating point errors
miiizen Feb 20, 2025
8937b8b
Add test
miiizen Feb 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 40 additions & 40 deletions src/framework/draw/types/geometry.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -565,8 +565,8 @@ RectX<T> RectX<T>::united(const RectX<T>& r) const
template<typename T>
RectX<T> RectX<T>::intersected(const RectX<T>& r) const
{
T l1 = m_x;
T r1 = m_x;
number_t<T> l1 = m_x;
number_t<T> r1 = m_x;
if (m_w.is_negative()) {
l1 += m_w;
} else {
Expand All @@ -575,8 +575,8 @@ RectX<T> RectX<T>::intersected(const RectX<T>& r) const
if (l1 == r1) { // null rect
return RectX<T>();
}
T l2 = r.m_x;
T r2 = r.m_x;
number_t<T> l2 = r.m_x;
number_t<T> r2 = r.m_x;
if (r.m_w.is_negative()) {
l2 += r.m_w;
} else {
Expand All @@ -588,8 +588,8 @@ RectX<T> RectX<T>::intersected(const RectX<T>& r) const
if (l1 >= r2 || l2 >= r1) {
return RectX<T>();
}
T t1 = m_y;
T b1 = m_y;
number_t<T> t1 = m_y;
number_t<T> b1 = m_y;
if (m_h.is_negative()) {
t1 += m_h;
} else {
Expand All @@ -598,8 +598,8 @@ RectX<T> RectX<T>::intersected(const RectX<T>& r) const
if (t1 == b1) { // null rect
return RectX<T>();
}
T t2 = r.m_y;
T b2 = r.m_y;
number_t<T> t2 = r.m_y;
number_t<T> b2 = r.m_y;
if (r.m_h.is_negative()) {
t2 += r.m_h;
} else {
Expand All @@ -622,47 +622,47 @@ RectX<T> RectX<T>::intersected(const RectX<T>& r) const
template<typename T>
bool RectX<T>::intersects(const RectX<T>& r) const
{
T l1 = m_x;
T r1 = m_x;
number_t<T> l1 = m_x;
number_t<T> r1 = m_x;
if (m_w.is_negative()) {
l1 += m_w;
} else {
r1 += m_w;
}
if (isEqual(l1, r1)) { // null rect
if (l1 == r1) { // null rect
return false;
}
T l2 = r.m_x;
T r2 = r.m_x;
number_t<T> l2 = r.m_x;
number_t<T> r2 = r.m_x;
if (r.m_w.is_negative()) {
l2 += r.m_w;
} else {
r2 += r.m_w;
}
if (isEqual(l2, r2)) { // null rect
if (l2 == r2) { // null rect
return false;
}
if (l1 >= r2 || l2 >= r1) {
return false;
}
T t1 = m_y;
T b1 = m_y;
number_t<T> t1 = m_y;
number_t<T> b1 = m_y;
if (m_h.is_negative()) {
t1 += m_h;
} else {
b1 += m_h;
}
if (isEqual(t1, b1)) { // null rect
if (t1 == b1) { // null rect
return false;
}
T t2 = r.m_y;
T b2 = r.m_y;
number_t<T> t2 = r.m_y;
number_t<T> b2 = r.m_y;
if (r.m_h.is_negative()) {
t2 += r.m_h;
} else {
b2 += r.m_h;
}
if (isEqual(t2, b2)) { // null rect
if (t2 == b2) { // null rect
return false;
}
if (t1 >= b2 || t2 >= b1) {
Expand All @@ -674,27 +674,27 @@ bool RectX<T>::intersects(const RectX<T>& r) const
template<typename T>
bool RectX<T>::contains(const PointX<T>& p) const
{
T l = m_x;
T r = m_x;
number_t<T> l = m_x;
number_t<T> r = m_x;
if (m_w.is_negative()) {
l += m_w;
} else {
r += m_w;
}
if (isEqual(l, r)) { // null rect
if (l == r) { // null rect
return false;
}
if (p.x() < l || p.x() > r) {
return false;
}
T t = m_y;
T b = m_y;
number_t<T> t = m_y;
number_t<T> b = m_y;
if (m_h.is_negative()) {
t += m_h;
} else {
b += m_h;
}
if (isEqual(t, b)) { // null rect
if (t == b) { // null rect
return false;
}
if (p.y() < t || p.y() > b) {
Expand All @@ -706,47 +706,47 @@ bool RectX<T>::contains(const PointX<T>& p) const
template<typename T>
bool RectX<T>::contains(const RectX<T>& r) const
{
T l1 = m_x;
T r1 = m_x;
number_t<T> l1 = m_x;
number_t<T> r1 = m_x;
if (m_w.is_negative()) {
l1 += m_w;
} else {
r1 += m_w;
}
if (isEqual(l1, r1)) { // null rect
if (l1 == r1) { // null rect
return false;
}
T l2 = r.m_x;
T r2 = r.m_x;
number_t<T> l2 = r.m_x;
number_t<T> r2 = r.m_x;
if (r.m_w.is_negative()) {
l2 += r.m_w;
} else {
r2 += r.m_w;
}
if (isEqual(l2, r2)) { // null rect
if (l2 == r2) { // null rect
return false;
}
if (l2 < l1 || r2 > r1) {
return false;
}
T t1 = m_y;
T b1 = m_y;
number_t<T> t1 = m_y;
number_t<T> b1 = m_y;
if (m_h.is_negative()) {
t1 += m_h;
} else {
b1 += m_h;
}
if (isEqual(t1, b1)) { // null rect
if (t1 == b1) { // null rect
return false;
}
T t2 = r.m_y;
T b2 = r.m_y;
number_t<T> t2 = r.m_y;
number_t<T> b2 = r.m_y;
if (r.m_h.is_negative()) {
t2 += r.m_h;
} else {
b2 += r.m_h;
}
if (isEqual(t2, b2)) { // null rect
if (t2 == b2) { // null rect
return false;
}
if (t2 < t1 || b2 > b1) {
Expand All @@ -758,8 +758,8 @@ bool RectX<T>::contains(const RectX<T>& r) const
template<typename T>
T RectX<T>::distanceTo(const PointX<T>& p) const
{
T dx = std::max({ bottomLeft().x() - p.x(), 0.0, p.x() - bottomRight().x() });
T dy = std::max({ bottomLeft().y() - p.y(), 0.0, p.y() - topLeft().y() });
number_t<T> dx = std::max({ bottomLeft().x() - p.x(), 0.0, p.x() - bottomRight().x() });
number_t<T> dy = std::max({ bottomLeft().y() - p.y(), 0.0, p.y() - topLeft().y() });
return std::sqrt(dx * dx + dy * dy);
}

Expand Down
1 change: 1 addition & 0 deletions src/framework/global/tests/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ set(MODULE_TEST_SRC
${CMAKE_CURRENT_LIST_DIR}/containers_tests.cpp
${CMAKE_CURRENT_LIST_DIR}/version_tests.cpp
${CMAKE_CURRENT_LIST_DIR}/number_tests.cpp
${CMAKE_CURRENT_LIST_DIR}/geometry_tests.cpp
)

include(SetupGTest)
66 changes: 66 additions & 0 deletions src/framework/global/tests/geometry_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/*
* SPDX-License-Identifier: GPL-3.0-only
* MuseScore-CLA-applies
*
* MuseScore
* Music Composition & Notation
*
* Copyright (C) 2025 MuseScore BVBA and others
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 3 as
* published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <https://www.gnu.org/licenses/>.
*/
#include <gtest/gtest.h>

#include "draw/types/geometry.h"

using namespace muse;

class Global_Types_GeometryTests : public ::testing::Test
{
public:
};

TEST_F(Global_Types_GeometryTests, intersects) {
RectX<double> r1 { 10.0, 10.0, 10.0, 10.0 };

// above - no collision
RectX<double> r2 { 10.0, 0.0, 5.0, 5.0 };
EXPECT_FALSE(r1.intersects(r2));
// above - collision
r2 = RectX<double>(10.0, 8.0, 5.0, 5.0);
EXPECT_TRUE(r1.intersects(r2));
// above - touching
r2 = RectX<double>(10.0, 0.0, 10.0, 10.0);
EXPECT_FALSE(r1.intersects(r2));
// above - touching (floating point case)
r1 = RectX<double>(10.0, 10.0, 10.0000002, 10.0);
r2 = RectX<double>(20.0000001, 10.0, 10.0, 10.0);
EXPECT_FALSE(r1.intersects(r2));

r1 = RectX<double>(10.0, 10.0, 10.0, 10.0);
// adjacent - no collision
r2 = RectX<double>(25.0, 10.0, 10.0, 10.0);
EXPECT_FALSE(r1.intersects(r2));
// adjacent - collision
r2 = RectX<double>(17.0, 10.0, 10.0, 10.0);
EXPECT_TRUE(r1.intersects(r2));
// adjacent - touching
r2 = RectX<double>(20.0, 10.0, 10.0, 10.0);
EXPECT_FALSE(r1.intersects(r2));

r1 = RectX<double>(10.0, 10.0, 10.0000002, 10.0);

// adjacent - touching (floating point case)
r2 = RectX<double>(20.0000001, 10.0, 10.0, 10.0);
EXPECT_FALSE(r1.intersects(r2));
}
Loading