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

Jump to bottom

Store angles internally as radians #2978

Merged
merged 1 commit into from
May 14, 2024
Merged

Store angles internally as radians #2978

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 10 additions & 4 deletions include/SFML/System/Angle.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,20 +144,20 @@ class Angle
friend constexpr Angle radians(float angle);

////////////////////////////////////////////////////////////
/// \brief Construct from a number of degrees
/// \brief Construct from a number of radians
///
/// This function is internal. To construct angle values,
/// use sf::radians or sf::degrees instead.
///
/// \param degrees Angle in degrees
/// \param radians Angle in radians
///
////////////////////////////////////////////////////////////
constexpr explicit Angle(float degrees);
constexpr explicit Angle(float radians);

////////////////////////////////////////////////////////////
// Member data
////////////////////////////////////////////////////////////
float m_degrees{}; //!< Angle value stored as degrees
float m_radians{}; //!< Angle value stored as radians
};

////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -187,6 +187,7 @@ class Angle
////////////////////////////////////////////////////////////
/// \relates Angle
/// \brief Overload of == operator to compare two angle values
/// \note Does not automatically wrap the angle value
eXpl0it3r marked this conversation as resolved.
Show resolved Hide resolved
///
/// \param left Left operand (an angle)
/// \param right Right operand (an angle)
Expand All @@ -199,6 +200,7 @@ class Angle
////////////////////////////////////////////////////////////
/// \relates Angle
/// \brief Overload of != operator to compare two angle values
/// \note Does not automatically wrap the angle value
///
/// \param left Left operand (an angle)
/// \param right Right operand (an angle)
Expand All @@ -211,6 +213,7 @@ class Angle
////////////////////////////////////////////////////////////
/// \relates Angle
/// \brief Overload of < operator to compare two angle values
/// \note Does not automatically wrap the angle value
///
/// \param left Left operand (an angle)
/// \param right Right operand (an angle)
Expand All @@ -223,6 +226,7 @@ class Angle
////////////////////////////////////////////////////////////
/// \relates Angle
/// \brief Overload of > operator to compare two angle values
/// \note Does not automatically wrap the angle value
///
/// \param left Left operand (an angle)
/// \param right Right operand (an angle)
Expand All @@ -235,6 +239,7 @@ class Angle
////////////////////////////////////////////////////////////
/// \relates Angle
/// \brief Overload of <= operator to compare two angle values
/// \note Does not automatically wrap the angle value
///
/// \param left Left operand (an angle)
/// \param right Right operand (an angle)
Expand All @@ -247,6 +252,7 @@ class Angle
////////////////////////////////////////////////////////////
/// \relates Angle
/// \brief Overload of >= operator to compare two angle values
/// \note Does not automatically wrap the angle value
///
/// \param left Left operand (an angle)
/// \param right Right operand (an angle)
Expand Down
60 changes: 29 additions & 31 deletions include/SFML/System/Angle.inl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,14 @@ namespace sf
{
namespace priv
{
constexpr float pi = 3.141592654f;
constexpr float pi = 3.141592654f;
ChrisThrasher marked this conversation as resolved.
Show resolved Hide resolved
constexpr float tau = pi * 2.f;

constexpr float positiveRemainder(float a, float b)
{
assert(b > 0.0f && "Cannot calculate remainder with non-positive divisor");
assert(b > 0.f && "Cannot calculate remainder with non-positive divisor");
const float val = a - static_cast<float>(static_cast<int>(a / b)) * b;
if (val >= 0.f)
return val;
else
return val + b;
return val >= 0.f ? val : val + b;
}
} // namespace priv

Expand All @@ -55,104 +53,104 @@ constexpr Angle::Angle() = default;
////////////////////////////////////////////////////////////
constexpr float Angle::asDegrees() const
{
return m_degrees;
return m_radians * (180.f / priv::pi);
Copy link

@L0laapk3 L0laapk3 May 13, 2024 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that these constructs already carry a baked in rounding error.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same with calls to asDegrees(), I guess it could have a double overload, I'm not sure if we have to care that much? Just pointing it out

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you saying that we could get marginally better accuracy if we used a single literal to represent 180 / pi rather than using this particular expression?

Copy link

@L0laapk3 L0laapk3 May 14, 2024 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Keyword here is marginally.

I tried to demonstrate that with the godbolt link, a different value is present when I calculated the constant with extra precision and then converted it to float.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

57.295776 - float (Compiler Explorer)
57.2957802 - double (Compiler Explorer)
57.295779513082323402053960025447 - actual (Windows calculator)

0.000003513082323402053960025447 - difference float
0.000000686917676597946039974553 - difference double

I guess in theory you could also do the full math in doubles and only covert at the end, but I feel like if you care about such a precision, you may want to implement the mathy bits yourself.

The change makes sense to me, but could also be a separate PR.

}


////////////////////////////////////////////////////////////
constexpr float Angle::asRadians() const
{
return m_degrees * (priv::pi / 180);
return m_radians;
}


////////////////////////////////////////////////////////////
constexpr Angle Angle::wrapSigned() const
{
return degrees(priv::positiveRemainder(m_degrees + 180, 360) - 180);
return radians(priv::positiveRemainder(m_radians + priv::pi, priv::tau) - priv::pi);
}


////////////////////////////////////////////////////////////
constexpr Angle Angle::wrapUnsigned() const
{
return degrees(priv::positiveRemainder(m_degrees, 360));
return radians(priv::positiveRemainder(m_radians, priv::tau));
}


////////////////////////////////////////////////////////////
constexpr Angle::Angle(float degrees) : m_degrees(degrees)
constexpr Angle::Angle(float radians) : m_radians(radians)
{
}


////////////////////////////////////////////////////////////
constexpr Angle degrees(float angle)
{
return Angle(angle);
return Angle(angle * (priv::pi / 180.f));
}


////////////////////////////////////////////////////////////
constexpr Angle radians(float angle)
{
return Angle(angle * (180 / priv::pi));
return Angle(angle);
}


////////////////////////////////////////////////////////////
constexpr bool operator==(Angle left, Angle right)
{
return left.asDegrees() == right.asDegrees();
return left.asRadians() == right.asRadians();
}


////////////////////////////////////////////////////////////
constexpr bool operator!=(Angle left, Angle right)
{
return left.asDegrees() != right.asDegrees();
return left.asRadians() != right.asRadians();
}


////////////////////////////////////////////////////////////
constexpr bool operator<(Angle left, Angle right)
{
return left.asDegrees() < right.asDegrees();
return left.asRadians() < right.asRadians();
}


////////////////////////////////////////////////////////////
constexpr bool operator>(Angle left, Angle right)
{
return left.asDegrees() > right.asDegrees();
return left.asRadians() > right.asRadians();
}


////////////////////////////////////////////////////////////
constexpr bool operator<=(Angle left, Angle right)
{
return left.asDegrees() <= right.asDegrees();
return left.asRadians() <= right.asRadians();
}


////////////////////////////////////////////////////////////
constexpr bool operator>=(Angle left, Angle right)
{
return left.asDegrees() >= right.asDegrees();
return left.asRadians() >= right.asRadians();
}


////////////////////////////////////////////////////////////
constexpr Angle operator-(Angle right)
{
return degrees(-right.asDegrees());
return radians(-right.asRadians());
}


////////////////////////////////////////////////////////////
constexpr Angle operator+(Angle left, Angle right)
{
return degrees(left.asDegrees() + right.asDegrees());
return radians(left.asRadians() + right.asRadians());
}


Expand All @@ -166,7 +164,7 @@ constexpr Angle& operator+=(Angle& left, Angle right)
////////////////////////////////////////////////////////////
constexpr Angle operator-(Angle left, Angle right)
{
return degrees(left.asDegrees() - right.asDegrees());
return radians(left.asRadians() - right.asRadians());
}


Expand All @@ -180,7 +178,7 @@ constexpr Angle& operator-=(Angle& left, Angle right)
////////////////////////////////////////////////////////////
constexpr Angle operator*(Angle left, float right)
{
return degrees(left.asDegrees() * right);
return radians(left.asRadians() * right);
}


Expand All @@ -201,39 +199,39 @@ constexpr Angle& operator*=(Angle& left, float right)
////////////////////////////////////////////////////////////
constexpr Angle operator/(Angle left, float right)
{
assert(right != 0 && "Angle::operator/ cannot divide by 0");
return degrees(left.asDegrees() / right);
assert(right != 0.f && "Angle::operator/ cannot divide by 0");
return radians(left.asRadians() / right);
}


////////////////////////////////////////////////////////////
constexpr Angle& operator/=(Angle& left, float right)
{
assert(right != 0 && "Angle::operator/= cannot divide by 0");
assert(right != 0.f && "Angle::operator/= cannot divide by 0");
return left = left / right;
}


////////////////////////////////////////////////////////////
constexpr float operator/(Angle left, Angle right)
{
assert(right.asDegrees() != 0 && "Angle::operator/ cannot divide by 0");
return left.asDegrees() / right.asDegrees();
assert(right.asRadians() != 0.f && "Angle::operator/ cannot divide by 0");
return left.asRadians() / right.asRadians();
}


////////////////////////////////////////////////////////////
constexpr Angle operator%(Angle left, Angle right)
{
assert(right.asDegrees() != 0 && "Angle::operator% cannot modulus by 0");
return degrees(priv::positiveRemainder(left.asDegrees(), right.asDegrees()));
assert(right.asRadians() != 0.f && "Angle::operator% cannot modulus by 0");
return radians(priv::positiveRemainder(left.asRadians(), right.asRadians()));
}


////////////////////////////////////////////////////////////
constexpr Angle& operator%=(Angle& left, Angle right)
{
assert(right.asDegrees() != 0 && "Angle::operator%= cannot modulus by 0");
assert(right.asRadians() != 0.f && "Angle::operator%= cannot modulus by 0");
return left = left % right;
}

Expand Down
4 changes: 2 additions & 2 deletions include/SFML/Window/Sensor.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,11 @@ namespace sf::Sensor
enum class Type
{
Accelerometer, //!< Measures the raw acceleration (m/s^2)
Gyroscope, //!< Measures the raw rotation rates (degrees/s)
Gyroscope, //!< Measures the raw rotation rates (radians/s)
Magnetometer, //!< Measures the ambient magnetic field (micro-teslas)
Gravity, //!< Measures the direction and intensity of gravity, independent of device acceleration (m/s^2)
UserAcceleration, //!< Measures the direction and intensity of device acceleration, independent of the gravity (m/s^2)
Orientation //!< Measures the absolute 3D orientation (degrees)
Orientation //!< Measures the absolute 3D orientation (radians)
};

// NOLINTNEXTLINE(readability-identifier-naming)
Expand Down
4 changes: 2 additions & 2 deletions src/SFML/Audio/AudioDevice.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,8 +304,8 @@ void AudioDevice::setCone(const Listener::Cone& cone)

ma_engine_listener_set_cone(&*instance->m_engine,
0,
std::clamp(cone.innerAngle.asRadians(), 0.f, degrees(360).asRadians()),
std::clamp(cone.outerAngle.asRadians(), 0.f, degrees(360).asRadians()),
std::clamp(cone.innerAngle, Angle::Zero, degrees(360.f)).asRadians(),
std::clamp(cone.outerAngle, Angle::Zero, degrees(360.f)).asRadians(),
cone.outerGain);
}

