Line<int>::getPointAlongLine() bug

General JUCE discussion
1

Line::getPointAlongLineProportionally() does not work properly when ValueType is int, because the argument gets rounded :

Line<int> (0, 0, 100, 0).getPointAlongLineProportionally (0.1f);

returns (0, 0) instead of (10, 0)

that should be changed to something more like that I guess :

template <typename FloatType>
Point<ValueType> getPointAlongLineProportionally (FloatType proportionOfLength) const noexcept
{
    return start + (end - start) * proportionOfLength;
}

there is the same kind of issue with the getPointAlongLine() methods.

2

The Line class is really designed to be used with doubles or floats. Integers will work with the basic methods but a lot of the methods that perform mathematical calculations won’t give the expected result due to rounding.

Ed

3

Well, if it is not supposed to work, then it should really be stated in the doc, and you should add some assertions!
Because it is perfectly legitimate to think that something like that should work :

Line<int> line (0, 0, 100, 0);
Point<int> point = line.getPointAlongLine (50);

and I really think that it should.
But it does not, and it does not raise any warning or assertion.

1 Like
4

It’s stated at the top of the juce_Line.h file here

Ed

5

ok … fair enough… :blush:

6

Maybe a bit off topic but you could actually use std::enable_if to block methods that cannot be used with integral types:
The code below will only allow an implementation for non integral types, if you remove the negation it will only allow integral types.

template<typename T>
typename std::enable_if<!std::is_integral<T>::value>::type func()
{
}
1 Like
7

Well, for getPointAlongLineProportionally() the only meaningful values for its parameter are floating point values, so the implementation proposed by @lalala actually makes more sense than the current one, and shouldn’t break any existing code because it only extends its functionality to int Lines.

Obviously, the returned Point should also be <int> and rounded to the nearest integer coordinates, but I think that’d be implicit in the contract if you are using a Line <int> (and turns out to be very useful in contexts where you are dealing with pixels)

8

That’s a good point - I’ve made the change and pushed it to develop. Thanks @yfede and @lalala!

Ed

9

Maybe it’s also the case to add a unit test to check the behavior in the integer case?