Skip to content

Commit

Permalink
Math: make one-dimensional range just from scalar types.
Browse files Browse the repository at this point in the history
Since Range1D is now used all over Animation, the vector made it very
annoying to use. That's fixed now. This is a backwards-incompatible
change, but I don't expect the 1D range to be used much, mainly because
it was so shitty to use. Generic code that needs a vector can always
cast to it, like this:

    Math::Vector<dimensions, T>{range.min()}

Test for the constructor from pair is no longer accepting pairs of 1D
vectors. I have no idea what I meant by that test case (it's testing the
same thing twice), so I removed one of these.
  • Loading branch information
mosra committed Oct 21, 2018
1 parent d6017ad commit 0226ab2
Show file tree
Hide file tree
Showing 10 changed files with 258 additions and 40 deletions.
6 changes: 6 additions & 0 deletions doc/changelog.dox
Original file line number Diff line number Diff line change
Expand Up @@ -528,6 +528,12 @@ See also:
working to fail with an assertion. See the new
@ref Math::Matrix3::rotationShear() and @ref Math::Matrix4::rotationShear()
accessors for a possible solution.
- Since one-dimensional @ref Math::Range is now used on many new places such
as the @ref Animation library, its underlying type is just @cpp T @ce
instead of @ref Math::Vector "Math::Vector<1, T>" to make it easier to use.
This may break rare existing code that was using one-dimensional ranges or
generic code that's always expecting a vector type. See
@ref Math-Range-generic for possible solutions.
- @ref SceneGraph::Camera::setViewport() and @ref SceneGraph::Camera::draw()
are no longer @cpp virtual @ce functions. If you need to override their
functionality, simply call the subclass implementation directly instead of
Expand Down
13 changes: 13 additions & 0 deletions doc/snippets/MagnumMath.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -873,6 +873,19 @@ Debug{} << Math::Vector3<UnsignedShort>{a}; // prints {16968, 48552, 15993}
/* [Half-usage-vector] */
}

{
Range1D range, a, b;
constexpr UnsignedInt dimensions = 1;
typedef Float T;
/* [Range-generic] */
Math::Vector<dimensions, T> min = range.min(); // works for 1D, 2D and 3D

T c = Math::max<dimensions, T>(a.size(), b.size()).product(); // vector max()
/* [Range-generic] */
static_cast<void>(min);
static_cast<void>(c);
}

