sf::Rect extension

[achpile]

I think it will be useful functions to get position, size and (position + size) of the rectangle class instead of using everywhere in the code sf::Vector2(rect.left, rect.top) use simple rect.getPosition().

I don't want to make a pull request, 'cause some my things (like doxygen comments and formatting) may differ of what supposed to be. So here's a simple patch. And I don't know, how to name function returning (position + size)

diff --git a/include/SFML/Graphics/Rect.hpp b/include/SFML/Graphics/Rect.hpp
index 2880722..d565b5e 100644
--- a/include/SFML/Graphics/Rect.hpp
+++ b/include/SFML/Graphics/Rect.hpp
@@ -146,6 +146,36 @@ public :
     bool intersects(const Rect<T>& rectangle, Rect<T>& intersection) const;
 
     ////////////////////////////////////////////////////////////
+    /// \brief Get the position of the rectangle
+    ///
+    /// This returns the position of the rectangle
+    ///
+    /// \return Rectangle position
+    ///
+    ////////////////////////////////////////////////////////////
+    Vector2<T> getPosition();
+
+    ////////////////////////////////////////////////////////////
+    /// \brief Get the size of the rectangle
+    ///
+    /// This returns the size of the rectangle
+    ///
+    /// \return Rectangle size
+    ///
+    ////////////////////////////////////////////////////////////
+    Vector2<T> getSize();
+
+    ////////////////////////////////////////////////////////////
+    /// \brief Get the position of the far point of the rectangle
+    ///
+    /// This returns the position of the far point of the rectangle
+    ///
+    /// \return Rectangle position of the far point
+    ///
+    ////////////////////////////////////////////////////////////
+    Vector2<T> getPositionFar();
+
+    ////////////////////////////////////////////////////////////
     // Member data
     ////////////////////////////////////////////////////////////
     T left;   ///< Left coordinate of the rectangle
diff --git a/include/SFML/Graphics/Rect.inl b/include/SFML/Graphics/Rect.inl
index 4a92397..779c904 100644
--- a/include/SFML/Graphics/Rect.inl
+++ b/include/SFML/Graphics/Rect.inl
@@ -157,3 +157,27 @@ inline bool operator !=(const Rect<T>& left, const Rect<T>& right)
 {
     return !(left == right);
 }
+
+
+////////////////////////////////////////////////////////////
+template <typename T>
+Vector2<T> Rect<T>::getPosition()
+{
+    return Vector2<T>(left, top);
+}
+
+
+////////////////////////////////////////////////////////////
+template <typename T>
+Vector2<T> Rect<T>::getSize()
+{
+    return Vector2<T>(width, height);
+}
+
+
+////////////////////////////////////////////////////////////
+template <typename T>
+Vector2<T> Rect<T>::getPositionFar()
+{
+    return Vector2<T>(static_cast<T>(left + width), static_cast<T>(top + height));
+}
 

[eXpl0it3r]

Let me bump this thread, so others from the SFML Team get another chance to comment.

[Hiura]

What is the "position" of a rectangle? Its top left corner, its center, its bottom right corner, ...? It's not clear and people might have different answer to that question. So if we really want a function to return a specific point of the rectangle, it should not be called «getPosition» IMO. Same apply for the «far position».

And then, if we have `getTopLeftPoint()`, we should also get the 4 other important points of the rectangle with a similar method for consistency matter. But I feel it would just bloat the API for pretty much nothing.

Since anyone can implement those function according to their specific needs with free function, I'd rather not add those 6 functions in the API. Would you agree?

[Cornstalks]

What is the "position" of a rectangle? Its top left corner, its center, its bottom right corner, ...? It's not clear and people might have different answer to that question. So if we really want a function to return a specific point of the rectangle, it should not be called «getPosition» IMO. Same apply for the «far position».

