SFML community forums

General => Feature requests => Topic started by: mashedtatoes on November 26, 2014, 09:51:39 am

Title: Make CircleShape, RectangleShape, and ConvexShape only be shapes. Not drawables.
Post by: mashedtatoes on November 26, 2014, 09:51:39 am
CircleShape, RectangleShape, ConvexShape, and Sprite do very similar things. They all apply a texture to some shape. What I am suggesting is to remove the functionality that allows the shapes to be drawable and add the ability to apply a shape to a sprite. Then you create the shape first and pass that shape into the constructor for the sprite. So rather than creating just a CircleShape, you would create a CircleShape then create a sprite using that CircleShape. When you apply the texture to the sprite, it will only give you a circular cutout of the texture.

This does add an extra line of code but, to me, nothing about CircleShape seems like it should be anything more than a shape (just a bunch of points in some space). You shouldn't be able to apply a texture to this shape and draw it because it is just a shape. It shouldn't be able to represent an image. That is where Sprite and Image should be used.

It just seems a lot more intuitive to me if the process of creating a shaped drawable was
Box2D creates shaped objects in a similar way so if I did not explain it very good and you would like some code to look at, you can look at Box2D's b2Fixture.
Title: AW: Make CircleShape, RectangleShape, and ConvexShape only be shapes. Not drawables.
Post by: eXpl0it3r on November 26, 2014, 10:16:12 am
Shapes are not intended to be used as clipping masks. This feature has been requested for a long time (#1 (https://github.com/SFML/SFML/issues/1)) though.

It essentially is more questionable why shapes can use a texture to begin with. In the end there's very little difference between a sprite and a rectangle shape, both can render a texture quad. Since both are about the same, your suggested hierarchy doesn't really make sense.

Keep in mind however that you can do all of this by writing your own class hierarchy based on vertex arrays. ;)
Title: Re: Make CircleShape, RectangleShape, and ConvexShape only be shapes. Not drawables.
Post by: Nexus on November 26, 2014, 04:56:26 pm
What I am suggesting is to remove the functionality that allows the shapes to be drawable and add the ability to apply a shape to a sprite.
These are indeed some interesting ideas.

It essentially is more questionable why shapes can use a texture to begin with. In the end there's very little difference between a sprite and a rectangle shape, both can render a texture quad. Since both are about the same, your suggested hierarchy doesn't really make sense.
It would not make sense with the current sf::Shape hierarchy, but it would with a hierarchy of purely logical shapes, as suggested by mashedtatoes. Exactly the fact that sf::Sprite and a textured sf::RectangleShape are so similar indicates that some parts of the API can potentially be improved.

