memononen / nanovg

Antialiased 2D vector drawing library on top of OpenGL for UI and visualizations.
zlib License
5.11k stars 766 forks source link

SUGGESTION: Performance hit from storing textures in an array. (VCVRack) #451

Open Chaircrusher opened 6 years ago

Chaircrusher commented 6 years ago

I did some profiling of the VCVRack project with an eye towards improving performance. The immediate low hanging fruit was glnvg__findTexture. This turned up at the top of the CPU usage for VCVRack in profiling.

The problem is that findTexture uses a sequential search through the texture array to find a texture based on ID. VCVRack uses a large number of textures, and glnvg__findTexture was being called hundreds (if not thousands) of times on every GUI [draw.]([url](url

))

As a proof of concept, I used a C++ STL map to store textures; this took findTexture off the top of the CPU usage list and buried it far down the list.

Since nanovg is a pure C library, you probably don't want to pollute it with C++ code. I have a possible suggestion as to how to fix this in a pure C way:

Instead of using an incrementing counter for the ID of textures, have the texture ID be the index into the textures array. When allocating, search for an empty texture in the array, and if there is none, do the realloc thing to allocate a new one.

Then findTexture becomes trivial, since if you have the textureID you simply dereference the array with it.

Deleting a texture would work exactly the same as it does now.

The only problem I see is that a program that mistakenly keeps the IDs around for deleted textures. In that case the wrong texture would be returned instead of NULL.

The alternative would be to use a data structure in C with better search characteristics, and preserve the same method of generating texture IDs.

texture.txt is a diff for my proof of concept. texture.txt

lieff commented 6 years ago

Use "```" to format block of code.

memononen commented 6 years ago

@Chaircrusher I think your C like solution sounds feasible. And deleted textures can be found using "salted" id. Something like this (untested):

struct GLNVGtexture {
    int id;
    GLuint tex;
    int width, height;
    int type;
    int flags;
    int salt;
};

static GLNVGtexture* glnvg__allocTexture(GLNVGcontext* gl)
{
    GLNVGtexture* tex = NULL;
    int i, idx = -1, salt;

    for (i = 0; i < gl->ntextures; i++) {
        if (gl->textures[i].id == 0) {
            idx = i;
            break;
        }
    }
    if (tex == NULL) {
        if (gl->ntextures+1 > gl->ctextures) {
            GLNVGtexture* textures;
            int ctextures = glnvg__maxi(gl->ntextures+1, 4) +  gl->ctextures/2; // 1.5x Overallocate
            textures = (GLNVGtexture*)realloc(gl->textures, sizeof(GLNVGtexture)*ctextures);
            if (textures == NULL) return NULL;
            gl->textures = textures;
            gl->ctextures = ctextures;
        }
        idx = gl->ntextures++;
    }

    tex = &gl->textures[idx];
    salt = tex->salt;   // keep salt
    memset(tex, 0, sizeof(*tex));
    tex->salt = salt; 
    tex->id = (idx & 0xffff) | (salt << 16);

    return tex;
}

static GLNVGtexture* glnvg__findTexture(GLNVGcontext* gl, int id)
{
    int idx = id & 0xffff, salt = (id >> 16) & 0xffff;
    // Check that the texture index is valid
    if (idx < 0 || idx >= gl->ntextures)
        return NULL;
    // Check that we're not trying to access deleted texture.
    if (gl->textures[idx].salt != salt)
        return NULL;
    return &gl->textures[idx];
}

static int glnvg__deleteTexture(GLNVGcontext* gl, int id)
{
    int idx = id & 0xffff, salt = (id >> 16) & 0xffff;
    GLNVGtexture* tex = NULL;
    // Check that the texture index is valid
    if (idx < 0 || idx >= gl->ntextures)
        return 0;
    // Check that we're not trying to access deleted texture.
    if (gl->textures[idx].salt != salt)
        return 0;

    // Clear texture
    tex = &gl->textures[idx];
    if (tex->tex != 0 && (tex->flags & NVG_IMAGE_NODELETE) == 0)
        glDeleteTextures(1, &tex->tex);
    memset(tex, 0, sizeof(*tex));
    // Increment salt, so that we can recognise deleted textures.
    tex->salt = (salt + 1) & 0xffff;

    return 1;
}

Salt is basically the version of the texture in use. We increment it in delete so that we can recognize if old version is being used.

Can you test if the above fixes your bottleneck? If it does, would you mind making a PR?