{
/* [Range-construct-minmax] */
Vector3 a, b, c;
Expand Down
6 changes: 3 additions & 3 deletions src/Magnum/Animation/Player.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ template<class T, class K> std::pair<UnsignedInt, K> Player<T, K>::elapsed(const
T startTime = _startTime;
T pauseTime = _stopPauseTime;
State state = _state;
const K duration = _duration.size()[0];
const K duration = _duration.size();
const Containers::Optional<std::pair<UnsignedInt, K>> elapsed = Implementation::playerElapsed(duration, _playCount, _scaler, time, startTime, pauseTime, state);
if(elapsed) return *elapsed;

Expand Down Expand Up @@ -282,12 +282,12 @@ template<class T, class K> std::pair<UnsignedInt, K> Player<T, K>::elapsed(const
template<class T, class K> Player<T, K>& Player<T, K>::advance(const T time) {
/* Get the elapsed time. If we shouldn't advance anything (player already
stopped / not yet playing, quit */
Containers::Optional<std::pair<UnsignedInt, K>> elapsed = Implementation::playerElapsed(_duration.size()[0], _playCount, _scaler, time, _startTime, _stopPauseTime, _state);
Containers::Optional<std::pair<UnsignedInt, K>> elapsed = Implementation::playerElapsed(_duration.size(), _playCount, _scaler, time, _startTime, _stopPauseTime, _state);
if(!elapsed) return *this;

/* Advance all tracks. Properly handle durations that don't start at 0. */
for(Track& t: _tracks)
t.advancer(t.track, _duration.min()[0] + elapsed->second, t.hint, t.destination, t.userCallback, t.userCallbackData);
t.advancer(t.track, _duration.min() + elapsed->second, t.hint, t.destination, t.userCallback, t.userCallbackData);

return *this;
}
Expand Down
2 changes: 1 addition & 1 deletion src/Magnum/Animation/Test/PlayerCustomTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ void PlayerCustomTest::test() {
player.add(Track, value)
.play(2000000ull);

CORRADE_COMPARE(player.duration().size()[0], 24*3);
CORRADE_COMPARE(player.duration().size(), 24*3);

/* 1.75 secs in */
player.advance(3750000ull);
Expand Down
4 changes: 2 additions & 2 deletions src/Magnum/Animation/Test/PlayerTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1230,7 +1230,7 @@ void PlayerTest::runFor100YearsFloat() {

/* The track must fit an integer number of times into the day for this test
to work (3 seconds do fit) */
CORRADE_COMPARE(player.duration().size()[0], 3.0f);
CORRADE_COMPARE(player.duration().size(), 3.0f);

CORRADE_COMPARE(player.state(), State::Playing);
CORRADE_COMPARE(value, -1.0f);
Expand Down Expand Up @@ -1277,7 +1277,7 @@ void PlayerTest::runFor100YearsChrono() {

/* The track must fit an integer number of times into the day for this test
to work (3 seconds do fit) */
CORRADE_COMPARE(player.duration().size()[0], 3.0f);
CORRADE_COMPARE(player.duration().size(), 3.0f);

CORRADE_COMPARE(player.state(), State::Playing);
CORRADE_COMPARE(value, -1.0f);
Expand Down
8 changes: 4 additions & 4 deletions src/Magnum/GL/AbstractTexture.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1795,7 +1795,7 @@ template<UnsignedInt dimensions> void AbstractTexture::subImage(const GLint leve

const Math::Vector<dimensions, Int> size = range.size();
const std::size_t dataSize = Magnum::Implementation::imageDataSizeFor(image, size);
const Vector3i paddedOffset = Vector3i::pad(range.min());
const Vector3i paddedOffset = Vector3i::pad<dimensions>(range.min());
const Vector3i paddedSize = Vector3i::pad(size, 1);

/* Reallocate only if needed */
Expand All @@ -1818,7 +1818,7 @@ template<UnsignedInt dimensions> void AbstractTexture::subImage(const GLint leve

const Math::Vector<dimensions, Int> size = range.size();
const std::size_t dataSize = Magnum::Implementation::imageDataSizeFor(image, size);
const Vector3i paddedOffset = Vector3i::pad(range.min());
const Vector3i paddedOffset = Vector3i::pad<dimensions>(range.min());
const Vector3i paddedSize = Vector3i::pad(size, 1);

/* Reallocate only if needed */
Expand Down Expand Up @@ -1848,7 +1848,7 @@ template<UnsignedInt dimensions> void AbstractTexture::compressedSubImage(const
createIfNotAlready();

const Math::Vector<dimensions, Int> size = range.size();
const Vector3i paddedOffset = Vector3i::pad(range.min());
const Vector3i paddedOffset = Vector3i::pad<dimensions>(range.min());
const Vector3i paddedSize = Vector3i::pad(size, 1);

/* Internal texture format */
Expand Down Expand Up @@ -1882,7 +1882,7 @@ template<UnsignedInt dimensions> void AbstractTexture::compressedSubImage(const
createIfNotAlready();

const Math::Vector<dimensions, Int> size = range.size();
const Vector3i paddedOffset = Vector3i::pad(range.min());
const Vector3i paddedOffset = Vector3i::pad<dimensions>(range.min());
const Vector3i paddedSize = Vector3i::pad(size, 1);

/* Internal texture format */
Expand Down
4 changes: 2 additions & 2 deletions src/Magnum/GL/Renderer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ Range1D Renderer::lineWidthRange() {
Range1D& value = state.lineWidthRange;

/* Get the value, if not already cached */
if(value.max().isZero())
if(!value.max())
value = state.lineWidthRangeImplementation();

return value;
Expand All @@ -55,7 +55,7 @@ Range1D Renderer::lineWidthRangeImplementationDefault() {
#ifndef MAGNUM_TARGET_GLES
Range1D Renderer::lineWidthRangeImplementationMesaForwardCompatible() {
Range1D value = lineWidthRangeImplementationDefault();
value.max() = Math::min(1.0f, value.max()[0]);
value.max() = Math::min(1.0f, value.max());
return value;
}
#endif
Expand Down
2 changes: 1 addition & 1 deletion src/Magnum/GL/Test/RendererGLTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ void RendererGLTest::maxLineWidth() {

MAGNUM_VERIFY_NO_GL_ERROR();

Renderer::setLineWidth(lineWidthRange.max()[0]);
Renderer::setLineWidth(lineWidthRange.max());

MAGNUM_VERIFY_NO_GL_ERROR();
}
Expand Down
76 changes: 57 additions & 19 deletions src/Magnum/Math/Range.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ namespace Magnum { namespace Math {
namespace Implementation {
template<UnsignedInt, class> struct RangeTraits;

template<class T> struct RangeTraits<1, T> { typedef Vector<1, T> Type; };
template<class T> struct RangeTraits<1, T> { typedef T Type; };
template<class T> struct RangeTraits<2, T> { typedef Vector2<T> Type; };
template<class T> struct RangeTraits<3, T> { typedef Vector3<T> Type; };

Expand All @@ -47,9 +47,19 @@ namespace Implementation {
/**
@brief N-dimensional range
Axis-aligned line (in 1D), rectangle (in 2D) or cube (in 3D). Minimal
Axis-aligned line (in 1D), rectangle (in 2D) or cube (in 3D). The minimal
coordinate is inclusive, maximal exclusive. See @ref Range1D, @ref Range2D and
@ref Range3D specializations for given dimension count.
@section Math-Range-generic Use in generic code
While @ref Range2D and @ref Range3D have a vector underlying type, @ref Range1D
is just a scalar. This makes common usage simpler, but may break in generic
code that expects a vector type for any dimension. Solution is to
unconditionally cast the value to a vector type or explicitly specify template
parameters to choose a vector function overload. For example:
@snippet MagnumMath.cpp Range-generic
*/
template<UnsignedInt dimensions, class T> class Range {
template<UnsignedInt, class> friend class Range;
Expand All @@ -58,8 +68,8 @@ template<UnsignedInt dimensions, class T> class Range {
/**
* @brief Underlying vector type
*
* @ref Math::Vector "Vector<1, T>" in 1D, @ref Math::Vector2 "Vector2<T>"
* in 2D, @ref Math::Vector3 "Vector3<T>" in 3D.
* @cpp T @ce in 1D, @ref Math::Vector2 "Vector2<T>" in 2D,
* @ref Math::Vector3 "Vector3<T>" in 3D.
*/
typedef typename Implementation::RangeTraits<dimensions, T>::Type VectorType;

Expand Down Expand Up @@ -91,10 +101,10 @@ template<UnsignedInt dimensions, class T> class Range {
*
* Construct zero-size range positioned at origin.
*/
constexpr /*implicit*/ Range(ZeroInitT = ZeroInit) noexcept: _min{ZeroInit}, _max{ZeroInit} {}
constexpr /*implicit*/ Range(ZeroInitT = ZeroInit) noexcept: Range<dimensions, T>{ZeroInit, typename std::conditional<dimensions == 1, void*, ZeroInitT*>::type{}} {}

/** @brief Construct without initializing the contents */
explicit Range(NoInitT) noexcept: _min{NoInit}, _max{NoInit} {}
explicit Range(NoInitT) noexcept: Range<dimensions, T>{NoInit, typename std::conditional<dimensions == 1, void*, NoInitT*>::type{}} {}

/** @brief Construct a range from minimal and maximal coordinates */
constexpr /*implicit*/ Range(const VectorType& min, const VectorType& max) noexcept: _min{min}, _max{max} {}
Expand Down Expand Up @@ -143,21 +153,23 @@ template<UnsignedInt dimensions, class T> class Range {
}

/** @brief Equality comparison */
constexpr bool operator==(const Range<dimensions, T>& other) const {
return _min == other._min && _max == other._max;
}
bool operator==(const Range<dimensions, T>& other) const;

/** @brief Non-equality comparison */
constexpr bool operator!=(const Range<dimensions, T>& other) const {
bool operator!=(const Range<dimensions, T>& other) const {
return !operator==(other);
}

/**
* @brief Raw data
* @return One-dimensional array of `dimensions`*2 length.
*/
T* data() { return _min.data(); }
constexpr const T* data() const { return _min.data(); } /**< @overload */
T* data() {
return dataInternal(typename std::conditional<dimensions == 1, void*, T*>::type{});
}
constexpr const T* data() const {
return dataInternal(typename std::conditional<dimensions == 1, void*, T*>::type{});
} /**< @overload */

/**
* @brief Minimal coordinates (inclusive)
Expand Down Expand Up @@ -258,7 +270,8 @@ template<UnsignedInt dimensions, class T> class Range {
* @ref min(), @ref max()
*/
bool contains(const VectorType& b) const {
return (b >= _min).all() && (b < _max).all();
return (Vector<dimensions, T>{b} >= _min).all() &&
(Vector<dimensions, T>{b} < _max).all();
}

/**
Expand All @@ -276,10 +289,27 @@ template<UnsignedInt dimensions, class T> class Range {
* @ref min(), @ref max()
*/
bool contains(const Range<dimensions, T>& b) const {
return (b._min >= _min).all() && (b._max <= _max).all();
return (Vector<dimensions, T>{b._min} >= _min).all() &&
(Vector<dimensions, T>{b._max} <= _max).all();
}

private:
/* Called from Range(ZeroInit), either using the ZeroInit constructor
(if available) or passing zero directly (for scalar types) */
constexpr explicit Range(ZeroInitT, ZeroInitT*) noexcept: _min{ZeroInit}, _max{ZeroInit} {}
constexpr explicit Range(ZeroInitT, void*) noexcept: _min{T(0)}, _max{T(0)} {}

/* Called from Range(NoInit), either using the NoInit constructor (if
available) or not doing anything */
explicit Range(NoInitT, NoInitT*) noexcept: _min{NoInit}, _max{NoInit} {}
explicit Range(NoInitT, void*) noexcept {}

/* Called from data(), always returning a T* */
constexpr const VectorType* dataInternal(void*) const { return &_min; }
VectorType* dataInternal(void*) { return &_min; }
constexpr const T* dataInternal(T*) const { return _min.data(); }
T* dataInternal(T*) { return _min.data(); }

VectorType _min, _max;
};

Expand Down Expand Up @@ -693,21 +723,29 @@ are undefined if any range has a negative size.
@ref Range::max()
*/
template<UnsignedInt dimensions, class T> inline bool intersects(const Range<dimensions, T>& a, const Range<dimensions, T>& b) {
return (a.max() > b.min()).all() && (a.min() < b.max()).all();
return (Vector<dimensions, T>{a.max()} > b.min()).all() &&
(Vector<dimensions, T>{a.min()} < b.max()).all();
}

/** @debugoperator{Range} */
template<UnsignedInt dimensions, class T> Corrade::Utility::Debug& operator<<(Corrade::Utility::Debug& debug, const Range<dimensions, T>& value) {
debug << "Range({" << Corrade::Utility::Debug::nospace << value.min()[0];
debug << "Range({" << Corrade::Utility::Debug::nospace << Vector<dimensions, T>{value.min()}[0];
for(UnsignedInt i = 1; i != dimensions; ++i)
debug << Corrade::Utility::Debug::nospace << "," << value.min()[i];
debug << Corrade::Utility::Debug::nospace << "," << Vector<dimensions, T>{value.min()}[i];
debug << Corrade::Utility::Debug::nospace << "}, {"
<< Corrade::Utility::Debug::nospace << value.max()[0];
<< Corrade::Utility::Debug::nospace << Vector<dimensions, T>{value.max()}[0];
for(UnsignedInt i = 1; i != dimensions; ++i)
debug << Corrade::Utility::Debug::nospace << "," << value.max()[i];
debug << Corrade::Utility::Debug::nospace << "," << Vector<dimensions, T>{value.max()}[i];
return debug << Corrade::Utility::Debug::nospace << "})";
}

template<UnsignedInt dimensions, class T> inline bool Range<dimensions, T>::operator==(const Range<dimensions, T>& other) const {
/* For non-scalar types default implementation of TypeTraits would be used,
which is just operator== */
return TypeTraits<VectorType>::equals(_min, other._min) &&
TypeTraits<VectorType>::equals(_max, other._max);
}

/* Explicit instantiation for commonly used types */
#ifndef DOXYGEN_GENERATING_OUTPUT
extern template MAGNUM_EXPORT Corrade::Utility::Debug& operator<<(Corrade::Utility::Debug&, const Range<1, Float>&);
Expand Down
Loading

0 comments on commit 0226ab2

Please sign in to comment.