Shapes are not intended to be used as clipping masks. This feature has been requested for a long time (#1 (https://github.com/SFML/SFML/issues/1)) though.
Good you mention clipping masks. I see different approaches to implement them in OpenGL, each with different flexibility: scissors (rectangles), clipping planes (convex polygons with a point limit) and stencil masking (arbitrary pixel-based masks). The fragment shader is a further user-side possibility to not render certain pixels. All approaches are however different from shapes, which are split into triangles a priori and don't require additional resources (stencil buffer) or checks (stencil check, clipping check) to function. Thus I don't think one mechanism can replace the other.

By the way, write masks (https://www.opengl.org/wiki/Write_Mask) are a different concept, they limit rendering based on color components, not areas.
Title: Re: Make CircleShape, RectangleShape, and ConvexShape only be shapes. Not drawables.
Post by: Hiura on November 26, 2014, 08:56:16 pm
That's indeed some interesting food for thought. I've two things to add:

 a) I think it was already mentioned somewhere around here, so if I remember correctly it was Laurent who explained that we haven't merged Sprite and RectangleShape because the former was for a simple usage/beginners.
 b) We should probably push the discussion further to examine a broader generalisation of the concept for SFML 3. I have in mind something similar to my Curve (https://github.com/mantognini/sftools/blob/develop/include/sftools/Curve.hpp) tool where you can draw an arbitrary curve if you have its mathematical representation. Would it make sense to have something similar in SFML or is it too high level? Can we make it even better?
Title: Re: Make CircleShape, RectangleShape, and ConvexShape only be shapes. Not drawables.
Post by: mashedtatoes on November 27, 2014, 01:03:14 am
Curves would be a neat feature. That actually gives me another idea though. Maybe you can split Shape up even more into a group of Line(s). A line would be a set of continuous points with no curve greater than 180o. A shape is a set of lines chained together (each line shares an endpoint with only two other line). Then you could have curved lines and straight lines on a single shape and make some real funky shapes. So a circle shape would be made with 2 lines. Both of them are 180o curves and they are linked together by their endpoints. A rectangle would be just four lines linked together...
Title: Re: Make CircleShape, RectangleShape, and ConvexShape only be shapes. Not drawables.
Post by: Gambit on November 27, 2014, 03:02:35 am
If drawing was removed, how would one go about drawing a circle or a square? I guess you would have to query a list of lines or points then draw them yourself with a shader or something?
Title: Re: Make CircleShape, RectangleShape, and ConvexShape only be shapes. Not drawables.
Post by: mashedtatoes on November 27, 2014, 05:51:06 am
Quote
how would one go about drawing a circle or a square
If you are talking about drawing a shape without a texture that is a good question. I guess the Sprite would initialize with some drawable shape depending on the shape you pass into it. Then you would be able to fill it with a color or change the size or line thickness like you can with shapes now.
Title: AW: Make CircleShape, RectangleShape, and ConvexShape only be shapes. Not drawables.
Post by: eXpl0it3r on November 27, 2014, 08:39:07 am
As seen by Hirua's code, things like that can be built on top of SFML, thus I don't think it needs to be part of SFML.

Things like a clipping area which is currently rather hard if at all to implement on top of SFML (without using raw OpenGL), seem to be a limitation.

Requiring an additional shape object for every sprite would make things very quickly, very annoying. In 2D about everything is done with quads, even when the objects are round one mostly just uses a round texture. Making this basic usecase more complicated than necessary isn't really a good idea.

Also if you go down to "lines" you might as well just go down to vertices, which is exactly what shapes (and sprites) are, a collection of vertices that build a convex polygon.
Making edges of shapes curved is an interesting idea, but keep in kind that if you want an area you'll have to write some sort of a solver to get the triangles layed out properly etc. This is something rather specific (not that many applications would need that), it can already be written on top of SFML and thus it's too complex and too high-level IMHO.
Title: Re: Make CircleShape, RectangleShape, and ConvexShape only be shapes. Not drawables.
Post by: Hapax on November 28, 2014, 01:39:33 am
Quote
how would one go about drawing a circle or a square
If you are talking about drawing a shape without a texture that is a good question. I guess the Sprite would initialize with some drawable shape depending on the shape you pass into it. Then you would be able to fill it with a color or change the size or line thickness like you can with shapes now.
Doesn't a sprite without a texture already default to white? Then setting its (filter) colour would change its colour.

However, removing the shapes as drawable objects also removes extras that sprites don't (currently) have. I can think of outlines/inlines. Would this suggestion then be to add outlines to the sprite class?
Title: Re: Make CircleShape, RectangleShape, and ConvexShape only be shapes. Not drawables.
Post by: mashedtatoes on November 28, 2014, 02:07:39 am
eXpl0it3r actually did a good job at interpreting what I was really requesting and it was shaped clipping masks. I just didn't know what it was until he mentioned it.
Quote
However, removing the shapes as drawable objects also removes extras that sprites don't (currently) have. I can think of outlines/inlines. Would this suggestion then be to add outlines to the sprite class?
Yes, this would mean that any drawable functionality currently in shape would get put in sprite.

Quote
Also if you go down to "lines" you might as well just go down to vertices, which is exactly what shapes (and sprites) are, a collection of vertices that build a convex polygon.
I see what you are saying, but I still feel like the difference between a shape and a sprite should be more distinct. If you are going to use a shape at all, it should be as a clipping mask. Otherwise, like you said
Quote
even when the objects are round one mostly just uses a round texture.
There isn't any real reason to have a shaped drawable (that I can see) when you can just make the texture whatever shape you want.
Title: Re: Make CircleShape, RectangleShape, and ConvexShape only be shapes. Not drawables.
Post by: Gambit on November 28, 2014, 02:59:30 am
That creates a lot of unnecessary assets for applications, especially for simple games with simple geometry and coloring.
Title: Re: Make CircleShape, RectangleShape, and ConvexShape only be shapes. Not drawables.
Post by: mashedtatoes on November 28, 2014, 09:29:12 am
Quote
That creates a lot of unnecessary assets for applications
Good point. I over looked that. How about this. First off, remove the ability to apply a texture to a shape. Next, create a ClippingMask class that Sprite will inherit. That way, you can still use shapes to generate simple graphics in a game without many assets. Then, Sprite would be used for pretty much everything else. When you create the sprite, you set the size and/or shape of the clipping mask. I guess this would replace the current setTextureRect in sprites with some setShape thing or setClippingMask.

Or maybe you leave the setTextureRect and add a setShape that selects some shape in the current texture rect.
Title: Re: Make CircleShape, RectangleShape, and ConvexShape only be shapes. Not drawables.
Post by: Hiura on November 28, 2014, 10:12:35 am
So there're two things here:

 a) disassociating shape from drawable
 b) having clipping mask.

