Welcome, Guest. Please login or register. Did you miss your activation email?

Author Topic: sf::String obsolete?  (Read 7569 times)

0 Members and 1 Guest are viewing this topic.

Tank

  • SFML Team
  • Hero Member
  • *****
  • Posts: 1486
    • View Profile
    • Blog
    • Email
sf::String obsolete?
« on: February 06, 2014, 02:23:17 pm »
Hey,

when this commit was pushed, some guys in #sfgui started discussing about why sf::String is needed at all.

The only data member is a std::basic_string, which makes the class itself a pure proxy. The conversion functions for the Unicode stuff is implemented using free functions, so there's not really a need for sf::String anymore.

I'd therefore ask for a discussion about removing sf::String and simply using std::basic_string<sf::Uint32>.

You may also want to checkout an older thread that was about exactly the same problem.

eXpl0it3r

  • SFML Team
  • Hero Member
  • *****
  • Posts: 11034
    • View Profile
    • development blog
    • Email
Re: sf::String obsolete?
« Reply #1 on: February 06, 2014, 02:27:37 pm »
some guys in #sfgui started discussing about why sf::String is needed at all.
I was there as well. ;D

Going back to when it was first introduced, it's clear that nobody was really satisfied with the class. Laurent even said "Like I said, this solution is kind of temporary" and "I'm still not satisfied with this solution".

I've yet to see a reason for having such a proxy class vs using std::basic_string directly and free conversion functions. Thus I'd be interested in seeing what the reasoning was/is for such a class.
Official FAQ: https://www.sfml-dev.org/faq.php
Official Discord Server: https://discord.gg/nr4X7Fh
——————————————————————
Dev Blog: https://duerrenberger.dev/blog/

Nexus

  • SFML Team
  • Hero Member
  • *****
  • Posts: 6287
  • Thor Developer
    • View Profile
    • Bromeon
Re: sf::String obsolete?
« Reply #2 on: February 06, 2014, 02:41:49 pm »
An important point was simplicity. std::basic_string<sf::Uint32> does not offer implicit conversions and it uses a different naming convention (which was more severe when SFML used PascalCase).

The lack of implicit conversions would complicate the API a lot. Simple code such as
sf::Text text;
text.setString("My text");
would be no longer possible.
Zloxx II: action platformer
Thor Library: particle systems, animations, dot products, ...
SFML Game Development:

wintertime

  • Sr. Member
  • ****
  • Posts: 255
    • View Profile
Re: sf::String obsolete?
« Reply #3 on: February 06, 2014, 03:26:21 pm »
C++11 gives char32_t and u32string, so I would think its better to wait and only change it once than change it twice (now and again when SFML starts using C++11).

Tank

  • SFML Team
  • Hero Member
  • *****
  • Posts: 1486
    • View Profile
    • Blog
    • Email
Re: sf::String obsolete?
« Reply #4 on: February 06, 2014, 03:39:14 pm »
Quote
uses a different naming convention (which was more severe when SFML used PascalCase).
I don't think that's a valid point, because the string class would be completely out of SFML. Otherwise you had to argue that std::vector also uses a different naming scheme. ;)

Regarding implicit conversions you could provide a conversion class that takes care of the implicit conversions, like this:

class StringConversion {
  public:
    StringConversion( const char* string );
    StringConversion( const std::string& string );
    // Etc...
    std::basic_string<sf::Uint32> m_string; // Gets loaded with converted string.
};

And then use StringConversion everywhere where sf::String is currently being used. It's even a good way to support char32_t and u32string in the future, as wintertime mentioned.

Nexus

  • SFML Team
  • Hero Member
  • *****
  • Posts: 6287
  • Thor Developer
    • View Profile
    • Bromeon
Re: sf::String obsolete?
« Reply #5 on: February 06, 2014, 04:35:33 pm »
I don't think that's a valid point, because the string class would be completely out of SFML. Otherwise you had to argue that std::vector also uses a different naming scheme. ;)
The point is that there would probably have been a sf::String typedef for it, so it would have looked like an SFML class. But anyway, that wasn't the main criterion.

