Request: allow either move semantics or direct pointer access in sf::Image

[Nall-ohki]

I'm looking at doing some work with Skia, and I want to create a rendering context that renders directly into an Image object.  Unfortunately, the current Image implementation blocks this without performing an expensive texture copy for every render.  There are several solutions that could work for me:

1) Change from private to protected visibility for m_pixels
2)  Provide a non-const getPixelsPtr
3) Give me a c++11 -style constructor or factory function with move semantics, e.g:
     Image(unsigned int width, unsigned int height, std::vector<Uint8> &&pixels);

4) Provide:
     void sf::Image::create(unsigned int width, unsigned int height, std::vector<Uint8> &pixels);
which performs an actual std::vector<>::swap() or std::swap() so that only a pointer exchange occurs.

Out of these, 1) is probably the best.

If one of these could make it into 2.0 it would save me (and I suspect others) from having to create a modified version of SFML to run on my engine.

[Laurent]

sf::Image is just a very simple wrapper around an array of sf::Uint8. You can easily drop it and use your own pixel array instead.

4) Provide:
     void sf::Image::create(unsigned int width, unsigned int height, std::vector<Uint8> pixels);

If you already have a std::vector<sf::Uint8>, why do you want to put it inside a sf::Image? The next step should be sf::Texture.

Out of these, 1) is probably the best.

This is the most ugly in my opinion. 3) is much better, and will be implemented when I add support for C++11 in SFML.

By the way, I don't understand what you're trying to do. Can you give more details?

[Tank]

when I add support for C++11 in SFML.

Have you already decided the target version for C++11 support?

[eXpl0it3r]

If you have a sf::vector<sf::Uint8> and a C++11 compiler you can simply call tex.update(vec.data()).

[Laurent]

Have you already decided the target version for C++11 support?

Maybe 2.1

[Nall-ohki]

Hey, wonderful response everyone.

What I was originally trying to do was create a SkiaImage type derived from Image that could allow Skia to render directly into it's internal buffer without a copy operation.  However, I realize that I came in while still looking at the docs and didn't notice that sf:Texture took raw Uint8 pointers, which makes it possible to do pretty much everything I need without deriving from the sf:Image object (Yay! Design!).

I realize that while I'd still love to see a C++11 style constructor with move semantics (which sounds like it is on the way), I can work with this in a different way.

Thanks!

[MorleyDev]

Just to chime in about the C++11 topic: It's my understanding of move that if you provided

sf::Image (unsigned int Width, unsigned int Height, std::vector<Color> data)

then in theory calling

sf::Image(W, H, std::move(imageVector))

should correctly use the move constructor between imageVector and data so you shouldn't need to add explicit C++11 support yet but it could make the move easier when you do. Providing this kind of interface is, as I understand it, preferable when you don't need to explicitly specialise between moves and copies.

This can be demonstrated very simply, such as with the following test code. It's a Unit Test using my unit testing library but I'd hope it's readable enough to convey the point since that was the whole point of me rolling my own xD. When ran, both the Then's pass, so in the nuwen build of Mingw at least I can confirm what I said is true

[Laurent]

should correctly use the move constructor between imageVector and data so you shouldn't need to add explicit C++11 support

Yes, but then I would have to swap vectors inside the function, instead of copying.

[binary1248]

std::move only creates an rvalue-reference out of whatever you pass to it. You need std::move because there is no other way to tell your compiler you are crazy enough to pass a named variable and expect it to get potentially destroyed in the process. Even if there was a method that took an rvalue, you would still have to do the needed moving and make sure everything stays safe (this includes swapping the other fields of sf::Image). I'd keep my hands off std::move and keep tugging at Laurent to provide an sf::Image constructor that can do some useful data initialization so that we have legitimate access to rvalue sf::Images in the future .

edit: Forgot to mention, rvalue support for sf::Vector would be cool too. That could indirectly speed up SFGUI by a noticable amount .

[Laurent]

Forgot to mention, rvalue support for sf::Vector would be cool too. That could indirectly speed up SFGUI by a noticable amount

sf::Vector2/3 is just 2/3 floats, how could rvalue support could speed up your code? And what kind of rvalue support could I provide for these classes?

[binary1248]

rvalue support not only encompasses construction and assignment but also arithmetic operations. As it is now, when you perform arithmetic on sf::Vectors you create a crapton of temporaries causing the constructor to be called more times than you would expect.

Here is an authentic snippet of a gprof profile of SFGUI test:

  %   cumulative   self              self     total           
 time   seconds   seconds    calls   s/call   s/call  name   
 35.62      7.32     7.32     6919     0.00     0.00  sfg::Renderer::RefreshVBO(sf::RenderTarget const&)
  3.50      8.04     0.72 188042280     0.00     0.00  sf::Vector2<float>::Vector2(float, float)
  3.11      8.68     0.64 132964266     0.00     0.00  std::vector<unsigned int, std::allocator<unsigned int> >::push_back(unsigned int const&)

As you can see, the sf::Vector2 constructor is the second most time consuming function that is called. RefreshVBO contains a lot of sf::Vector2 code that's why it also racks up a lot of time.

If you do something like:

sf::Vector2f a = sf::Vector2f( 1.f, 1.f ) + sf::Vector2f( 2.f, 2.f );
 

You end up with something like 3? constructions, whereas I assume this can be done with less if you use rvalue references appropriately. There are already many other linear algebra libraries that have rvalue support even on "simple" vectors that contain few POD fields.

And don't forget, sf::Vector isn't just restricted to containing floats. If for some reason some insane person wants to use sf::Vector3<sf::Image> then rvalue support just might make a difference .

[Laurent]

Sorry I haven't worked much with rvalues yet; what kind of optimization can you do with it that you can't do with copies? You can safely steal resources of a rvalue since you know it will be destroyed, but what can you steal from a pair of floats? You'll end up copying one float value to another float variable in any case.

And everything's inlined, so in the end it doesn't matter what functions are called with which arguments, I think that only the minimum needed operations are kept in the compiled code -- today's compilers are more than able to handle this kind of code perfectly.

[Nexus]

I think the idea of rvalue references here is not to allow move semantics, it's rather to avoid copies and moves. As you say Laurent, for 2D/3D vectors copies and moves are equally expensive.

I don't see however how there can be fewer than 3 objects in this particular case. The two temporaries must exist, and one cannot safely reuse them for the local object.