Regarding the latter, have a look at the task tracker here (https://github.com/SFML/SFML/issues/1). Unless I'm mistaken it is not specific to sprite. It should work with any drawable (even raw vertices). Thus no inheritance should be involved here.

Now, about the former idea: Yes it can build on top of SFML but still we could provide the basic system. And if the API is well designed we don't need an extra shape for each sprite. By default it could still be a rectangle. But we could have factories to create other shape. And of course a more general constructor for generic shaped sprites. (I'm on my phone so it's very cumbersome to write code but if my thoughts are not so clear then tell me and I'll show something later this weekend.)
Title: Re: Make CircleShape, RectangleShape, and ConvexShape only be shapes. Not drawables.
Post by: Gambit on November 28, 2014, 11:04:22 am
I still dont like the idea of removing drawable from shapes. While it might make sense in some cases, it doesnt make sense in others. For example: It makes sense to remove drawable from shapes IF you are going to use something else to draw the shapes instead (Example being that you only need the verticies of the shape or something). It does not make sense to remove drawable from a shape for applications which draw shapes.

If I have a program which draws 4 squares on the screen, then I would just make 4 sf::RectangleShapes then draw them. Having to make a texture just to draw a square is pointless and as I said early, creates unnecessary assets for these types of applications.

Alternatively, one could use a geometry shader to work around this but not everyone has the knowledge to do that.
Title: Re: Make CircleShape, RectangleShape, and ConvexShape only be shapes. Not drawables.
Post by: Nexus on November 28, 2014, 11:35:26 am
So there're two things here:

 a) disassociating shape from drawable
 b) having clipping mask.
Yes, exactly. As mentioned in my last post, the concepts of shapes and clipping masks are different. In particular, the latter are either very limited (finite number of clipping planes) or expensive (stencil buffer and check). I still don't think it's a good idea to implement convex shapes via clipping masks.

And as always in designing an API, let's not focus on technical details ("A is assigned a B", "I don't like if X no longer inherits Y")... It makes no sense to discuss at them as long as we're not clear what functionality is actually desired.

As far as I see it, there are different requests:
Title: Re: Make CircleShape, RectangleShape, and ConvexShape only be shapes. Not drawables.
Post by: wintertime on November 28, 2014, 12:19:26 pm
I dont like that idea, because it is really backwards. Currently Sprite is there to be the simplest possible thing to be drawn, where it just puts a texture on screen as it is (often with a 1:1 mapping of texels to pixels), with the textures alpha determining the perceived shape.
The Shape classes are for complicated uses, can have any number of vertices determining the draw area and have outlines with a different color. Bloating the Sprite class with all kinds of rarely used extraneous functionality will result in loosing the simple class that is probably used more often and complicating that usecase.

I would think if a change is done its more useful to add the possibility of putting all Drawables into a VertexArray or Batch class and only then actually drawing everything at once. Maybe direct drawing could then even be removed to simplify porting to modern OpenGL using VBOs.
Title: Re: Make CircleShape, RectangleShape, and ConvexShape only be shapes. Not drawables.
Post by: Hiura on November 28, 2014, 06:26:47 pm
And as always in designing an API, let's not focus on technical details ("A is assigned a B", "I don't like if X no longer inherits Y")... It makes no sense to discuss at them as long as we're not clear what functionality is actually desired.
Yes and no. Example: some people here are confused (IMO) and think that you would not be able to create a sprite without having to create an additional resource (shape). And that's not necessarily true if your API is well defined. So you need to explain at least *a bit* the API to show how it could work. But otherwise you're right: defining a precise API now would be premature.