Regarding implicit conversions you could provide a conversion class that takes care of the implicit conversions
But then you still have a custom string class in the API. The difference is that you can't modify it directly, only through conversions from other string types. This has some drawbacks: when the StringConversion class is used in the API and std::basic_string<sf::Uint32> on user side, there would be a constant need to copy. An API of sf::String on the other hand allows the user to use sf::String as well, and pass const references.

The word "allows" is important; it doesn't enforce it. It's already now possible to have std::basic_string<sf::Uint32> on user side, and implicitly convert it to sf::String.

At the moment we can't do much anyway: sf::String can't be removed, and a new solution should also take C++11 features into account, such as new standard character/string types and move semantics.
« Last Edit: February 06, 2014, 04:45:36 pm by Nexus »
Zloxx II: action platformer
Thor Library: particle systems, animations, dot products, ...
SFML Game Development:

binary1248

  • SFML Team
  • Hero Member
  • *****
  • Posts: 1405
  • I am awesome.
    • View Profile
    • The server that really shouldn't be running
Re: sf::String obsolete?
« Reply #6 on: February 07, 2014, 01:41:40 pm »
But then you still have a custom string class in the API. The difference is that you can't modify it directly, only through conversions from other string types.
The idea was that it is only part of the public interface. StringConversion objects are never stored and would only exist as temporary objects during the duration of the call.

This has some drawbacks: when the StringConversion class is used in the API and std::basic_string<sf::Uint32> on user side, there would be a constant need to copy. An API of sf::String on the other hand allows the user to use sf::String as well, and pass const references.
Yeah... and what do you do with the const references? Right... you copy them into the object's own storage. This is the reason why it makes more sense to pass sink objects as values in C++11, because in almost all cases, you will be moving from an r-value, or performing a copy anyway. The only difference is when you perform the copy. Browsing through SFML source, I couldn't find many places where the contents of the reference are merely "inspected" i.e. not copied. Almost all of them were in some form of conversion function back to wide strings, which implies copying yet again. Even if moves aren't present, copy elision would result in superior performance.

I took the time to create a test to measure the performance penalty of all these alleged copies that are potentially made.
#include <iostream>
#include <string>
#include <iterator>
#include <cstring>
#include <SFML/System.hpp>

class StringConverter {
public:
        StringConverter( const char* ansiString, const std::locale& locale = std::locale() ) {
                if( ansiString ) {
                        std::size_t length = strlen( ansiString );
                        if( length > 0 ) {
                                m_string.reserve( length );
                                sf::Utf32::fromAnsi( ansiString, ansiString + length, std::back_inserter( m_string ), locale );
                        }
    }
        }

        StringConverter( std::basic_string<unsigned int> utf32String ) : m_string( utf32String ) {}

        // More conversions here...

        operator std::basic_string<unsigned int>() {
                return m_string; // <-- Conversions return an r-value.
        }
private:
        std::basic_string<unsigned int> m_string;
};

class Text {
public:
        void setString( StringConverter string ) {
                m_string = string; // <-- Request a conversion from a (potential) r-value. Even if no move, copy can be elided.

                // Just to prevent too much optimization.
                if( m_string.empty() ) {
                        m_string += 1000;
                        std::cout << m_string.length();
                }
        }
private:
        std::basic_string<unsigned int> m_string;
};

int main() {
        std::basic_string<unsigned int> str( 100000000, 1000 );

        for( int i = 0; i < 100000; i++ ) {
                Text text;
                text.setString( "Hello, World!" );
                text.setString( str );
        }
}
I built the executable 3 times using -O -O1 and -O2 with profiling enabled.

These are the flat profiles:
-O:
Code: [Select]
Flat profile:

Each sample counts as 0.01 seconds.
  %   cumulative   self              self     total           
 time   seconds   seconds    calls  us/call  us/call  name   
 47.06      0.48     0.48                             _mcount_private
 14.71      0.63     0.15                             _spin_lite_lock
 12.75      0.76     0.13                             _spin_lite_unlock
  3.92      0.80     0.04 10300001     0.00     0.00  std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_M_data() const
  1.96      0.82     0.02  9000001     0.00     0.01  std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_M_rep() const
  1.96      0.84     0.02  2900001     0.01     0.01  __gnu_cxx::char_traits<unsigned int>::assign(unsigned int&, unsigned int const&)
  1.96      0.86     0.02  1300000     0.02     0.02  std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_Rep::_M_is_shared() const
  1.96      0.88     0.02  1300000     0.02     0.08  std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::push_back(unsigned int)
  1.96      0.90     0.02   600001     0.03     0.07  std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::~basic_string()
  1.96      0.92     0.02   300001     0.07     0.07  std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_Rep::_M_destroy(std::allocator<unsigned int> const&)
  1.96      0.94     0.02                             __pthread_self_lite
  0.98      0.95     0.01  3300003     0.00     0.00  __gnu_cxx::new_allocator<unsigned int>::~new_allocator()
  0.98      0.96     0.01  1300000     0.01     0.01  unsigned int sf::Utf<32u>::decodeAnsi<char>(char, std::locale const&)
  0.98      0.97     0.01   600000     0.02     0.02  std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_Rep::_M_grab(std::allocator<unsigned int> const&, std::allocator<unsigned int> const&)
  0.98      0.98     0.01   100000     0.10     0.17  Text::~Text()
  0.98      0.99     0.01                             _Unwind_SjLj_Unregister
  0.98      1.00     0.01                             __gnu_cxx::__atomic_add(int volatile*, int)
  0.98      1.01     0.01                             _fentry__
  0.98      1.02     0.01                             pthread_setspecific
  0.00      1.02     0.00  4500003     0.00     0.00  __gcc_deregister_frame
  0.00      1.02     0.00  3000000     0.00     0.01  std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::size() const
  0.00      1.02     0.00  2900002     0.00     0.00  __gnu_cxx::new_allocator<unsigned int>::new_allocator(__gnu_cxx::new_allocator<unsigned int> const&)
  0.00      1.02     0.00  2700002     0.00     0.00  std::allocator<unsigned int>::~allocator()
  0.00      1.02     0.00  2500003     0.00     0.00  std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_Rep::_M_refdata()
  0.00      1.02     0.00  2300001     0.00     0.00  std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::get_allocator() const
  0.00      1.02     0.00  2300001     0.00     0.00  std::allocator<unsigned int>::allocator(std::allocator<unsigned int> const&)
  0.00      1.02     0.00  1900002     0.00     0.00  std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_Rep::_M_set_sharable()
  0.00      1.02     0.00  1600001     0.00     0.01  std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_Rep::_M_set_length_and_sharable(unsigned int)
  0.00      1.02     0.00  1400000     0.00     0.01  std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::capacity() const
  0.00      1.02     0.00  1300000     0.00     0.08  std::back_insert_iterator<std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> > >::operator=(unsigned int const&)
  0.00      1.02     0.00  1300000     0.00     0.00  std::back_insert_iterator<std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> > >::operator*()
  0.00      1.02     0.00  1300000     0.00     0.00  std::back_insert_iterator<std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> > >::operator++(int)
  0.00      1.02     0.00   900001     0.00     0.02  std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_Rep::_M_dispose(std::allocator<unsigned int> const&)
  0.00      1.02     0.00   600002     0.00     0.00  __gnu_cxx::new_allocator<char>::new_allocator()
  0.00      1.02     0.00   600002     0.00     0.00  std::allocator<char>::allocator<unsigned int>(std::allocator<unsigned int> const&)
  0.00      1.02     0.00   600001     0.00     0.00  std::allocator<unsigned int>::allocator(std::allocator<unsigned int> const&)
  0.00      1.02     0.00   600001     0.00     0.00  std::allocator<unsigned int>::~allocator()
  0.00      1.02     0.00   600001     0.00     0.00  std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_Alloc_hider::_Alloc_hider(unsigned int*, std::allocator<unsigned int> const&)
  0.00      1.02     0.00   600001     0.00     0.00  std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_Alloc_hider::~_Alloc_hider()
  0.00      1.02     0.00   600000     0.00     0.00  std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_Rep::_M_is_leaked() const
  0.00      1.02     0.00   600000     0.00     0.00  std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_Rep::_M_refcopy()
  0.00      1.02     0.00   600000     0.00     0.00  bool std::operator==<unsigned int>(std::allocator<unsigned int> const&, std::allocator<unsigned int> const&)
  0.00      1.02     0.00   400001     0.00     0.00  __gnu_cxx::new_allocator<unsigned int>::new_allocator()
  0.00      1.02     0.00   400001     0.00     0.00  std::allocator<unsigned int>::allocator()
  0.00      1.02     0.00   400000     0.00     0.03  std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::basic_string(std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> > const&)
  0.00      1.02     0.00   300001     0.00     0.00  __gnu_cxx::new_allocator<char>::deallocate(char*, unsigned int)
  0.00      1.02     0.00   300001     0.00     0.00  __gnu_cxx::new_allocator<char>::allocate(unsigned int, void const*)
  0.00      1.02     0.00   300001     0.00     0.00  __gnu_cxx::new_allocator<char>::max_size() const
  0.00      1.02     0.00   300001     0.00     0.00  std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_Rep::_S_create(unsigned int, unsigned int, std::allocator<unsigned int> const&)
  0.00      1.02     0.00   300001     0.00     0.00  operator new(unsigned int, void*)
  0.00      1.02     0.00   300000     0.00     0.00  std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_M_data(unsigned int*)
  0.00      1.02     0.00   200001     0.00     0.01  std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_S_construct(unsigned int, unsigned int, std::allocator<unsigned int> const&)
  0.00      1.02     0.00   200000     0.00     0.07  StringConverter::~StringConverter()
  0.00      1.02     0.00   200000     0.00     0.03  StringConverter::operator std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >()
  0.00      1.02     0.00   200000     0.00     0.18  Text::setString(StringConverter)
  0.00      1.02     0.00   200000     0.00     0.01  std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::empty() const
  0.00      1.02     0.00   200000     0.00     0.07  std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::assign(std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> > const&)
  0.00      1.02     0.00   200000     0.00     0.01  std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::basic_string()
  0.00      1.02     0.00   200000     0.00     0.07  std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::operator=(std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> > const&)
  0.00      1.02     0.00   100000     0.00     1.20  StringConverter::StringConverter(char const*, std::locale const&)
  0.00      1.02     0.00   100000     0.00     0.03  StringConverter::StringConverter(std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >)
  0.00      1.02     0.00   100000     0.00     1.13  std::back_insert_iterator<std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> > > sf::Utf<32u>::fromAnsi<char const*, std::back_insert_iterator<std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> > > >(char const*, char const*, std::back_insert_iterator<std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> > >, std::locale const&)
  0.00      1.02     0.00   100000     0.00     0.01  Text::Text()
  0.00      1.02     0.00   100000     0.00     0.01  std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_Rep::_M_clone(std::allocator<unsigned int> const&, unsigned int)
  0.00      1.02     0.00   100000     0.00     0.06  std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::reserve(unsigned int)
  0.00      1.02     0.00   100000     0.00     0.00  std::back_insert_iterator<std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> > >::back_insert_iterator(std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >&)
  0.00      1.02     0.00   100000     0.00     0.00  std::iterator<std::output_iterator_tag, void, void, void, void>::iterator()
  0.00      1.02     0.00   100000     0.00     0.00  std::back_insert_iterator<std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> > > std::back_inserter<std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> > >(std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >&)
  0.00      1.02     0.00        1     0.00     0.00  __gnu_cxx::char_traits<unsigned int>::assign(unsigned int*, unsigned int, unsigned int)
  0.00      1.02     0.00        1     0.00     0.01  unsigned int* std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_S_construct<int>(int, int, std::allocator<unsigned int> const&)
  0.00      1.02     0.00        1     0.00     0.01  unsigned int* std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_S_construct_aux<int>(int, int, std::allocator<unsigned int> const&, std::__true_type)
  0.00      1.02     0.00        1     0.00     0.01  std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_S_construct_aux_2(unsigned int, unsigned int, std::allocator<unsigned int> const&)
  0.00      1.02     0.00        1     0.00     0.00  std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_M_assign(unsigned int*, unsigned int, unsigned int)
  0.00      1.02     0.00        1     0.00     0.01  std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::basic_string<int>(int, int, std::allocator<unsigned int> const&)
  0.00      1.02     0.00        1     0.00     0.00  std::_Iter_base<unsigned int*, false>::_S_base(unsigned int*)
  0.00      1.02     0.00        1     0.00     0.00  __gnu_cxx::__enable_if<std::__is_scalar<unsigned int>::__value, unsigned int*>::__type std::__fill_n_a<unsigned int*, unsigned int, unsigned int>(unsigned int*, unsigned int, unsigned int const&)
  0.00      1.02     0.00        1     0.00     0.00  std::_Niter_base<unsigned int*>::iterator_type std::__niter_base<unsigned int*>(unsigned int*)
  0.00      1.02     0.00        1     0.00     0.00  unsigned int* std::fill_n<unsigned int*, unsigned int, unsigned int>(unsigned int*, unsigned int, unsigned int const&)
