Remove sf::VertexArray in lieu of std::vector<sf::Vertex>

[vittorioromeo]

See #2208 for context/discussion.

sf::VertexArray is a wrapper over std::vector<sf::Vertex> that exposes a subset of std::vector's API, making its usefulness quite limited.

We should just use std::vector<sf::Vertex> instead, and encourage our users to do the same.

[Bambo-Borris]

Pretty sure the conclusion at the end of said discussion was not to move forward with this idea. Personally, I don't think even retaining a VA like concept is worth it and all shapes should be VB based anyway. If I was going to go any route, that's the one I'd promote. It was trialled in the past but reverted because users relied on temporary drawables heavily, which is a pattern I think SFML shouldn't promote.

[danieljpetersen]

I think it's unfortunate that we have two constructs which perform a very similar role.

I think it's unfortunate that the API of sf::VertexArray is different than that of sf::VertexBuffer.

I think the API of VertexArray is much simpler and easier to use than VertexBuffer. The API of VertexBuffer makes sense given what it needs to do though. And it is nice that VertexBuffer is just a wrapper around std::vector.

Years ago when the VertexBuffer was first introduced, I tested it out in my projects. I found that VertexArray performed better than VertexBuffer for streaming use cases, so if you were clearing the verts each frame, I got better performance with VertexArray. No idea why or if that even makes sense, but that's what I was seeing. Not a scientific test, and this was years ago and maybe just a quirk of my driver. I felt it worth mentioning.

[vittorioromeo]

Pretty sure the conclusion at the end of said discussion was not to move forward with this idea.

The original issue was about making sf::VertexArray fully public, but we discussed removal as well. @eXpl0it3r said:

I don't see us converting sf::VertexArray into a plain struct. If we'd approach that direction, I'd rather just remove the type as @vittorioromeo said.

So there was at least some general sympathy towards sf::VertexArray removal in that issue.

It was trialled in the past but reverted because users relied on temporary drawables heavily, which is a pattern I think SFML shouldn't promote.

SFML 3.x is a good opportunity to make this breaking change and steer users towards less frequent use of temporary drawables.

[vittorioromeo]

I think it's unfortunate that we have two constructs which perform a very similar role.

I think it's unfortunate that the API of sf::VertexArray is different than that of sf::VertexBuffer.

I think the API of VertexArray is much simpler and easier to use than VertexBuffer. The API of VertexBuffer makes sense given what it needs to do though. And it is nice that VertexBuffer is just a wrapper around std::vector.

Years ago when the VertexBuffer was first introduced, I tested it out in my projects. I found that VertexArray performed better than VertexBuffer for streaming use cases, so if you were clearing the verts each frame, I got better performance with VertexArray. No idea why or if that even makes sense, but that's what I was seeing. Not a scientific test, and this was years ago and maybe just a quirk of my driver. I felt it worth mentioning.

It's an interesting anecdote -- I believe that sf::VertexBuffer is faster than a vertex array only if vertex data changes infrequently.

Regardless, I think that comparing sf::VertexArray to sf::VertexBuffer is a bit misguided/off-topic for this PR, the main comparison I'd like to have is between sf::VertexArray and std::vector<sf::Vertex>, as the former is just an inferior version of the latter IMHO.

[eXpl0it3r]

I'm not in favor of this. There's no point in remove something just because it has a limited usefulness.

You also misunderstood my point that you quote. Before I'd accept turning sf::VertexArray into a struct with public data members, while still deriving from sf::Drawable, I'd rather remove it entirely. That doesn't mean I'm will to remove it. It just gives weight to my fact that I'm not willing to turn it into a plain struct either.

If people need something more advanced that what the simple sf::VertexArray provides, they can implement it on their own. Just removing it for everyone else who is perfectly fine with its simplicity isn't helping anyone here.

[vittorioromeo]

I'm not in favor of this. There's no point in remove something just because it has a limited usefulness.