Anyway, about features. What could be interesting to investigate is a mix of texture and shape where a sprite would be a basic construct (class, factory, ...) based on this general idea to simply display a rectangle texture, while the more general construct could handle simple shapes, or custom ones provided by the user, without mandatory texture. On top of that we can manage border thickness, border color, fill color and all the properties we currently have.

How shape are defined is another interesting question. Would it be whole shapes or sequences of connected segments? Something different?
Title: Re: Make CircleShape, RectangleShape, and ConvexShape only be shapes. Not drawables.
Post by: Critkeeper on December 10, 2014, 01:00:19 pm
The biggest argument against this that I can see is having to create a shape in order to create a sprite, and how managing an additional asset for every sprite that would annoy some people. You could make the default behaviour exactly the same as it is now, that is that sprites use rectangular shapes, by using a C++ feature called implicit conversion.

Just make a single parameter constructor for rectangle shape, for example a vec2d giving its dimensions.

Then if you want to build a sprite shape but you specify a vec2d instead of an actual shape, c++ will implicitly convert it to a rectangle shape before passing it through the constructor. Notice we don't actually have to overload the constructor or anything like that.

We didn't tell the constructor that a vec2d is a valid parameter, but c++ doesn't care. You don't have to write any additional code in the library, and if you later change your mind about the type of the single argument that is passed to single argument constructors for various shapes, then you don't have to rewrite any code.

http://stackoverflow.com/questions/121162/what-does-the-explicit-keyword-in-c-mean (read the first answer)

If you try to make single argument constructors to other shapes, like a circle for example, specifying its radius, then this technique will work just the same.

What happens when you try to specify a single argument constructor whose argument shares a type that you've already covered, like an ellipse whose constructor takes a single argument sf::vec2d, I don't really know but I would expect a compiler error because of ambiguity.
Title: Re: Make CircleShape, RectangleShape, and ConvexShape only be shapes. Not drawables.
Post by: Hiura on December 10, 2014, 01:14:55 pm
The biggest argument against this that I can see is having to create a shape in order to create a sprite, and how managing an additional asset for every sprite that would annoy some people. You could make the default behaviour exactly the same as it is now, that is that sprites use rectangular shapes, by using a C++ feature called implicit conversion.
It all depends if the sprite owns the shape or not. If it does own the shape then you don't need an extra asset. (That's why I've been trying to say for a while but apparently failed to do so.)

Let's assume a sprite has its own shape. Then, by default, a sprite creates a rectangle shape. If told otherwise (e.g. with an explicit shape as constructor parameter or through factory methods) then it will copy this shape for its own usage. In both cases we don't have to keep track of this shape ourselves.

On the other hand, if it does *not* own the shape then we need to keep it alive long enough, like the texture. But I think it doesn't make sense to go that way: a shape is not that heavy (unlike the texture) and don't live on the GPU (also unlike the texture) so we can copy it a few times without trouble.
Title: Re: Make CircleShape, RectangleShape, and ConvexShape only be shapes. Not drawables.
Post by: Critkeeper on December 10, 2014, 01:26:11 pm
The biggest argument against this that I can see is having to create a shape in order to create a sprite, and how managing an additional asset for every sprite that would annoy some people. You could make the default behaviour exactly the same as it is now, that is that sprites use rectangular shapes, by using a C++ feature called implicit conversion.
It all depends if the sprite owns the shape or not. If it does own the shape then you don't need an extra asset. (That's why I've been trying to say for a while but apparently failed to do so.)

Let's assume a sprite has its own shape. Then, by default, a sprite creates a rectangle shape. If told otherwise (e.g. with an explicit shape as constructor parameter or through factory methods) then it will copy this shape for its own usage. In both cases we don't have to keep track of this shape ourselves.