-O1:
Code: [Select]
Flat profile:

Each sample counts as 0.01 seconds.
  %   cumulative   self              self     total           
 time   seconds   seconds    calls  ns/call  ns/call  name   
 37.50      0.03     0.03                             main
 12.50      0.04     0.01   400000    25.00    25.00  std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::basic_string(std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> > const&)
 12.50      0.05     0.01                             _Unwind_SjLj_Unregister
 12.50      0.06     0.01                             std::locale::~locale()
 12.50      0.07     0.01                             operator new(unsigned int)
 12.50      0.08     0.01                             _mcount_private
  0.00      0.08     0.00   300001     0.00     0.00  std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_Rep::_S_create(unsigned int, unsigned int, std::allocator<unsigned int> const&)
  0.00      0.08     0.00   200001     0.00     0.00  std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_S_construct(unsigned int, unsigned int, std::allocator<unsigned int> const&)
  0.00      0.08     0.00   200000     0.00     0.00  std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::assign(std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> > const&)
  0.00      0.08     0.00   100000     0.00     0.00  std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_Rep::_M_clone(std::allocator<unsigned int> const&, unsigned int)
  0.00      0.08     0.00   100000     0.00     0.00  std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::reserve(unsigned int)
  0.00      0.08     0.00        1     0.00     0.00  std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_Rep::_M_dispose(std::allocator<unsigned int> const&)
