1
\$\begingroup\$

For a game that updates a board every frame I am calculating the next arrangement of board, updating pixel array data and render board as 2D texture to quad the size of the screen using OpenGL. I use:

glTexImage2d(GL_TEXTURE_2D,0,GL_RGBA,tex_width,tex_height,0,GL_BGRA,GL_UNSIGNED_INT_8_8_8_8_REV, &pixel_data.front()) 

to initialize the texture object and

glTexSubImage2d(GL_TEXTURE_2D,0,0,0,tex_width,tex_height, BGRA,GL_UNSIGNED_INT_8_8_8_8_REV, &pixel_data.front()) 

to update the texture.

The slowest part of the code is update_pixels() (below) Taking 256x128 elements input array and creating four component pixel array based on that. This function is very slow(45ms) and it seems it should be done differently. What would be the proper way to implement this function?

Update pixel data per frame:

void  update_pixels() {
//update pixels
//std::vector <unsigned char> pixel_data
float state = 0;
size_t it = 0;

for (size_t y = 0; y < board.getWorldHeight(); ++y) {
    for (size_t x = 0; x < board.getWorldWidth(); ++x) {
        state = board.get_state(y * golf_board.getWorldWidth() + x);

        pixel_data[y * board.getWorldWidth() + x + it ] = static_cast<unsigned char>(state * 255);
        ++it;
        pixel_data[y * board.getWorldWidth() + x + it] = static_cast<unsigned char>(state * 255);
        ++it;
        pixel_data[y * board.getWorldWidth() + x + it] = static_cast<unsigned char>(state * 255);
        ++it;
        pixel_data[y * board.getWorldWidth() + x + it] = static_cast<unsigned char>(1 * 255);

    }

}
return;
}

Time measurement:

void render(){
render_frame() //glDrawElements() for quad and scale uniform update takes 0.2ms
generate_states() //new states for board takes 1.3ms
auto t1 = std::chrono::high_resolution_clock::now();
update_pixels(); // create new pixel array, takes 32 ms
auto t2 - std::chrono::high_resolution_clock::now();
float update_time = std::chrono::duration_cast<std::chrono::microseconds>(t2-t1).count() /1000.0f;
//cout <<updae_time
glfwSwapBuffers() //from update pixels to swap takes 13ms
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Should it be set to zero in the loop body? \$\endgroup\$ – Kerndog73 Aug 30 '17 at 11:29
  • \$\begingroup\$ Or y * board.getWorldWidth() + x removed from the subscript of pixel_data? \$\endgroup\$ – Kerndog73 Aug 30 '17 at 11:31
  • 1
    \$\begingroup\$ You could also calculate things once, store them, and use them multiple times to improve performance just a tad. \$\endgroup\$ – Kerndog73 Aug 30 '17 at 11:33
  • 1
    \$\begingroup\$ y * board.getWorldWidth() + x is calculated 5 times in the loop body when it only needs to be calculated once. board.getWorldWidth() is called 196608 times in the whole function but only needs to be called once. board.getWorldHeight() is called 128 times when it only needs to be called once. That's what I mean by "calculate things once and use them multiple times" \$\endgroup\$ – Kerndog73 Aug 30 '17 at 22:03
  • 1
    \$\begingroup\$ I find it hard to believe that this function is running for 45ms. Are you certain that this function is taking all that time and not something else? \$\endgroup\$ – Kerndog73 Aug 30 '17 at 22:11
0
\$\begingroup\$

One way to speed it up would be to access the arrays directly rather than doing a monstrous number of indexed look-ups into them when you're accessing the sequentially. It could look something like this:

unsigned char* nextPixel = &pixel_data [ 0 ];
const int boardHeight = board.getWorldHeight();
const int boardWidth = board.getWorldWidth();
const int numPixels = boardHeight * boardWidth;
for (int i = 0; i < numPixels; ++i)
{
    const float state = board.get_state(i);
    const unsigned char channel = static_cast<unsigned char>(state * 255.0);

    // red
    *nextPixel = channel;
    nextPixel++;

    // green
    *nextPixel = channel;
    nextPixel++;

    // blue
    *nextPixel = channel;
    nextPixel++;

    // alpha
    *nextPixel = 255;
    nextPixel++;
}

I also recommend using the appropriate types for your data. An OpenGL texture is not a series of individual unsigned chars. It's a 2 dimensional array of RGBA pixels. You should make a structure for them:

struct RGBAPixel {
    unsigned char red;
    unsigned char green;
    unsigned char blue;
    unsigned char alpha;
};

Then your code wouldn't need the comments I added. But this isn't Code Review, so I'll stop here.

\$\endgroup\$
  • \$\begingroup\$ Using these fixes save 5 ms given the same amount of input data. Still slow for larger boards. \$\endgroup\$ – fredric Sep 1 '17 at 6:35
  • \$\begingroup\$ It occurs to me that my code is still repeating stuff unnecessarily. You could calculate static_cast<unsigned char>(state * 255) once per loop. I'll update that. The other thing you could do is instead of calling board.get_state(i) every iteration, also get the start of the state array and run through it just like with the pixel array. I don't know if you have a method that gives you that address, though. \$\endgroup\$ – user1118321 Sep 1 '17 at 14:47
  • \$\begingroup\$ Oh, and as mentioned in the comments to the original question, you could use a single channel image with no alpha channel. That would reduce your workload in the loop by 75%. \$\endgroup\$ – user1118321 Sep 1 '17 at 14:49
  • 1
    \$\begingroup\$ yes, see the comment by tkausl to first question. the whole update_pixels() function can be avoided, just a little tweak is needed in shader to represent color accurately. \$\endgroup\$ – fredric Sep 1 '17 at 17:24
0
\$\begingroup\$

The first thing to speed up would be to use the smallest texture format for the job.

Since your Alpha is always 1 and all three components RGB are the same value you should use GL_RED (OpenGL 3.0 and later) or GL_LUMINANCE (up to OpenGL 3.0).

This creates a single-component texture so you do not need to copy the value 3 times and add a constant alpha value of 1.0 (255).

Up to OpenGL 3.0:

glTexImage2d(GL_TEXTURE_2D, 0, GL_LUMINANCE, tex_width,tex_height, 0,GL_LUMINANCE, GL_UNSIGNED_BYTE, pixel_data.data());

or OpenGL 3.0 onward:

glTexImage2d(GL_TEXTURE_2D, 0, GL_RED, tex_width,tex_height, 0, GL_RED, GL_UNSIGNED_BYTE, pixel_data.data());

const GLint swizzleMask[] = {GL_RED, GL_RED, GL_RED, GL_ONE};
glTexParameteriv(GL_TEXTURE_2D, GL_TEXTURE_SWIZZLE_RGBA, swizzleMask);

This makes your texture a series of single bytes, one per pixel. Reducing CPU memory bandwidth, CPU<->GPU bus transfer, and GPU memory bandwidth requirements.

The texture is returned by the sampler as {RED, RED, RED, 1} or {LUMINANCE, LUMINANCE, LUMINANCE, 1} (same thing, older OpenGL).

This way only 1 byte per pixel need to be written.

Also reduce the two loops to a single continuous loop of width*height and avoid calculating the loop limit on every iteration:

//GLubyte *pixels = &pixel_data.front();
GLubyte *pixels = pixel_data.data(); // C++11

for (size_t i=0, count=board.getWorldHeight()*board.getWorldWidth(); count--; ) {
    *pixels++ = static_cast<GLubyte>(board.get_state(i++) * 255);
}

The post-decrement/post-increment operators helps modern CPUs do out-of-order execution.

To speed things up further you should also consider storing your board state as fixed points or a copy of it as a bytes to avoid the costly float to integer conversion multiple times every redraw.

If you maintain a copy of the state as bytes whenever modifying your states you can directly update the texture when something change.

And look into avoiding updating the texture when no state have effectively changed if this is a frequent occurrence.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.