On the other hand, if it does *not* own the shape then we need to keep it alive long enough, like the texture. But I think it doesn't make sense to go that way: a shape is not that heavy (unlike the texture) and don't live on the GPU (also unlike the texture) so we can copy it a few times without trouble.

Oh yes I was agreeing with you.

I'm just saying that you have people who want to be able to make a sprite without bothering with shapes, and you have people that do want to bother with it.

Most people would implement your idea by setting the shape to be the last argument given in the constructor for the sprite class, and then giving it a default value equal to an arbitrary rectangle like a unit square. (ugly because we are stuck with a default size for the rectangle)

Or they would provide a a "shape selector" parameter (ugly because we are stuck with a "default" version, like a unit circle or unit rectangle)

Or they would make it so that sprite is unusable even after it is constructed until a factor method is used to set the shape (very bad implementation imho).

All of those implementations have things wrong with them that people could argue over.


What I'm saying is you can get around that with a nifty feature of C++ that allows you to just specify the argument in the constructor of the sprite to be of the same type of the argument that constructs the shape-- but only if the shape can be built from one single argument. And whats nifty about it is you don't have to write any code. C++ just says "Oh gee I don't know how to build a sprite out of a vec2d or a float, but I know how to build it out of a shape, and I know how to build a circle out of a float and I know how to build a rectangle out of a vec2d, and circles and rectangles are both shapes, so I'll just use the right one."

Furthermore the sprite can have multiple arguments. It will substitute the right argument in the right place. You could write in the definition of sprite that it should take an argument shape, an argument color, and an argument position, and you automatically get atleast 2 constructors out of that "magically". Namely, the one that happens if I provide a float instead of a shape, and it gets turned into a circle, and the one that happens when I provide a vec2d instead of a shape, and it gets turned into a rectangle.

In other words there is no need to have a single "default shape". With this we get a default rectangle and a default circle (and a default w/e, one for each shape that can be built from a single unique parameter) and not only that, we don't have to settle for a "unit" rectangle or "unit" circle or whatever. We can specify the dimensions at the constructor and it will know what we meant, because there is no ambiguity. And it costs us nothing because we don't have to write more constructor code.
Title: Re: Make CircleShape, RectangleShape, and ConvexShape only be shapes. Not drawables.
Post by: Hiura on December 10, 2014, 06:24:44 pm
You're approach is too specific IMO. As Nexus said we should try to think about functionality.

If I understood you correctly you want to use some kind of Prototype Patter Design. For me, it's way to complicated to generate a shape. You don't need to do all that to get a bon unit rectangle: Notice how I said "by default, a sprite creates a rectangle shape" and not "the default value is a rectangle". We can achieve that with two constructors: One with a shape parameter and one without (and thus building the default rectangle according to some other data like the texture). We can of course have more than two ctors.

Since you're interested in the topic, what's a shape for you?
Title: Re: Make CircleShape, RectangleShape, and ConvexShape only be shapes. Not drawables.
Post by: Critkeeper on December 11, 2014, 01:30:48 am
You're approach is too specific IMO. As Nexus said we should try to think about functionality.

If I understood you correctly you want to use some kind of Prototype Patter Design. For me, it's way to complicated to generate a shape. You don't need to do all that to get a bon unit rectangle: Notice how I said "by default, a sprite creates a rectangle shape" and not "the default value is a rectangle". We can achieve that with two constructors: One with a shape parameter and one without (and thus building the default rectangle according to some other data like the texture). We can of course have more than two ctors.

Since you're interested in the topic, what's a shape for you?

You aren't quite understanding me. I'm saying we don't have to pass a shape to the constructor of the sprite to get a default rectangle, we can just pass a vec2d instead. And I'm saying we don't have to write a brand new constructor that has a vec2d as a parameter because C++ will automatically turn a vec2d into a rectangle, even without us writing any code to tell it how to do that, and it will use that.


A shape is a set of vertices connected by lines in a specific order such that the first and last vertices are connected. If the first and last vertices aren't connected then all we have is a set of connected lines that approximates a curve. If there isn't a specific order then the only way we can uniquely assign lines is if the number of vertices is equal to 2, which is a degenerate case and the definition of a line.

Therefore as far as I'm concerned, a shape has 3 or more vertices, the same number of lines, and the vertices are ordered.

