Sfml: Suggestion : remove unnecessary use of runtime polymorphism

[groundswellaudio]

I've found a lot of places in the API where virtual inheritance is used without an apparent reason.
I propose to remove them in order to :

• improve performance
• simplify the design
• encourage better coding practices for beginners
  All uses that i have seen can be rewritten without virtual inheritance without breaking existing code (at least without breaking the code that doesn't store objects with base classes pointers, but all of these base classes weren't really designed for that anyway)

Here is a non-exhaustive list, followed by proposed solutions :

1/ RenderTarget : use inheritance as a way to reuse the common code in the RenderTarget base class.
2/ Drawable : use a virtual function draw() for no particular reason. Used by RenderTarget interface
3/ Shape : use a virtual function for accessing the points
4/ Socket : has a virtual destructor but no virtual functions
5/ SoundSource : has play/pause/stop virtual functions
6/ Window : has onCreate() and onResize() virtual functions

Propositions :
1/ remove the base class, stick the reusable code in a CRTP or mixin class, make all the functions taking a RenderTarget a template, and forward the needed data of Target type to the implementation in the .cpp file (e.g instead of querying getSize() through the vtable, take a template type and pass the result of getSize() to the implementation)
2/ remove the base class, and transform the draw(Drawable) methods for "RenderTargetable" types into a template, since there is zero logic inside of it already
3/ same as 1/, remove the base class, make template functions, assume that any ShapeType has a points() methods returning a range of points
4/ remove virtual destructor and the enum, have SocketSelector store a std::variant
5/ it's not clear to me why does this class needs virtual functions? code reuse? if so, replace by a mixin or CRTP class
6/ i don't understand why these functions exists : if i want some code to run after i called create() in my program, i can just shadow the create() function in my class, and resize has a dedicated event already. remove virtual inheritance, stick common code somewhere else

There might be a few more. If these changes are approved, i can work on it.

[LaurentGomila]

Just to be clear: this is about public inheritance, not virtual inheritance; these are two different (but non-exclusive) concepts in C++.

First, thanks for bringing such technical discussions, I really like it. But... I strongly disagree with your proposed changes 😉

Templates

Converting half of SFML and user code to templated code, just to avoid indirections (v-table calls), is just insane.

• Templates increase compilation times, make you expose unecessary code, and create more dependencies between files.
• Templates don't enforce any design -- how do I know the contract to implement to make my class compatible with the template functions? With a base class, everything is clear.
• How do users know what template type "T" is supposed to be for such functions? The API would become such a mess.

In my opinion, templates are a very bad choice in this context.

std::variant

That would make things more complicated, and performances would not even be better.

onCreate and onResize

These functions exist for internal needs, and as far as I remember, they were the only way to solve specific problems. But I'll be happy to reconsider this part of the code if a better solution to these problems is found.

I don't understand why you fight so strongly public inheritance. It's a very common/useful/powerful concept in OO programming and in C++; not something that should be avoided at all costs.

I also fail to see how your proposed solutions satisfy your 3 "improvements" (performances, design, coding practice).

But that's just my opinion, let's see what others think about it.

[Bromeon]

Totally agree with Laurent here. The "S" in SFML stands for "simple", and I think your proposal would work against this idea. For example, look at some Boost libraries to see what the results of template-based overengineering are: a hard-to-understand API with implicit constraints, compiling for eternities, and resulting in pages of unintuitive compiler errors at the slightest user mistake.

Regarding performance, you only pay for virtual function calls when they are done indirectly (dynamic). If the compiler can determine at compile time which type to call the method on, the indirection can be optimized away. The memory overhead of the VTable is most often negligible. There are entire game engines written in C# or Java, where _every single object_ is heap-allocated and contains dozens of bytes of book-keeping and reflection metadata overhead. This shouldn't be a justification; the point is that the overhead in C++ is extremely small (most often pay-what-you-use), and to show that virtual functions in SFML are a real-world performance problem, we would need some benchmarks.

Also, what people often forget is that runtime polymorphism solves a real problem: deciding what to do, based on information _only available at runtime_. You cannot simply replace that with templates in all scenarios and still have the flexibility -- at some point you need to do dynamic dispatch, if it's through a VTable or by hand-writing it (if-else/switch/map + type ID).

[groundswellaudio]

Just to be clear: this is about public inheritance, not virtual inheritance; these are two different (but non-exclusive) concepts in C++.