Presumably, "position" is exactly what it is in the constructor "Rect (const Vector2< T > &position, const Vector2< T > &size)". I get what you're saying, but reality is the Rect already has a sense of "position" (which, from the docs, is "Position of the top-left corner of the rectangle"). I think if you don't want to have a notion of "position" then that constructor ought to be removed. If that constructor stays, though, then Rects should have a proper sense of position.

Personally, I think it might be useful to have getPosition() and getSize() methods that return Vector2. I know you can just access the left/top/width/height members, but being able to concisely get a Vector2 that's consistent with an existing constructor would be useful. I don't really like getPositionFar().

[Hiura]

Presumably, "position" is exactly what it is in the constructor "Rect (const Vector2< T > &position, const Vector2< T > &size)".

Thanks for mentioning it. I think we should rename this parameter to `topLeftPoint` or something.

[Hapax]

getPositionFar() sounds horrible 
When I first read this, I thought that getBottomRight() would be better and then Hiura brings up that the "position" should be topLeft. I think we're onto a winner here... 

[Nexus]

This shows already a typical problem of such features: for consistency reasons, they pull many more behind. When there is an accessor for top-left and a bottom-right corner, why not one for top-right and bottom-left? Why no setter for the corresponding parts? What about vertical and horizontal centers?

It can make sense to provide such an API, but it should be more general than for sf::Rect. Getting corner points is useful for other objects as well (sprites, views, windows, ...). Global functions are standing to reason for this. But I'm not sure whether and how this should be part of SFML; I once wanted to introduce something like that in Thor (that's why there is a header called UniformAccess.hpp), but I was not really convinced by the different attempts I've made.

[Hapax]

Rect already returns the top-left; it's just called position at the moment.
Top-left and bottom-right are common, as are top-left and size. Bottom-right and size can obviously easily converted back and forth, of course.
Rect is the more general API. Sprites and views already return rects so it would automatically be applied to those too.

[Nexus]

Rect already returns the top-left; it's just called position at the moment.

No, there's merely a constructor from position and size, and it only exists for convenience as an alternative to the constructor taking 4 coordinates. sf::Rect has no getter functions, just 4 public members.

Rect is the more general API. Sprites and views already return rects so it would automatically be applied to those too.

sf::Rect can't represent rotated rectangles. And even then, it's not possible to implement something like

void setLeft(sf::Sprite& sprite, float x);

via sf::Rect -- sf::Sprite::setPosition() needs to be called directly.

But as mentioned, I implemented this for some cases, and there are a few non-trivial design decisions if you want to keep it truly generic. Ideally, a user could provide a small set of global functions for his own classes, and then all these accessors like setLeft(), getRightBottom(), getCenter(), ... would work automatically. But this is something that can be implemented on top of SFML -- and there should be a well-designed version of it before even considering the integration into SFML itself.

A sf::Rect::getBottomRight() method is not completely thought through. A simple example is the case where the rectangle has negative width and height (an allowed scenario, e.g. for mirrored sprites) -- now the term "bottom-right" is suddenly lying, as it refers to the top-left corner. And it's not simply fixed by a case differentiation, because no user would expect that. The next step would be "if there is getBottomRight(), there should also be setBottomRight()", which comes not only with the previous problem, but can be implemented with different policies: maintain the size and move the rectangle, or resize the rectangle.

To sum up, additions like a getBottomRight() always sound simple at first glance, but you need to think about the long-term implications on the API whenever you consider adding functions. Often, it's better to keep APIs small and intuitive, especially when they provide all the necessary means to write more powerful functionality on top of them.

[Hapax]

No, there's merely a constructor from position and size, and it only exists for convenience as an alternative to the constructor taking 4 coordinates. sf::Rect has no getter functions, just 4 public members.

Ah, you're right, of course. It doesn't return top and left as a combined type. Still, the "position" being rename topLeft would be a bit more accurate.