You also misunderstood my point that you quote. Before I'd accept turning sf::VertexArray into a struct with public data members, while still deriving from sf::Drawable, I'd rather remove it entirely. That doesn't mean I'm will to remove it. It just gives weight to my fact that I'm not willing to turn it into a plain struct either.

If people need something more advanced that what the simple sf::VertexArray provides, they can implement it on their own. Just removing it for everyone else who is perfectly fine with its simplicity isn't helping anyone here.

What is the point of implementing, documenting, testing, and maintaining something of limited usefulness?

What is the point of providing a more limited version of a std::vector?

What users are we targeting with sf::VertexArray? Users looking for simplicity are not going to bother with vertices. Users looking for performance are going to be more familiar with standard vectors rather than our own in-house barebones wrapper.

Please reconsider, otherwise let's put this PR to a vote between all active SFML maintainers.

[eXpl0it3r]

You're having a way too simplistic view of SFML users. I've been an SFML user and have helped hundreds of people over the last 12+ years. There isn't just two categories of users, as such you may want to add a new profile of a user to your understanding, one who uses SFML in a simple way, who wants performance, knows how to use classes, but doesn't really need complex algorithms to add some vertices.

It also doesn't require a lot of complex arguing to simply verify that there are hundreds of projects using sf::VertexArray thanks to GitHub's search.

It's a very simple class, if you can't handle the maintenance and testing efforts, I'm sure someone else from the community will help out, given that it's been minimal work over the past many years as the histories show (1, 2).

Removing an existing construct on the arguments that it's somehow not useful enough (and I guess because I don't want the plain struct) and that it requires maintenance, isn't good enough.

Additionally, it would require us to explain a concept of a vertex array, without having a concrete implementation in SFML and it would create an asymmetry to sf::VertexBuffer.

I don't see how this needs a vote. It costs so much time and energy for everyone to get involved in all of this for what? So we can remove existing functionality? Why can't my word not be enough for you to reconsider your position? Why does it always need to be the whole team and even then you don't really accept the other position and just accept defeat? We're not asking for the whole team when we agree on something either. I think we can learn to respect and truly understand each others perspectives without having to resort to voting battles, as this gets really much more into politics: "Oh you don't like it, lets look around for others to like it". Maybe this fits better in the forum thread you started.

[vittorioromeo]

You're having a way to simplistic view of SFML users. I've been an SFML user and have helped hundreds of people over the last 12+ years. There isn't just two categories of users, as such you may want to add a new profile of a user to your understanding, one who uses SFML in a simple way, who wants performance, knows how to use classes

I've been a professional C++ teacher and mentor for the past ~7 years, and I've taught C++ informally before that.

Standard vectors are among the first things taught to someone learning C++, and they're quite easy to grasp.

I really don't understand why we should have so much mistrust in our users' capabilities that we feel like they can't understand .push_back instead of .append, or .size instead of .getVertexCount, so much so that we're trying to justify wrapping a subset of the most popular C++ container so that some member function names can be changed to something which you subjectively believe is simple, despite the entire world having already agreed on using standard vectors. And we lose emplacement, range-based for loops, standard iterator interface, reserving, and so on. How can you justify that...?

I disagree with your user classification. I cannot believe that anyone who is able to generate vertices and store them in a container is not able to understand the concept of a "vertex array" or not able to use std::vector.

but doesn't really need complex algorithms to add some vertices.

What "complex algorithms" is sf::VertexArray abstracting away from users...? The only thing that remotely fits the bill is getBounds, which can and should be a simple free function that works on any subrange of vertices.

It's a very simple class, if you can't handle the maintenance and testing efforts, I'm sure someone else from the community will help out, given that it's been minimal work over the past many years as the histories show

It's not about effort. Even if it takes minimal effort to maintain something useless and redundant, it doesn't mean we should keep doing it.

Removing an existing construct on the arguments that it's somehow not useful enough (and I guess because I don't want the plain struct) and that it requires maintenance, isn't good enough.

More arguments:

• Leads users away from Standard C++ vectors for absolutely no good reason
• Prevents users from realizing that there's nothing special about sf:: VertexArray
• Encourages users to use nonstandard terminology such as "append", "getVertexCount",
• Discourages users from reserving memory in advance
• Prevents users from using range-based for loops
• Prevents users from using standard algorithms on ranges of vertices

It also doesn't require a lot of complex arguing to simply verify that there are hundreds of projects using sf::VertexArray thanks to GitHub's search.

Showing that there are many uses of something is also not an argument proving its quality. People constantly use bad/unnecessary abstractions. People have used many standard utilities and facilities that have been deprecated or removed from the C++ Standard, because it evolves and their significance or value lowers, just like SFML evolves and its old OOP-centric abstractions go through the same path.

All those uses can also be replaced with a standard vector with minimal effort.

I don't see how this needs a vote. It costs so much time and energy for everyone to get involved in all of this for what? So we can remove existing functionality? Why can't my word not be enough for you to reconsider your position? Why does it always need to be the whole team and even then you don't really accept the other position and just accept defeat? We're not asking for the whole team when we agree on something either. I think we can learn to respect and truly understand each others perspectives without having to resort to voting battles, as this gets really much more into politics: "Oh you don't like it, lets look around for others to like it". Maybe this fits better in the forum thread you started.

For me, it's not about "defeat" or "success", and frankly that's a very childish way of thinking about open source software engineering.

I propose things that I believe actively improve the quality of SFML, and I always provide technical arguments and reasoning for my proposals and choices.

If you are consistently shutting down any proposal that doesn't fit your direction, and I believe that direction is actively harming SFML's quality and success (which I do), of course I'm going to ask the opinion of the whole team, as I don't think SFML is following the "benevolent dictator" model.

If the whole team disagrees with my vision and direction for SFML, that's fine, and at that point I might as well stop contributing or make my own more opinionated fork.

I genuinely think that the OOP-centric direction/vision you hold is too extreme, OOP doesn't need to be the only paradigm a library follows, especially in situations like this one where the benefits are nonexistent and the drawbacks are real.

Maybe ask yourself why people flock to simpler libraries where drawing stuff is done via functions, and not objects. Maybe there's a balance between OOP and other paradigms that you're not willing to accept.

[eXpl0it3r]

I disagree with your user classification. I cannot believe that anyone who is able to generate vertices and store them in a container is not able to understand the concept of a "vertex array" or not able to use std::vector.

You‘re missing the point. It‘s not about understanding C++ constructs, but it‘s about the ergonomics and simplicity and frankly the standard library has a lot of terrible APIs, that I would not want to use a basis for SFML‘s API design.

Showing that there are many uses of something is also not an argument proving its quality. People constantly use bad/unnecessary abstractions. People have used many standard utilities and facilities that have been deprecated or removed from the C++ Standard, because it evolves and their significance or value lowers, just like SFML evolves and its old OOP-centric abstractions go through the same path.

This makes no sense. People use the provided feature because they found it useful, even in its current state.

If you are consistently shutting down any proposal that doesn't fit your direction, and I believe that direction is actively harming SFML's quality and success (which I do), of course I'm going to ask the opinion of the whole team, as I don't think SFML is following the "benevolent dictator" model.

You yourself with the rest of the team appointed me as BDFL.

If the whole team disagrees with my vision and direction for SFML, that's fine, and at that point I might as well stop contributing or make my own more opinionated fork.

I genuinely think that the OOP-centric direction/vision you hold is too extreme, OOP doesn't need to be the only paradigm a library follows, especially in situations like this one where the benefits are nonexistent and the drawbacks are real.

Before you joined as SFML maintainer the direction and priority of SFML 3 was already been quite clear. Thrasher and the rest of the community have done a ton to get SFML into the state it is now. We‘re not suddenly rewriting fundamental APIs and mudding the clear OOP-centric design at random parts, weeks before we actually want to freeze SFML 3‘s API.

