the way to probably speed up sfml's image loading

[Mr_Blame]

Once upon a time i was diving on SFML 2.3.1 source code and i found the image loading function and i saw the std::vector usage for holding bytes of image. I think that std::vector must be replaced with somothing else  because the std::vector works like that: when user calls push_back, it causes:
1.)the old data becomes buffer that holds previous values;
2.)than he creates new memory (calls new[size wich is 1 more bigger than previous]), that is empty,
3.) than he fills new memory with buffer's data, than removes the buffer from memory,
4.)and than initilizates the new, empty element of new memory with value that was an argument for push_back() function.

speeking shortly: the std::vector::push_back() function causes the hard allocation and dellocation of memory process. And as we know most of images have in average 1000 bytes inside (or more)!
Example: that std::vector allocates new 1000 bytes in memory than fills it from buffer that has (logically) 999 bytes with which it Fills the new memory(as i know it fills new memory with some kind of loop, maybe

for(...){}

) than removes the buffer(removes 999 bytes!) and than initilizates the new element with push_back() function argument.
Sounds terrible!!!

[Mr_Blame]

I don't know if i am allowed to place sfml-source here but here is what i don't undertstand

(click to show/hide)

Why do we need to load pixels to std::vector? STB image already gives us pointer to full acesses decoded array of pixels.

bool ImageLoader::loadImageFromFile(const std::string& filename, std::vector<Uint8>& pixels, Vector2u& size)
{
    // Clear the array (just in case)
    pixels.clear();

    // Load the image and get a pointer to the pixels in memory
    int width, height, channels;
    unsigned char* ptr = stbi_load(filename.c_str(), &width, &height, &channels, STBI_rgb_alpha);

    if (ptr && width && height)
    {
        // Assign the image properties
        size.x = width;
        size.y = height;

        // Copy the loaded pixels to the pixel buffer
        pixels.resize(width * height * 4);
        memcpy(&pixels[0], ptr, pixels.size());

        // Free the loaded pixels (they are now in our own pixel buffer)
        stbi_image_free(ptr);

        return true;
    }
    else
    {
        // Error, failed to load the image
        err() << "Failed to load image \"" << filename << "\". Reason: " << stbi_failure_reason() << std::endl;

        return false;
    }
}
 

[Jesper Juhl]

std::vector's size() increases by one when you do push_back() but the capacity() increases by much more (commonly doubles). So pushing 1000 elements into a vector does not result in 1000 allocations (since that would be terrible).
If the final size of the vector is known up-front, then you can call reserve() on the vector first to pre-allocate all the memory.
In short: a vector is just fine for something like this.

[Mr_Blame]

But i still does not understand why must we store image data in std::vector if STB image libray already gives us pointer to full acesses array of pixels after finishing loading.

[Mario]

You misunderstood the code.

The function quoted by you uses a std::vector<Uint8> by reference to pass the loaded image data. This is essentially one C++ way to pass "allocated" memory without knowing the required size beforehand.

Since the required memory is reserved in advance (by calling sf::Vector::resize()), there's only one reallocation (if at all).

This vector will never hold more than one image at a time and the contents are assigned at one time only (the memcpy() line). So the (indeed) expensive reallocation upon adding elements essentially never happens.

The only point I see, which might essentially cause some minor thing, is the actual call to std::vector::resize(), since I'm not 100% sure how it changes the reserved size (might stay the same or could double).

The reason for storying the image data "twice" is the fact that the original pixels array returned by stbi is owned by stbi. Essentially copying that data and freeing the stbi owned memory is just one way to better be able to track the image data in a constant way (remember: the image data might be loaded in some other way without stbi, so there isn't always memory allocated by stbi).

[Mr_Blame]

But still the code as I know isn't the fastest as it could be in some cases .

First of all the mecpy function makes binary coping, that means bit by bit. I think that is awful  .

Secondly there is way to make more fast way - this is using dynamic memory and there is a way to store the size of array without the std:vector - this formula is inside code that I posted:

width * height * 4

So I think that it would be released like that:
1.) copy the data from stb image array to another using new[width * height * 4]
2.) using same formula for memory reserving store the size of array

width * height * 4

So we get a simple structure for holding image

struct ImageData{
     sf::Uint8* data; //pointer to allocated data-array
     size_t size; //number of elements in array(width * height * 4)
};
 

[Laurent]

I don't mean to be rude, but please don't talk about things that you don't know. I could take all your points and prove how they are wrong, but I haven't got the time right now. Maybe someone will be kind enough to do it nicely.

[Jesper Juhl]

First of all the mecpy function makes binary coping, that means bit by bit. I think that is awful  .

Have you ever taken a look at an optimized implementation of memcpy? I'm guessing no - if you had, you'd know that it does not naively copy bit-by-bit or even byte-by-byte. You can't beat memcpy for speed (I'd like to see you try at least).

[Hapax]

Secondly there is way to make more fast way - this is using dynamic memory and there is a way to store the size of array without the std:vector
[...]
So I think that it would be released like that:
1.) copy the data from stb image array to another using new[width * height * 4]
[...]

I find it odd that you're complaining about the data being copied and then suggest a "faster" version "using dynamic memory" which copies the data.
It looks like you have a personal vendetta against std::vector (maybe against all of STL?) as you suggest doing the same thing using a raw pointer instead. Firstly, there are much safer pointers to use. Secondly, a vector uses memory in the same way that an array does. Its elements are laid out contiguously. Therefore, a pointer to the first element of a vector is identical to a pointer to the first element in a array that stores the same data.

[Mr_Blame]

Oh sorry. 
No i see that whole post looks supid  thank for Hapax and others for explaning it.