I've said virtual inheritance when what i really meant was "runtime polymorphic inheritance". Public inheritance does not imply runtime polymorphism.

Converting half of SFML and user code to templated code, just to avoid indirections (v-table calls), is just insane.
Templates increase compilation times, make you expose unecessary code, and create more dependencies between files.

To clarify : i'm not proposing to make everything a template, just transform the functions that takes virtual base classes references into an overload set and/or template functions, and avoid trying to reuse code with polymorphism by sticking common code elsewhere. A lot of these changes would in fact be very small. Look at the RenderTarget draw() method : it's just drawable.draw(*this, states), why is runtime polymorphism needed here?

Templates don't enforce any design -- how do I know the contract to implement to make my class compatible with the template functions? With a base class, everything is clear.

Fair point , but :
1/ Since templates functions are in the header anyway, you can see immediately what is being expected
2/ Pre C++20, it's pretty easy to write custom type traits and use static_assert to provide helpful error messages
3/ C++20 concepts entirely solve this issue
4/ i'm all for self-documenting code, but at the end of the day, code is code and not documentation, things can be tacitly agreed upon/commented/documented fully elsewhere (the C++ STL is a great example of this)

How do users know what template type "T" is supposed to be for such functions? The API would become such a mess.

i don't understand your point here. if we know that some functions must works for a given set of types, then we can just
provides overload for this set.

std::variant
That would make things more complicated, and performances would not even be better.

This is admittedly a minor issue, but I shudder when I see C++ code using enumeration + type erasure. std::variant is fully type safe, and std::visit is easy to use

Bromeon :

Regarding performance, you only pay for virtual function calls when they are done indirectly (dynamic). If the compiler can determine at compile time which type to call the method on, the indirection can be optimized away.

Every functions using these types in the SFML API use them through their base classes, and these functions cannot be analyzed by the compilers because they are not inline. Even with full link time optimization, i'd be surprised to see them being optimized away.

Also, what people often forget is that runtime polymorphism solves a real problem: deciding what to do, based on information only available at runtime.

Yes, but my point was that SFML use polymorphic base classes without actually needing the runtime dispatch anywhere.

You cannot simply replace that with templates in all scenarios and still have the flexibility -- at some point you need to do dynamic dispatch, if it's through a VTable or by hand-writing it (if-else/switch/map + type ID).

Yes, but when you really need runtime polymorphism it's good practice to "hide" it from the rest of the codebase, because a runtime polymorphic interface depends on the context in which it is used. As in, if i really need a polymorphic RenderTarget somewhere, I can privately define an interface and a wrapper for any types passed in, (as is described in the talk linked).

[Bromeon]

I've said virtual inheritance when what i really meant was "runtime polymorphic inheritance". Public inheritance does not imply runtime polymorphism.

True, but it has become good practice in C++ to make destructors of public base classes virtual, in the event that someone uses a base class polymorphically. The price to pay is very little compared to the potential UB havoc, especially for classes whose objects are already big.

Fair point , but :
1/ Since templates functions are in the header anyway, you can see immediately what is being expected
2/ Pre C++20, it's pretty easy to write custom type traits and use static_assert to provide helpful error messages
3/ C++20 concepts entirely solve this issue
4/ i'm all for self-documenting code, but at the end of the day, code is code and not documentation, things can be tacitly agreed upon/commented/documented fully elsewhere (the C++ STL is a great example of this)

You're basically saying: "we can _almost_ achieve the existing behavior, if we add type traits, static_assert, concepts and extra documentation". This makes both API and implementation _less_ simple.

Every functions using these types in the SFML API use them through their base classes, and these functions cannot be analyzed by the compilers because they are not inline. Even with full link time optimization, i'd be surprised to see them being optimized away.

If it calls virtual functions through base classes, it does so either for abstraction purposes (caller doesn't need to have the dependency of the implementation) or for dynamic polymorphism (allowing different implementations).

However, if _a user_ has a sf::CircleShape and calls methods on it, the compiler has a very easy job of making the call static. Plus, the user is free to use a sf::Shape when he needs the abstraction.

Yes, but when you really need runtime polymorphism it's good practice to "hide" it from the rest of the codebase, because a runtime polymorphic interface depends on the context in which it is used. As in, if i really need a polymorphic RenderTarget somewhere, I can privately define an interface and a wrapper for any types passed in, (as is described in the talk linked).

But wouldn't that rather advocate a non-polymorphic adapter in the front, which calls then to a polymorphic "back-end"? This is an option, but comes with a lot of code duplication (C++ doesn't have a proper language feature to abstract delegation), and would disallow valid polymorphic use cases.