It would help and I would appreciate if you could accept that we‘re not going to change the direction on wimp, especially not at this stage.

Maybe ask yourself why people flock to simpler libraries where drawing stuff is done via functions, and not objects. Maybe there's a balance between OOP and other paradigms that you're not willing to accept.

People are free to do so. SFML‘s design has proven itself and I‘m very happy with the community and the community‘s size. I‘m not here for the fame and don‘t want to deal with the toxicity that comes with being large and having to cater to a lot of entitled people.

[vittorioromeo]

You‘re missing the point. It‘s not about understanding C++ constructs, but it‘s about the ergonomics and simplicity and frankly the standard library has a lot of terrible APIs, that I would not want to use a basis for SFML‘s API design.

std::vector already appears in SFML's public API in a multitude of places.

sf::VertexArray does not improve on std::vector's API whatsoever, as it retains unsafe access with operator[], renames well-established convetions such as .size and .push_back, and prevents usage of the rest of std::vectors API, which is quite useful and evolved significantly since C++98.

sf::VertexArray harms ergonomics quite significantly, as the range-based for loop construct cannot be used with it, it cannot be used with standard algorithms due to the lack of iterator support, does not support std::initializer_list, etc...

Choosing to ignore all of these objective drawbacks is choosing to encourage our users towards a subpar in-house version of std::vector.

This makes no sense. People use the provided feature because they found it useful, even in its current state.

That is your assumption. and a bold one.

I'd argue that people use sf::VertexArray because the tutorials tell them to. That same tutorial could just tell people "to store and draw a bunch of vertices together, simply use a standard container such as std::vector or std::array", and they'd use that instead.

Frequency of use of X does not imply X being good.

You yourself with the rest of the team appointed me as BDFL.

I forgot about that, you are correct. As BDFL, feel free to close this PR if you are not willing to consider my points nor willing to put it to the vote. Also feel free to ask me to stop contributing if you feel like my direction/vision for the project is inherently compatible with the BDFL's one. I won't take it personally, it is clear that we disagree on what "simplicity" means and on what is the value that SFML brings to its audience.

[coveralls]

Pull Request Test Coverage Report for Build 9629419719

Details

• 63 of 70 (90.0%) changed or added relevant lines in 6 files are covered.
• No unchanged relevant lines lost coverage.
• Overall first build on feature/remove_vertexarray at 56.183%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/SFML/Graphics/Sprite.cpp	0	1	0.0%
src/SFML/Graphics/Shape.cpp	31	33	93.94%
src/SFML/Graphics/Text.cpp	16	20	80.0%

Totals
Change from base Build 9618958132:	56.2%
Covered Lines:	11624
Relevant Lines:	19648

---

💛 - Coveralls

[ChrisThrasher]

I'm focused on getting v3.0.0 shipped and a major step towards that goal is freezing the API. Early this year we had a meeting amongst the SFML team to discuss what remaining API changes we wanted to make. I'm glad to say with the exception of #2992 we're done with everything we planned! 🥳

I'm still open to further API breakages but at this point in the development process I want the remaining API changes to be relatively modest. This PR is a rather drastic change and I would say is too drastic of a change for me to support so late in v3's development cycle.

[vittorioromeo]

I'm focused on getting v3.0.0 shipped and a major step towards that goal is freezing the API. Early this year we had a meeting amongst the SFML team to discuss what remaining API changes we wanted to make. I'm glad to say with the exception of #2992 we're done with everything we planned! 🥳

I'm still open to further API breakages but at this point in the development process I want the remaining API changes to be relatively modest. This PR is a rather drastic change and I would say is too drastic of a change for me to support so late in v3's development cycle.

I guess we disagree on the severity of the change -- for me this is a minor change compared to the way we're now loading resources and using std::optional everywhere. Getting rid of this class now would have been a good idea as we're already asking our users to make quite a lot of changes to their projects when migrating to 3.x.

Adding "please replace sf::VertexArray with std::vector<sf::Vertex>" to our 2.x -> 3.x migration guide would have been ideal IMHO.