Expand Down
2 changes: 1 addition & 1 deletion src/SFML/Audio/AudioDevice.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ class AudioDevice
Vector3f position{0, 0, 0};
Vector3f direction{0, 0, -1};
Vector3f velocity{0, 0, 0};
Listener::Cone cone{degrees(360), degrees(360), 1};
Listener::Cone cone{degrees(360.f), degrees(360.f), 1};
Vector3f upVector{0, 1, 0};
};

Expand Down
4 changes: 2 additions & 2 deletions src/SFML/Audio/MiniaudioUtils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ struct SavedSettings
float minGain{0.f};
float maxGain{1.f};
float rollOff{1.f};
float innerAngle{degrees(360).asRadians()};
float outerAngle{degrees(360).asRadians()};
float innerAngle{degrees(360.f).asRadians()};
float outerAngle{degrees(360.f).asRadians()};
float outerGain{0.f};
};

Expand Down
4 changes: 2 additions & 2 deletions src/SFML/Audio/SoundSource.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,8 @@ void SoundSource::setCone(const Cone& cone)
{
if (auto* sound = static_cast<ma_sound*>(getSound()))
ma_sound_set_cone(sound,
std::clamp(cone.innerAngle, degrees(0), degrees(360)).asRadians(),
std::clamp(cone.outerAngle, degrees(0), degrees(360)).asRadians(),
std::clamp(cone.innerAngle, Angle::Zero, degrees(360.f)).asRadians(),
std::clamp(cone.outerAngle, Angle::Zero, degrees(360.f)).asRadians(),
cone.outerGain);
}

