Send sf::Texture using sf::Packet

[GadgetMan]

Hi.

I'm currently working on a game idea for a tile based multiplayer game in which I need to send a sf::Texture to the connected clients. This is the relevant code:

Server:

sf::Packet& operator << (sf::Packet& packet, sf::Texture texture)
{
        packet << texture.getSize().x << texture.getSize().y;

        for (int i = 0; i < texture.getSize().x * texture.getSize().y * 4; i++)
                packet << texture.copyToImage().getPixelsPtr()[i];

        return packet;
}

Client:

sf::Packet& operator >> (sf::Packet& packet, sf::Texture texture)
{
        unsigned int x, y;
        packet >> x >> y;

        tmpUint = new sf::Uint8[x * y * 4];

        for (int i = 0; i < x * y * 4; i++)
                packet >> tmpUint[i];

        tmpImg.create(x, y, tmpUint);

        return packet;
}

tmpUint and tmpImg are in a structure which i use instead of the texture in the package to store them for later use.

sf::Uint8* tmpUint = NULL;
sf::Image tmpImg;

So far I haven't been able to figure out the issue and I've spent quite some time now looking through the forums and through google results.

[eXpl0it3r]

This seems like it could work, as such I wonder what exactly did you expect from us?

    for (int i = 0; i < texture.getSize().x * texture.getSize().y * 4; i++)
        packet << texture.copyToImage().getPixelsPtr()[i];

This code is very inefficient, because calling copyToImage() is a heavy operation that moves the image data from the GPU to the RAM and you're doing this for every single pixel.

[Nexus]

Two other major issues with your code:

sf::Packet& operator << (sf::Packet& packet, sf::Texture texture)
sf::Packet& operator >> (sf::Packet& packet, sf::Texture texture)

<< is very inefficient, >> won't work at all. Use references as described in the tutorial (please read that carefully!).

tmpUint = new sf::Uint8[x * y * 4];

That's a nice memory leak. There's no reason to use new, work with a std::vector<sf::Uint8> instead.

And three minor issues in this line:

for (int i = 0; i < texture.getSize().x * texture.getSize().y * 4; i++)

• Use std::size_t (or at least unsigned int), not int -- you'll only get annoying conversion warnings otherwise
  
• ++i instead of i++ (convention, can be faster with iterators)
• precompute the size

I would also be careful with non-fixed size types. (unsigned) int has no guaranteed size, that's why there are the sf::{Int|Uint}{8|16|32|64} typedefs.

And you might consider working with sf::Image rather than sf::Texture. You only need textures for rendering, and you can still load them from images when you need that.

[GadgetMan]

Thank you for the quick replies. I've changed the code a bit but even though I'm not able to perform this in an acceptable amount of time...

This is what the code looks like:

sf::Packet& operator << (sf::Packet& packet, sf::Texture& texture)
{
        size_t sizeX, sizeY;
        sizeX = texture.getSize().x;
        sizeY = texture.getSize().y;

        packet << sizeX << sizeY;

        sf::Image tmpImg = texture.copyToImage();

        std::vector<sf::Uint8> tmpUint;
        for (int i = 0; i < sizeX * sizeY * 4; ++i)
        {
                tmpUint.push_back(tmpImg.getPixelsPtr()[i]);
        }

        for (int i = 0; i < sizeX * sizeY * 4; ++i)
        {
                packet << tmpUint.at(i);
        }

        return packet;
}

This is only the server-side code this time because I don't want to try and fix the client as long as the server doesn't work fast enough. I'm later going to adapt the remaining suggestions because imo they shouldn't really affect the performance.

Is there a way i can get the sf::Uint8* returned by getPixelsPtr() directly into a std::vector? The way my code looks now I'm questioning how using a vector would really improve performance... because puting it into the vector takes as much time as putting them into the packet afterwards. How can I improve the performance of this method? Also where can I find information regarding performance analysis so I can learn to see my own mistakes and fix them without having to rely on someone else to know better?

[Nexus]

Again: Don't send sf::Textures, send sf::Images! That is, don't copy unnecessarily, and don't use the GPU to store data if you don't need it for rendering. The server side never requires a texture.

Yes, have a look at the various std::vector constructors. One overload takes an iterator range, you can use that. Of course you don't need to copy everything to a vector if you don't store the data -- you can directly work on the temporary buffer provided by getPixelsPtr(). Just don't write to it.

[GadgetMan]

Thank you for the answers. I managed to solve the problem by changing the code to the following:

sf::Packet& operator << (sf::Packet& packet, Tileset& tileset)
{
        size_t sizeX, sizeY;
        sizeX = tileset.getSpriteTexture().getSize().x;
        sizeY = tileset.getSpriteTexture().getSize().y;

        int size = sizeX * sizeY * 4;

        packet << tileset.tileSize;
        packet << sizeX << sizeY;

        sf::Image tmpImg = tileset.getSprite()->getTexture()->copyToImage();

        const sf::Uint8* tmpUint = tmpImg.getPixelsPtr();
       
        for (int i = 0; i < size; ++i)
        {
                packet << tmpUint[i];
        }

        return packet;
}
 

Somehow removing the "put the Uint8* in a vector" part made it work the way I wanted it to work... even though I don't understand why, because before removing it, the loop to put all sf::Uint8s in the packet took as long as the conversion.

In my case the server side DOES require a texture because it is like another part of the game. Like a GM in classic P&P Games/Tabletop games (basically the player who is running the server is also able to play at the same time and just has more "possibilities")