I'm going to close as there's no consensus here, if you do reconsider feel free to open as I still think that 3.x is a great opportunity to make these sort of API breaking changes that require simple, minimal, and purely mechanical migration effort from the users.

[ChrisThrasher]

for me this is a minor change compared to the way we're now loading resources and using std::optional everywhere

The std::optional changes are quite major, but they were also agreed upon many months ago. At this point we're in the process to freeze the API and it's hard to do that when non-trivial API changes/breakages are being proposed. At some point the train that is SFML 3.0.0 has to leave the station even if that means potentially good API changes get left behind. We can't be like C++0x (which famously did not ship in the 2000s as you're well aware) and wait many extra years to ship. We need to be more like C++14 and beyond and ship even if some stuff barely misses the deadline. If we keep postponing v3 while we keep tweaking the API then I'm afraid we'll be tweaking the API well into 2025.

[eXpl0it3r]

You‘re missing the point. It‘s not about understanding C++ constructs, but it‘s about the ergonomics and simplicity and frankly the standard library has a lot of terrible APIs, that I would not want to use a basis for SFML‘s API design.

std::vector already appears in SFML's public API in a multitude of places.

You're again misrepresenting what I'm saying. The API design of the STL is in many parts quite bad, as such I don't consider it the ultimate standard for C++ API design. I doesn't mean that I wouldn't use STL types in the SFML API, just that I wouldn't use it as a template for designing (simple) APIs.

Choosing to ignore all of these objective drawbacks is choosing to encourage our users towards a subpar in-house version of std::vector.

You always could and still can use std::vector<sf::Vertex> in your code.

That is your assumption. and a bold one.

I'd argue that people use sf::VertexArray because the tutorials tell them to. That same tutorial could just tell people "to store and draw a bunch of vertices together, simply use a standard container such as std::vector or std::array", and they'd use that instead.

I'm not making any assumption and find it frankly disrespectful. I didn't add my years of experiencing with SFML users as a brag, but to show that I've interacted with SFML users nearly every single day of this past decade and I have a pretty good understanding of the various SFML users, which are the people we're writing this library for!

Frequency of use of X does not imply X being good.

I'm not making that argument, I'm merely saying that users exist that like the abstraction enough to use it and/or didn't have the need for anything more.

I forgot about that, you are correct. As BDFL, feel free to close this PR if you are not willing to consider my points nor willing to put it to the vote.

Quite honesty, I want(ed) you to display some maturity to recognize, understand and accept the set direction and close your own PRs without me having to do it for you.

I consider your points, I just won't discuss them, because discussing them wastes a lot of time (look at the multiple hours I've already spent on theses posts here), when I know, I'm not going to change my mind on the matter and I'm fairly certain, you won't change your mind either. There's no scenario where I gain anything from discussing the points, because the ones I vaguely agree with, will be used against me (e.g. by making yet some other PR that I wouldn't want) and the ones I disagree with, are marked as not "valid" disagreements.
I'm happy to talk about API design and my "vision" of SFML, but not as part of a specific change.

Also feel free to ask me to stop contributing if you feel like my direction/vision for the project is inherently compatible with the BDFL's one. I won't take it personally, it is clear that we disagree on what "simplicity" means and on what is the value that SFML brings to its audience.

I value your input and your experience. I believe it's healthy to have certain things challenged. However, when we have to spend hours discussing self-inflicted problems while a pile of higher priority items are not getting worked on, we're really running into an issue, especially in an open source project. I have a limited set of hours to work on SFML and if I have to spend all of it, putting out fires that we've created ourselves, then other important topics (like the website, documentation, SFML.Net, etc.) won't get any work done. Even worse when these discussions spiral and pull in every other maintainer, so that development crawls to a halt and we're just spinning in circles.

Maybe it's worth for both of us to articulating our "visions"/directions/motivation, so we can see/understand much more clearly where the other is headed and agree or disagree on some topics, so we can stop bring those up moving forward.