Expand Down
2 changes: 1 addition & 1 deletion src/SFML/Graphics/CircleShape.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ std::size_t CircleShape::getPointCount() const
////////////////////////////////////////////////////////////
Vector2f CircleShape::getPoint(std::size_t index) const
{
const Angle angle = static_cast<float>(index) / static_cast<float>(m_pointCount) * degrees(360) - degrees(90);
const Angle angle = static_cast<float>(index) / static_cast<float>(m_pointCount) * degrees(360.f) - degrees(90.f);
return Vector2f(m_radius, m_radius) + Vector2f(m_radius, angle);
}

Expand Down
21 changes: 8 additions & 13 deletions src/SFML/Window/iOS/SensorImpl.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,6 @@
namespace
{
unsigned int deviceMotionEnabledCount = 0;

float toDegrees(float radians)
{
return sf::radians(radians).asDegrees();
}
}


Expand Down Expand Up @@ -145,10 +140,10 @@ float toDegrees(float radians)
break;

case Sensor::Type::Gyroscope:
// Rotation rates are given in rad/s, convert to deg/s
value.x = toDegrees(static_cast<float>(manager.gyroData.rotationRate.x));
value.y = toDegrees(static_cast<float>(manager.gyroData.rotationRate.y));
value.z = toDegrees(static_cast<float>(manager.gyroData.rotationRate.z));
// Rotation rates are given in rad/s
value.x = static_cast<float>(manager.gyroData.rotationRate.x);
value.y = static_cast<float>(manager.gyroData.rotationRate.y);
value.z = static_cast<float>(manager.gyroData.rotationRate.z);
break;

case Sensor::Type::Magnetometer:
Expand All @@ -166,10 +161,10 @@ float toDegrees(float radians)
break;

case Sensor::Type::Orientation:
// Absolute rotation (Euler) angles are given in radians, convert to degrees
value.x = toDegrees(static_cast<float>(manager.deviceMotion.attitude.yaw));
value.y = toDegrees(static_cast<float>(manager.deviceMotion.attitude.pitch));
value.z = toDegrees(static_cast<float>(manager.deviceMotion.attitude.roll));
// Absolute rotation (Euler) angles are given in radians
value.x = static_cast<float>(manager.deviceMotion.attitude.yaw);
value.y = static_cast<float>(manager.deviceMotion.attitude.pitch);
value.z = static_cast<float>(manager.deviceMotion.attitude.roll);
break;

default:
Expand Down