Put the onus of saving/restoring textures on the user. #323

Closed
retep998 opened this Issue Dec 6, 2012 · 8 comments

Projects

None yet

4 participants

@retep998
retep998 commented Dec 6, 2012

As mentioned in #321, forcing SFML to automatically save/restore textures is a very costly operation which can severely degrade performance. By simply notifying the user that they are responsible for restoring their own texture bindings after using some SFML feature, we eliminate an expensive glGet.

@LaurentGomila LaurentGomila was assigned Dec 6, 2012
@MarioLiebisch
Member

I'm actually surprised about this, because so far I assumed you'd have to use pushGLState() and popGLState() anyway.

@LaurentGomila
Member

I'm actually surprised about this, because so far I assumed you'd have to use pushGLState() and popGLState() anyway.

push/pop only protects the draw calls. The stuff in sf::Texture ensure that functions such as setSmooth, update, etc. don't change the current texture binding.

By simply notifying the user that they are responsible for restoring their own texture bindings after using some SFML feature, we eliminate an expensive glGet.

After looking at the git commits for sf::TextureSaver, I realize that it was added again recently (yes, I already removed it before) to make the texture cache optimization work. If I remove it, the cache can no longer be used, and the result is worse performances when drawing a lot of entities that use the same texture.

@luiscubal

Are the performance improvements of caches greater than the penalties of glGet?

Come to think of it, are there sf::Texture functions called very frequently (at least a few times per frame) that depend on glGet to work?

@MarioLiebisch
Member

Had a look at the positions where sf::TextureSaver is used: Doesn't look that serious, especially considering it's mostly for transfer heavy stuff anyway (copying parts of textures, etc.). The only ones being more common to me would be sf::Texture::setSmooth() and sf::setRepeated(), which is just something you shouldn't change over and over again. How does the performance here compare to using a second (or third) texture? Would be worse I assume?

@LaurentGomila
Member

I guess that the only place where it can be a problem is Texture::update. It's true that it is a heavy operation, but triggering a glGet in the middle of drawing is heavy too.

@retep998
retep998 commented Dec 7, 2012

Well, I decided to do some actual benchmarking, and here are some results.
Using 5 textures with a resolution of 90x60.

#include <SFML/Graphics.hpp>
#include <SFML/OpenGL.hpp>
#include <iostream>
using namespace std;
const bool b[2] = {true, false};
GLuint load(string name) {
    sf::Texture * t = new sf::Texture();
    t->loadFromFile(name);
    t->bind();
    GLint tex;
    glGetIntegerv(GL_TEXTURE_BINDING_2D, &tex);
    return tex;
}
int main() {
    sf::Window win(sf::VideoMode(90, 60), "Benchmark", sf::Style::Close);
    GLuint tex[5];
    tex[0] = load("bsc.0.png");
    tex[1] = load("bsc.1.png");
    tex[2] = load("bsc.2.png");
    tex[3] = load("bsc.3.png");
    tex[4] = load("bsc.4.png");
    glClearColor(0, 0, 0, 0);
    glOrtho(0, 1, 1, 0, -1, 1);
    glEnable(GL_TEXTURE_2D);
    for (bool random : b) for (bool glget : b) {
        clock_t c1 = clock();
        for (size_t i = 0x10; i; --i) {
            win.display();
            sf::Event e;
            while (win.pollEvent(e));
            glClear(GL_COLOR_BUFFER_BIT);
            for (size_t j = 0x10000; j; --j) {
                GLuint t;
                if (random) t = tex[rand() % 5];
                else t = tex[0];
                if (glget) {
                    GLint g;
                    glGetIntegerv(GL_TEXTURE_BINDING_2D, &g);
                    if (g != t) glBindTexture(GL_TEXTURE_2D, t);
                } else if (random) glBindTexture(GL_TEXTURE_2D, t);
                glBegin(GL_QUADS);
                glTexCoord2i(0, 0);
                glVertex2i(0, 0);
                glTexCoord2i(1, 0);
                glVertex2i(1, 0);
                glTexCoord2i(1, 1);
                glVertex2i(1, 1);
                glTexCoord2i(0, 1);
                glVertex2i(0, 1);
                glEnd();
            }
        }
        clock_t c2 = clock();
        cout << random << ' ' << glget << ' ' << c2 - c1 << endl;
    }
}

Here are my results using an ATI Radeon 3200 and Phenom II x4 805.

1 1 8824
1 0 9617
0 1 7164
0 0 8541

If these benchmarks are correct, then using glGet is somehow improving performance for me. Even for the non-random test where I don't rebind the texture anyway, glGet still makes it faster somehow. F*** logic.

@MarioLiebisch
Member

The difference really depends on your system it seems.

Here's what I get with my Core 2 Quad 3 GHz/Geforce GTX 680:

1 1 1080
1 0 894
0 1 317
0 0 223

So glGet() slowing it down (as expected). Maybe something odd with your drivers for whatever reason?

Also enabled Nvidia's "Threaded Optimization" to do another run (I always keep it off due to performance issues with SFML):

1 1 3746
1 0 851
0 1 2679
0 0 286

From #sfml:

Intel Core i7-3610QM / Intel HD 4000

1 1 3070
1 0 3450
0 1 290
0 0 280
@LaurentGomila
Member

Since the results are contradictory, and it's required for the cache system to work, I don't consider changing it.

I close this issue, but don't hesitate to post new messages if you have interesting things to say.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment