New graphics API ready

[Laurent]

Hi

I've pushed a first version of the new graphics API, so that you can test it. Please focus on the API, there's no optimization yet so the performances are worse than before. I can't upload the documentation, so you'll have to either generate it yourself, or directly look into header files.

The new API is available in the 'drawables' branch of the main repository.

It is for testing only, if you want a final and optimized version then wait until it is merged into the master branch.

Thanks

[Nexus]

Great news, I'll test the new API as soon as possible!

[Nexus]

I've experimented a little with the new shapes, and some points took my attention:

• Sometimes the shape isn't updated correctly. After the following code, the shape isn't ready to be drawn. But when uncommenting the last line, it works.

Code: [Select]

sf::ConvexShape shape;
shape.SetPointsCount(3);
shape.SetFillColor(sf::Color::Magenta);
shape.SetPoint(0, sf::Vector2f(10, 10));
shape.SetPoint(1, sf::Vector2f(50, 10));
shape.SetPoint(2, sf::Vector2f(10, 50));
//shape.SetOutlineThickness(0);

• Private members don't appear in the documentation. You should enable them because of virtual functions like GetOutlinePoint().
  
  
• In the ConvexShape::ConvexShape() constructor, you write "Let's define a triangle by default... just so that it's not empty"... I think an empty shape (e.g. nothing) is more intuitive than an arbitrary triangle -- and the behavior remains the same as in the old API. It allows to write shape = sf::ConvexShape(); to clear a shape.
  
  
• Or is it an invariant that the amount of points in a sf::ConvexShape must be at least 3? If so, you should probably check it in SetPointsCount(). Still, the default triangle is of no real use and looks like a workaround ;)
  And why do you write "The default number of points of a polygon is 6"?

[minirop]

virtual isn't totally useless for a private member ?
ps : i wasn't in "drawable branch"

[Silvah]

virtual isn't totally useless for a private member ?

No, it's not. Private virtual methods still can be overridden (really, overridden, not just shadowed) in derived classes.

[Laurent]

Sometimes the shape isn't updated correctly

It's fixed. I forgot to call Update() in SetPoint and SetPointsCount.

Private members don't appear in the documentation. You should enable them because of virtual functions like GetOutlinePoint()

Or I could make them protected instead.

In the ConvexShape::ConvexShape() constructor, you write "Let's define a triangle by default... just so that it's not empty"... I think an empty shape (e.g. nothing) is more intuitive than an arbitrary triangle -- and the behavior remains the same as in the old API. It allows to write shape = sf::ConvexShape(); to clear a shape.

I made this to make the class consistent with other shapes (they all have a default definition which is a valid shape).
If I choose your solution, I should also use 0 for the radius of sf::CircleShape, for the size of sf::RectangleShape, etc.
I don't know which solution is the best.

Or is it an invariant that the amount of points in a sf::ConvexShape must be at least 3?

It is indeed forbidden to have less than 3 points, it is handled in the base class.

Still, the default triangle is of no real use and looks like a workaround

I totally agree :?

And why do you write "The default number of points of a polygon is 6"?

Just a typo

[Nexus]

Or I could make them protected instead.

Or directly public. At least, I have a use case in mind: My thor::ConcaveShape class should be able to create a copy of any sf::Shape, and this is not possible without accessing the single points. On the other side, I see that the single points can be an implementation detail, especially for circles. Hm... :?

Apart from that, I think it is a little bit confusing that there exists GetPoint() and GetOutlinePoint(), although they do the same (analogous for the count getter). Remember that you can also loosen access in derived classes, i.e. redeclare the method in the public section.

I don't know which solution is the best.

Me neither, I have to think about it more. I just think it has been comfortable to allow "empty" shapes, but with a better design they are probably not necessary. And the loss of class constraints is a high price.

Just a typo

I've seen another one: "offseted" instead of "offset"

[Laurent]

Or directly public. At least, I have a use case in mind: My thor::ConcaveShape class should be able to create a copy of any sf::Shape, and this is not possible without accessing the single points. On the other side, I see that the single points can be an implementation detail, especially for circles. Hm...

Apart from that, I think it is a little bit confusing that there exists GetPoint() and GetOutlinePoint(), although they do the same (analogous for the count getter). Remember that you can also loosen access in derived classes, i.e. redeclare the method in the public section.

At first it was exactly like this:
- GetPointsCount and GetPoint
- some classes just made them public

But I figured out that some classes may define public points that don't match the internal ones. For example, the number of points defined by sf::StarShape is the number of "branches". It must be multiplied by two to get the actual number of points composing the shape.

So I had to write this less elegant solution. However, if someone can find a synonym to "point" for the star shape (I spent hours on it, couldn't find anything ), it can probably solve the problem as well, and I could revert to the first, more elegant solution. Then, making GetPoint and GetPointsCount public in the base class would be a good option

I've seen another one: "offseted" instead of "offset"

I never know how to write this one in past tenses. Is this an irregular verb?

[luiscubal]

Just curious - is there any reason not to have "AddPoint", "Clear", etc. in ConvexShape?

About private virtuals: although this is a valid idiom, I have to wonder: how well will this map to other languages(e.g. .NET). Will there be an equivalent for those languages. Should there *be* an equivalent?

RectangleShape only has "size". No position. Is this intentional? Does this mean one would have to use RenderState(or whatever it is called) to set the position?
Also, RectangleShape should at least have two extra constructors:
1. RectangleShape(float sizex, float sizey)
2. RectangleShape(Vector2f size)

[Nexus]

However, if someone can find a synonym to "point" for the star shape (I spent hours on it, couldn't find anything ), it can probably solve the problem as well, and I could revert to the first, more elegant solution. Then, making GetPoint and GetPointsCount public in the base class would be a good option

I have already wondered the same thing, and I'll see if I can find another term

I never know how to write this one in past tenses. Is this an irregular verb?

Yes, verbs ending with "set" don't change in past forms. For example set, reset, offset, upset.

RectangleShape only has "size". No position.

It has a position, inherited from sf::Transformable.

Also, RectangleShape should at least have two extra constructors:
1. RectangleShape(float sizex, float sizey)
2. RectangleShape(Vector2f size)

I believe it is Laurent's design choice to remove these constructors for the sake of expressiveness. Other shape classes don't have them, and sf::Sprite or sf::Text don't have them either. Temporary constructions are no more possible, but expressions with unclear parameters like

Code: [Select]

sf::Shape::Rectangle(2.f, 3.f, 3.f, 5.f, sf::Color(255, 120, 100), 3.f, sf::Color(250, 250, 0)); are avoided.

[Laurent]

Just curious - is there any reason not to have "AddPoint", "Clear", etc. in ConvexShape?

I think people always know how many points they need when they create a convex shape, so SetPointsCount is enough.
Clear() could be added, yes.

About private virtuals: although this is a valid idiom, I have to wonder: how well will this map to other languages(e.g. .NET). Will there be an equivalent for those languages. Should there *be* an equivalent?

Other languages adapt what they can't do to their capabilities. Even what they can do directly, is often modified to fit the language better. Bindings don't have to be a perfect mapping of the C++ API
For example, the C binding will probably not provide the ability to create custom classes of shapes. And the .Net binding will have to recreate this hierarchy/virtual functions thing, since it's based on the C binding.

RectangleShape only has "size". No position. Is this intentional? Does this mean one would have to use RenderState(or whatever it is called) to set the position?

sf::Shape inherits from sf::Transformable

Also, RectangleShape should at least have two extra constructors:
1. RectangleShape(float sizex, float sizey)
2. RectangleShape(Vector2f size)

The constructor that takes the size directly is definitely needed, and it's true for other shapes too.
I'm still not sure about the overload taking two floats. It's convenient but it would make me duplicate so many functions...

[Laurent]

sf::Sprite or sf::Text don't have them either

They do!

Code: [Select]

sf::Sprite sprite(texture, rect);
sf::Text text(string, font, size);

[l0ud]

I really miss the sf::Sprite::GetSize() and Resize() functions Did you remove them just for tidiness?

[Nexus]

They do!

Code: [Select]

sf::Sprite sprite(texture, rect);
sf::Text text(string, font, size);

Indeed. The previous sprite constructor only took 1 parameter (while the one from SFML 1.6 took 5 parameters). Anyway, the current parameter lists of sf::Sprite and sf::Text are a good compromise in my opinion.

I really miss the sf::Sprite::GetSize() and Resize() functions Did you remove them just for tidiness?

There are GetLocalBounds() and GetGlobalBounds() to return the bounding rect.

About Resize(), I haven't ever used it... But you can achieve the same when directly modifying the scale.

[Laurent]

I really miss the sf::Sprite::GetSize() and Resize() functions Did you remove them just for tidiness?

Yes:
- you can get the size with GetGlobalBounds (or GetLocalBounds + GetScale)
- you can set the size with GetLocalBounds + SetScale