-O2:
Code: [Select]
Flat profile:

Each sample counts as 0.01 seconds.
  %   cumulative   self              self     total           
 time   seconds   seconds    calls  ns/call  ns/call  name   
 50.00      0.03     0.03                             main
 33.33      0.05     0.02                             _spin_lite_lock
 16.67      0.06     0.01   100000   100.00   100.00  std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::reserve(unsigned int)
  0.00      0.06     0.00   400002     0.00     0.00  __gcc_deregister_frame

As you can see, with all optimizations disabled, your fear of excessive copies is prevalent. However with higher optimization levels, less and less is done which is a result of copy elision taking place in the possible places. This was not compiled with C++11 enabled and so moves shouldn't have been performed, if they would have been performed the compiler would optimize them away as well and the profile would have looked similar.

The idea is that because the C++ syntax doesn't allow us to express our intentions through direct conversions, we have to take the detour via temporary objects. However, because this is probably a very typical approach, compilers are already ready to optimize these scenarios away, perhaps to the point where the conversion takes place at the initial call site and copied directly into the target object's memory.

I know this is said a lot, but: Don't try to help the compiler, more often than not, it knows better than you what you are trying to do. And if you ask me, performance optimizations should really be a result of the implementation and not the public interface.

The word "allows" is important; it doesn't enforce it. It's already now possible to have std::basic_string<sf::Uint32> on user side, and implicitly convert it to sf::String.
The difference is that the user would have less of a headache working with the values they get back from SFML. Unless the conversion was implicit in the other direction as well, they will keep having to explicitly convert to std::basic_string<sf::Uint32> which isn't a very pretty sight.

At the moment we can't do much anyway: sf::String can't be removed, and a new solution should also take C++11 features into account, such as new standard character/string types and move semantics.
You didn't state the reasons why sf::String can't be removed, merely difficulties for which there is no optimal solution yet. You have to bare in mind that simply activating C++11 when compiling SFML will already make Tank's StringConversion proposal just as fast if not faster than the current implementation. Move semantics will be automatically incorporated into SFML classes (and the StringConversion class) thereby alleviating this constant fear of excessive copying where it is unnecessary. If execution time is dominated more by the conversion back to std::basic_string<sf::Uint32> in the user's code (which is a plausible scenario) then this solution already outperforms the current implementation even without C++11 enabled.