It is fine if perimeter of the shape intersects with itself. The numeral 8 would be a shape for example.

In short, a shape is a special kind of curve, such that the first and last vertex are connected.
Title: Re: Make CircleShape, RectangleShape, and ConvexShape only be shapes. Not drawables.
Post by: Nexus on December 11, 2014, 07:45:02 am
I wouldn't call this questionable use of implicit conversion a feature. A constructor taking sf::Vector2f for rectangles is not any less tedious than one taking the rectangle shape. Plus, special treatment for rectangles is confusing. If you want to keep things simple, do it like now -- infer the size from the texture.

And as mentioned earlier by me and again by Hiura, before thinking about concrete constructor overloads, you should be clear about the general relationship between shapes and sprites. Does a sprite have a shape? Does it refer to one? How does the shape interact with the texture? Etc...
Title: Re: Make CircleShape, RectangleShape, and ConvexShape only be shapes. Not drawables.
Post by: Hiura on December 11, 2014, 07:55:34 am
Humm I went indeed a bit too fast on your vec2d thing. Sorry about that - got a lot on my mind right now.

But you still gives something to the ctor to build the shape, right? So you also have the problem of size not automatically matching the texture (if any) or did I missed something else? In either case we might want that the size is explicitly set (or not -- this is just a morning thought ^^).

Anyway, that's an interesting idea but I'm not sure implicit conversations are appropriate here. I would have to think a bit more about it. It would be probably easier to get the whole picture later since this is more or less an implementation detail.

I do like your definition of shape though. I feel it covers pretty much everything we need.

However we have to investigate more how we handle line thickness: With a circle,it's very easy (take two consecutive points, find the middle point, add a normal vector of the wanted length and you got one point of the "other" circle that make it fat) but for others like rectangle it's not that easy.

Have you (guys) think about it yet?
Title: Re: Make CircleShape, RectangleShape, and ConvexShape only be shapes. Not drawables.
Post by: Critkeeper on December 11, 2014, 08:31:27 am
And as mentioned earlier by me and again by Hiura, before thinking about concrete constructor overloads, you should be clear about the general relationship between shapes and sprites. Does a sprite have a shape? Does it refer to one? How does the shape interact with the texture? Etc...

This is an example of what vertex blended fill colors looks like:
(http://www.jayway.com/wp-content/uploads/2010/01/SmoothColoring.png)

In case you are wondering about performance, vertex blending is a fundamental graphics operation implemented in hardware for almost every GPU in the past 10 years, so there isn't going to be a performance penalty.
Title: Re: Make CircleShape, RectangleShape, and ConvexShape only be shapes. Not drawables.
Post by: Ixrec on December 11, 2014, 08:43:28 am
Quote
A sprite may or may not have a texture.

Wouldn't a sprite with no texture be identical to whatever the default shape for sprites is? (eg, a white square)
Title: Re: Make CircleShape, RectangleShape, and ConvexShape only be shapes. Not drawables.
Post by: Critkeeper on December 11, 2014, 08:45:39 am
Quote
A sprite may or may not have a texture.

Wouldn't a sprite with no texture be identical to whatever the default shape for sprites is? (eg, a white square)

Yeah sorry I rethought that at the last moment. It doesn't make sense for a sprite to not have a texture, because then it would literally be identical to a shape.

That means that a sprite MUST have a texture.

I also put in an additional clause that makes it so that if no shape is given to the sprite, a rectangle will be inferred from the texture.
Title: Re: Make CircleShape, RectangleShape, and ConvexShape only be shapes. Not drawables.
Post by: Hiura on December 11, 2014, 11:28:07 am
Well, this all depends on whether shapes are drawable or not.. if they are not, and I think that the point of this discussion, then a sprite can have no texture.
Title: Re: Make CircleShape, RectangleShape, and ConvexShape only be shapes. Not drawables.
Post by: mashedtatoes on December 12, 2014, 09:24:50 am
Oooh. Lots of discussion since I last checked this thread.
Quote
Well, this all depends on whether shapes are drawable or not.. if they are not, and I think that the point of this discussion, then a sprite can have no texture.
Yup, that is definitely a main point. The less clear, but I think the more important, request is that a sprites shape should function as a clipping mask.