---

TLDR: While there's definitely room for discussing different API designs with their pros and cons, I think you overstate the problems of SFML's current API. I'm not convinced that getting rid of runtime polymorphism is a categoric improvement in the way you describe it.

[Bromeon]

Off-topic: I personally don't consider std::variant and in particular std::visit() great attempts at algebraic sum types. They are OK given C++'s nature and being library-only, but they make code verbose, non-local and less expressive without named fields.

A nice solution to sum types would be Kotlin sealed classes, or even better, Rust enums + pattern matching.
Might be interesting to try an alternative type-safe approach in C++, even if it probably needs to be macro-based 🙂

[groundswellaudio]

True, but it has become good practice in C++ to make destructors of public base classes virtual, in the event that someone uses a base class polymorphically.

Said who? You can't use a non polymorphic base class polymorphically. Why would anyone store a pointer to a non polymorphic base class?

You're basically saying: "we can almost achieve the existing behavior, if we add type traits, static_assert, concepts and extra documentation". This makes both API and implementation less simple.

No, I'm saying we can actually do better with littles changes, using language mechanisms that any competent C++ developer is familiar with. Functions overloading is not hard to understand. Templates are not hard to document. Even less so in C++20, where they are not even hard to constrain (and i know it's too early to migrate to C++20). That doesn't make the API harder : if you have some requirements on a type in a function template, put that in the documentation, it's not any harder to read than seeking the base class definition.

If it calls virtual functions through base classes, it does so either for abstraction purposes (caller doesn't need to have the dependency of the implementation)
dependency of the implementation

What does this means?

However, if a user has a sf::CircleShape and calls methods on it, the compiler has a very easy job of making the call static. Plus, the user is free to use a sf::Shape when he needs the abstraction.

And how is the compiler going to do that, given that the implementation of the function is in another translation unit?
edit : i was previously talking about how the compiler cannot see through functions taking a polymorphic base class, obviously calling methods on a concrete type never result in a virtual call.

TLDR: While there's definitely room for discussing different API designs with their pros and cons, I think you overstate the problems of SFML's current API. I'm not convinced that getting rid of runtime polymorphism is a categoric improvement in the way you describe it.

Don't get me wrong : I'm not saying this is a huge problem, but it's a low hanging fruit, and I've used enough API where the indiscriminate use of runtime polymorphism made them a chore to work with (if you think making everything a template is bad, try using an API where everything is a virtual function/callback) to try to push SFML in the opposite direction.
On a side note, I think there's a good amount of beginners learning C++ with SFML, and you don't want a novice programmer to think that proper generic programming and runtime polymorphism are equivalent.

[groundswellaudio]

PS :

Off-topic: I personally don't consider std::variant and in particular std::visit() great attempts at algebraic sum types. They are OK given C++'s nature and being library-only, but they make code verbose, non-local and less expressive without named fields.

The big advantage of variant and visit is that you can work with open set of functions very easily. For example, if I have an variant V containing a window event, dispatching it onto an object O is simply std::visit( V, [&o] (auto e) { on_event(o, e); } );
Whenever O wants another event, the implementor can simply write the needed overload without having access to the calling code. This is a very powerful mechanism.

[Bromeon]

I think the discussion is drifting off from the original API design, the main points have been made. But to approach it from a different angle:

Do you have a C++ library in mind, which has an great API in your opinion? The C++ ecosystem is so diverse, you see everything from C with Classes to TMP insanity, thus it might be interesting to see what concretely you're thinking of 🙂

Thanks for the explanation of this std::visit() usage! It looks like Rust's if let; also this pattern seems quite cool. Those are definitely nice approaches given the state of C++, but sometimes I wonder why such an incredible amount of effort is put into language features for library writers, while ergonomics of daily usage falls short. Might just be an oversight on the other hand.

[groundswellaudio]

Do you have a C++ library in mind, which has an great API in your opinion?

I have yet to come across a library for interactive applications which takes advantage of C++ expressive power, and after trying to use more complete libraries I'm coming back to SFML/SDL because at least I can get the manual control I need (and without dependencies hell). It's a hard and open problem (providing useful abstractions over the soup that's OpenGL is alone a difficult task), but it can be done.