And regarding the new char and string types, it isn't as significant as some people might think. The only change is that the C++ standard finally provides definitions of the size of types that people can rely on. Before C++11 you would have to hope that whatever type you used would be stored using the same number of bits regardless of the platform the code was compiled for. This is pretty much what SFML tries to do with its sf::UintX and sf::IntX types, although with stronger (i.e. compiler conformance) guarantees. All you would have to do to make use of these "new" types is change the typedefs in Config.hpp to reflect the new types and std::basic_string<sf::Uint32> will be exactly the same as the typedefs provided by the standard. One must also not forget that perhaps std::u32string might not be what SFML wants to use, as there is no guarantee that char32_t is exactly 32 bits wide, merely that it is at least 32 bits wide which could hamper communication between hosts on different architectures.

I really can't see any real advantages for the user, performance-wise or development-wise by sticking with sf::String. As long as user code doesn't break (barring sf::String not being defined if simply removed instead of typedefed), it is a win-win scenario for everybody, even SFML maintainers as they would have to do less wheel-reinventing ;).
SFGUI # SFNUL # GLS # Wyrm <- Why do I waste my time on such a useless project? Because I am awesome (first meaning).

Nexus

  • SFML Team
  • Hero Member
  • *****
  • Posts: 6287
  • Thor Developer
    • View Profile
    • Bromeon
Re: sf::String obsolete?
« Reply #7 on: February 07, 2014, 09:01:17 pm »
Thanks for the detailed investigation.

The difference is that the user would have less of a headache working with the values they get back from SFML. Unless the conversion was implicit in the other direction as well, they will keep having to explicitly convert to std::basic_string<sf::Uint32> which isn't a very pretty sight.
There is currently the explicit conversion function
std::basic_string<Uint32> String::toUtf32() const
but no conversion operator.

You didn't state the reasons why sf::String can't be removed, merely difficulties for which there is no optimal solution yet.
We can't remove it now because it breaks a lot of code and without a replacement, all the nice conversions are gone. If we replace sf::String with sf::StringConversion, that's essentially a renaming of the class combined with the removal of all its string-related functions as well as the addition of very few additional conversions. In short, we break a lot of user code without providing an improvement that could not be implemented in the current sf::String class.

The future is a different topic, I totally agree that the handling of strings should be reconsidered. I don't know how Laurent sees that, but in past discussions, he wanted no API changes that break larger amounts of existing code before SFML 3...

You have to bare in mind that simply activating C++11 when compiling SFML will already make Tank's StringConversion proposal just as fast if not faster than the current implementation.
That's what I meant with "move semantics" in the last paragraph ;)

Another important point we should consider in a new design is how much locale and Unicode support SFML would provide, and how it would provide it. How do we link encoding to string types? They are not necessarily dependent on each other (although some defaults like now could be set, in order to still allow implicit conversions). Unfortunately, the whole Unicode and locale handling in C++ is quite a mess. C++11 adds a few Unicode conversions that might be useful, but still...

Something that's not relevant for the discussion, you're right that the occurring copies shouldn't be the decisive criterion in the API design. But out of curiosity, are you sure the compiler doesn't optimize away so much because the Text object is thrown away? Do you think it would make a difference if you stored the objects in a vector and output a random element to the console? I'm just wondering because in my experience, compilers are sometimes much smarter than one would assume :)
Zloxx II: action platformer
Thor Library: particle systems, animations, dot products, ...
SFML Game Development:

Laurent

  • Administrator
  • Hero Member
  • *****
  • Posts: 32498
    • View Profile
    • SFML's website
    • Email
Re: sf::String obsolete?
« Reply #8 on: February 07, 2014, 09:02:21 pm »
Quote
I don't know how Laurent sees that, but in past discussions, he wanted no API changes that break larger amounts of existing code before SFML 3...
The entire SFML 2.x branch should remain API compatible backward. So no removal or modification of the public API. Only additions. But things can become deprecated until SFML 3, though.
Laurent Gomila - SFML developer