I say a "bit" more accurate due to the rest of your post about mirrored sizes. It's all very interesting and, of course, correct. It's true that if the sizes are negative then the corners would no longer be named correctly. To counter this, "position" should be "initialCorner" or "startCorner".
However, the actual member names of Rect do not take into account the possibility of this negative sizing, with them being labelled as left and top. If the width and/or height are negative, left and top become right and bottom, depending on which are negative. I do not find this a problem as most of us would "read" a rectangle as "first corner to second corner" and automatically associate left and top with first corner. This brings us full circle where topLeft and bottomRight start looking okay again.

Strangely enough, though, I think I am playing Devil's advocate, as I don't see any "need" to provide so-called bottom-right location as it's easy enough to calculate using the members.

I do think the original "get vector" idea makes quite a bit of sense but could easily be coded on top of SFML.

That said, startCorner and endCorner make sense as locations if you're taking into account the mirroring. So, instead of "position", it should really be "startCorner", "startPosition" or just "start".

getStart() and getSize() seems reasonably useful functions.
getEnd() is just getStart() + getSize() since the two vectors can be added together 

[Cornstalks]

I think "position" and "size" are good. No need for the ability to get all the corners.

I might be the only one who wants this, but I feel like the notion of top/left (or any other specific corner) should be banished. Allow rects with a negative width/height. Then, "position" is just the anchor point for the rect, which idiomatically is the top-left corner, but no longer forces the notion of top/left, and then the user is free to make the anchor point whatever makes sense for his/her program (want a bottom-left anchor point? great! just use a negative height).

I think it's simple to say the position is the anchor (heck, I'd even be okay if it was called "anchor" instead, though "position" is fine too). A rectangle is defined by points in the range [position, position + size].

I don't think it's inconsistent to provide a position/anchor point and a size (and nothing else).

edit: I just read Hapax's comment above. I like most of what he said, though I don't like the names (getStart/getSize/getEnd). I'd personally prefer something like "anchor" and "size". While a member dedicated to getting (anchor + size) would be useful, I can't think of a name that I like for it, and I'm the kind of person that would rather not have something if it's named something that feels... weird.

[Hapax]

I don't like "anchor"; sorry 
It would be more consistent to call it "origin", maybe.
That said, "position" is still fine with me. I, personally, don't really have a problem with it. It's called that in the constructor; it's fine for the "getter".

Names aside, I think the point is really just "do we really need vector returning functions for these values or should we write our own functions to create them from its components (or just apply them manually)?"
I'm sort of on the fence with this one, though I think I'm leaning more towards the side of having them. It makes sense to be able to have a function to get the same types we used to construct the object with.

[wintertime]

A while ago I replaced a use of sf::Rect in my code with my own that contains 2 vectors. I think its more consistent that way, so there is no more need to manually access both x and y or to look if, for example, one y value was called top, up or height.

[Tank]

Made a pull request for getPosition() and getSize(): https://github.com/SFML/SFML/pull/1047

Part 1:

Why no other corner points: Because I wanted simple compound wrappers for the members that are already there, namely left/top, width/height. Other corners would contain logic and require calculations. This can be discussed, but it's not what I intended. It can even be discussed completely separated from this.

Why "position" and "size" as names: Because that's how SFML represents the rectangle as a concept. sf::Rect already takes "position" and "size" in one variant of a constructor, so this concept can be used further. And to be honest, it's totally well-known everywhere else: Where is the rect, and how large is it? Since we have a +x/-y coordinate system, expansion logic is also natural.

Part 2, for SFML 3:

Replace left/top by "sf::Vector2 position" and width/height by "sf::Vector2 size".

The main reason being, like in part 1, ease of usage ("rect.position + rect.size / 2" to get the center vs. "sf::Vector2f(rect.left, rect.top) + sf::Vector2f(rect.width, rect.height) / 2.0f"). The advantage over getPosition() and getSize() is clearly the elimination of creating copies all the time.

[achpile]

Wow, nice to see my suggestion going to be part of SFML  Thanks Tank!