15
\$\begingroup\$

I'm working on a Virtual Texturing library for mobile devices based on OpenGL-ES.

This is the PageIndirectionTable, one of the library components I would like to have some feedback on:

vt_page_indirection_table.hpp:

#ifndef VTLIB_VT_PAGE_INDIRECTION_TABLE_HPP
#define VTLIB_VT_PAGE_INDIRECTION_TABLE_HPP

namespace vt
{
    // ======================================================
    // PageIndirectionTable:
    // ======================================================

    class PageIndirectionTable
    {
    public:

        // Default constructor initializes the table texture.
        // Might throw and exception if initialization fails.
        PageIndirectionTable(const int * vtPagesX, const int * vtPagesY, int vtNumLevels);

        // Frees the OpenGL texture handle.
        virtual ~PageIndirectionTable();

        // Bind the texture as current OpenGL state.
        void bind(int texUnit = 0) const;

        // Draws the indirection texture as a screen-space quadrilateral for debug visualization.
        // Manually binding the texture is not necessary. 'overlayScale' controls the scale of the overlay quad. From 0 to 1.
        void visualizeIndirectionTexture(const float overlayScale[2]) const;

        // Update the indirection table texture. This is called whenever the page cache changes.
        // Array size must be 'PageTable::TotalTablePages'. Must bind first with PageIndirectionTable::bind().
        virtual void updateIndirectionTexture(const struct CacheEntry * const pages) = 0;

        // Write every mip-level of the indirection table to image files.
        // The file name/path should not include an extension. Each level will be named ( pathname + "_level_number" ).
        virtual bool writeIndirectionTextureToFile(const std::string & pathname, bool recolor) const = 0;

    protected:

        // OpenGL texture handle:
        GLuint indirectionTextureId;

        // Num mip-levels in the Virtual Texture and per-level page counts:
        int numLevels;
        std::array<int, MaxVTMipLevels> numPagesX;
        std::array<int, MaxVTMipLevels> numPagesY;
    };

    using PageIndirectionTablePtr = std::shared_ptr<PageIndirectionTable>;

    // ======================================================
    // PageIndirectionTableRgba8888:
    // ======================================================

    // Uses a RGBA 8:8:8:8 texture to store the page indirection table.
    class PageIndirectionTableRgba8888 final
        : public PageIndirectionTable, public NonCopyable
    {
    public:

        PageIndirectionTableRgba8888(const int * vtPagesX, const int * vtPagesY, int vtNumLevels);

        void updateIndirectionTexture(const struct CacheEntry * const pages) override;
        bool writeIndirectionTextureToFile(const std::string & pathname, bool recolor) const override;

    private:

        void initTexture();

        // Size of a RGBA 8:8:8:8 pixel.
        struct TableEntry
        {
            uint8_t cachePageX; // R
            uint8_t cachePageY; // G
            uint8_t scaleHigh;  // B
            uint8_t scaleLow;   // A
        };
        static_assert(sizeof(TableEntry) == 4, "Expected 4 bytes size!");

    private:

        int totalTableEntries;

        // Pointers to the indirection texture levels of 'tableEntryPool':
        std::array<TableEntry *, MaxVTMipLevels> tableLevels;

        // Data store. 'tableLevels' are pointers to this array.
        // This allows us to perform a single memory allocation.
        std::unique_ptr<TableEntry[]> tableEntryPool;
    };

    // ======================================================
    // PageIndirectionTableRgb565:
    // ======================================================

    // Uses a RGB 5:6:5 texture to store the page indirection table.
    class PageIndirectionTableRgb565 final
        : public PageIndirectionTable, public NonCopyable
    {
    public:

        PageIndirectionTableRgb565(const int * vtPagesX, const int * vtPagesY, int vtNumLevels);

        void updateIndirectionTexture(const struct CacheEntry * const pages) override;
        bool writeIndirectionTextureToFile(const std::string & pathname, bool recolor) const override;

    private:

        void initTexture();

        // Size of a RGB 5:6:5 pixel. Use bit shifting to manipulate the data.
        using TableEntry = uint16_t;
        static_assert(sizeof(TableEntry) == 2, "Expected 2 bytes size!");

    private:

        int log2VirtPagesWide;
        int totalTableEntries;

        // Pointers to the indirection texture levels of 'tableEntryPool':
        std::array<TableEntry *, MaxVTMipLevels> tableLevels;

        // Data store. 'tableLevels' are pointers to this array.
        // This allows us to perform a single memory allocation.
        std::unique_ptr<TableEntry[]> tableEntryPool;
    };

    // ======================================================
    // Factory function. Creates based on startup param.
    // ======================================================

    // Creates a PageIndirectionTable instance based on the
    // global indirection table format configuration parameter.
    PageIndirectionTablePtr createIndirectionTable(const int * vtPagesX, const int * vtPagesY, int vtNumLevels);

} // namespace vt {}

#endif // VTLIB_VT_PAGE_INDIRECTION_TABLE_HPP

vt_page_indirection_table.cpp:

namespace vt
{

// Filtering is fixed for the indirection tables.
static constexpr GLenum indirectionTexMinFilter  = GL_NEAREST_MIPMAP_NEAREST;
static constexpr GLenum indirectionTexMagFilter  = GL_NEAREST;
static constexpr GLenum indirectionTexAddressing = GL_REPEAT;

// ======================================================
// PageIndirectionTable:
// ======================================================

PageIndirectionTable::PageIndirectionTable(const int * vtPagesX, const int * vtPagesY, const int vtNumLevels)
    : indirectionTextureId(0)
    , numLevels(vtNumLevels)
{
    assert(numLevels > 0 && numLevels <= MaxVTMipLevels);

    clearArray(numPagesX);
    clearArray(numPagesY);
    for (int l = 0; l < numLevels; ++l)
    {
        assert(vtPagesX[l] > 0);
        assert(vtPagesY[l] > 0);
        numPagesX[l] = vtPagesX[l];
        numPagesY[l] = vtPagesY[l];
    }
}

PageIndirectionTable::~PageIndirectionTable()
{
    gl::delete2DTexture(indirectionTextureId);
}

void PageIndirectionTable::bind(const int texUnit) const
{
    gl::use2DTexture(indirectionTextureId, texUnit);
}

void PageIndirectionTable::visualizeIndirectionTexture(const float overlayScale[2]) const
{
    gl::useShaderProgram(getGlobalShaders().drawIndirectionTable.programId);
    gl::setShaderProgramUniform(getGlobalShaders().drawIndirectionTable.unifNdcQuadScale, overlayScale, 2);

    // Draw a quad with the texture applied to it:
    gl::use2DTexture(indirectionTextureId);
    gl::drawNdcQuadrilateral();
    gl::use2DTexture(0);

    gl::useShaderProgram(0);
}

// ======================================================
// PageIndirectionTableRgba8888:
// ======================================================

PageIndirectionTableRgba8888::PageIndirectionTableRgba8888(const int * vtPagesX, const int * vtPagesY, const int vtNumLevels)
    : PageIndirectionTable(vtPagesX, vtPagesY, vtNumLevels)
    , totalTableEntries(0)
{
    clearArray(tableLevels);
    initTexture();

    vtLogComment("New PageIndirectionTable RGBA-8:8:8:8 instance created...");
}

void PageIndirectionTableRgba8888::initTexture()
{
    assert(indirectionTextureId == 0 && "Duplicate initialization!");
    vtLogComment("Initializing page indirection texture with " << numLevels << " levels...");

    // Count total texture size, including all mip-levels:
    totalTableEntries = 0;
    for (int l = 0; l < numLevels; ++l)
    {
        totalTableEntries += numPagesX[l] * numPagesY[l];
    }

    tableEntryPool.reset(new TableEntry[totalTableEntries]);

    // Set up the pointers:
    totalTableEntries = 0;
    for (int l = 0; l < numLevels; ++l)
    {
        tableLevels[l] = tableEntryPool.get() + totalTableEntries;
        totalTableEntries += numPagesX[l] * numPagesY[l]; // Move to the next level
    }

    glGenTextures(1, &indirectionTextureId);
    if (indirectionTextureId == 0)
    {
        vtFatalError("Failed to generate a non-zero GL texture id for the page indirection texture!");
    }

    gl::use2DTexture(indirectionTextureId);

    // Create/set all the mip-levels:
    for (int l = 0; l < numLevels; ++l)
    {
        assert(tableLevels[l] != nullptr);

        // Default initialize the entries:
        for (int e = 0; e < numPagesX[l] * numPagesY[l]; ++e)
        {
            TableEntry & entry = tableLevels[l][e];
            entry.cachePageX = 0;
            entry.cachePageY = 0;

            const uint16_t scale = (numPagesX[0] * 16) >> l;
            entry.scaleHigh = (scale & 0xFF);
            entry.scaleLow  = (scale >> 8);
        }

        glTexImage2D(GL_TEXTURE_2D, l, GL_RGBA, numPagesX[l], numPagesY[l], 0, GL_RGBA, GL_UNSIGNED_BYTE, tableLevels[l]);
        vtLogComment("Allocated indirection tex level #" << l << ". Size: " << numPagesX[l] << "x" << numPagesY[l] << " pixels.");

        #if VT_EXTRA_GL_ERROR_CHECKING
        gl::checkGLErrors(__FILE__, __LINE__);
        #endif // VT_EXTRA_GL_ERROR_CHECKING
    }

    // iOS specific: Set max level (would probably have to use glGenerateMipmap() otherwise...)
    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAX_LEVEL_APPLE, numLevels - 1);

    // Set addressing mode:
    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, indirectionTexAddressing);
    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, indirectionTexAddressing);

    // Set filtering:
    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, indirectionTexMinFilter);
    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, indirectionTexMagFilter);

    gl::use2DTexture(0);
    gl::checkGLErrors(__FILE__, __LINE__);

    vtLogComment("Page indirection texture #" << indirectionTextureId << " created. Num entries: "
            << totalTableEntries << ". " << numLevels << " levels.");
}

void PageIndirectionTableRgba8888::updateIndirectionTexture(const CacheEntry * __restrict const pages)
{
    assert(pages != nullptr);

    // Texture must be already bound!
    // GL_UNPACK_ALIGNMENT should ideally be set to 4.
    assert(indirectionTextureId == gl::getCurrent2DTexture());

    // Assemble the indirection table, one mip-level at a time,
    // stating from the lowest resolution one:
    for (int l = (numLevels - 1); l >= 0; --l)
    {
        // Write all pages in a level:
        for (int p = 0; p < PageCacheMgr::TotalCachePages; ++p)
        {
            const CacheEntry & cacheEntry = pages[p];
            if ((pageIdExtractMipLevel(cacheEntry.pageId) != l) || (cacheEntry.pageId == InvalidPageId))
            {
                continue;
            }

            const int x = pageIdExtractPageX(cacheEntry.pageId);
            const int y = pageIdExtractPageY(cacheEntry.pageId);
            const int index = (x + y * numPagesX[l]);

            assert(index >= 0);
            assert(index < (numPagesX[l] * numPagesY[l]));
            TableEntry & entry = tableLevels[l][index];

            entry.cachePageX = cacheEntry.cacheCoord.x;
            entry.cachePageY = cacheEntry.cacheCoord.y;

            const uint16_t scale = (numPagesX[0] * 16) >> l;
            entry.scaleHigh = (scale & 0xFF);
            entry.scaleLow  = (scale >> 8);
        }

        // Upsample for next level:
        if (l != 0)
        {
            uint32_t * __restrict src  = reinterpret_cast<uint32_t *>(tableLevels[l]);
            uint32_t * __restrict dest = reinterpret_cast<uint32_t *>(tableLevels[l - 1]);

            const int srcW  = numPagesX[l];
            const int destW = numPagesX[l - 1];
            const int destH = numPagesY[l - 1];

            for (int y = 0; y < destH; ++y)
            {
                for (int x = 0; x < destW; ++x)
                {
                    dest[x + y * destW] = src[(x >> 1) + (y >> 1) * srcW];
                }
            }
        }
    }

    for (int l = 0; l < numLevels; ++l)
    {
        glTexImage2D(GL_TEXTURE_2D, l, GL_RGBA, numPagesX[l], numPagesY[l], 0, GL_RGBA, GL_UNSIGNED_BYTE, tableLevels[l]);

        #if VT_EXTRA_GL_ERROR_CHECKING
        gl::checkGLErrors(__FILE__, __LINE__);
        #endif // VT_EXTRA_GL_ERROR_CHECKING
    }
}

bool PageIndirectionTableRgba8888::writeIndirectionTextureToFile(const std::string & pathname, const bool recolor) const
{
    int levelsWritten = 0;
    std::string levelNameStr, result;

    for (int l = 0; l < numLevels; ++l)
    {
        Pixel4b * __restrict pixels = reinterpret_cast<Pixel4b *>(tableLevels[l]);

        // Reverse the bits in the image pixels to make them stand out.
        // Most of the pixels would be very dark otherwise.
        if (recolor)
        {
            const size_t numPixels = numPagesX[l] * numPagesY[l];
            for (size_t p = 0; p < numPixels; ++p)
            {
                Pixel4b & pix = pixels[p];
                pix.r = reverseByte(pix.r);
                pix.g = reverseByte(pix.g);
                // Mix alpha (the texture index) it with blue:
                uint8_t b = reverseByte(pix.b);
                uint8_t a = reverseByte(pix.a);
                pix.b = clampByte(a + b);
                pix.a = 0xFF;
            }
        }

        levelNameStr = pathname + "_" + std::to_string(l) + ".tga";
        if (tool::writeTgaImage(levelNameStr, numPagesX[l], numPagesY[l], 4, reinterpret_cast<uint8_t *>(pixels), true, &result))
        {
            vtLogComment(result);
            levelsWritten++;
        }
    }

    return levelsWritten == numLevels;
}

// ======================================================
// PageIndirectionTableRgb565:
// ======================================================

PageIndirectionTableRgb565::PageIndirectionTableRgb565(const int * vtPagesX, const int * vtPagesY, const int vtNumLevels)
    : PageIndirectionTable(vtPagesX, vtPagesY, vtNumLevels)
    , log2VirtPagesWide(static_cast<int>(std::log2(vtPagesX[0])))
    , totalTableEntries(0)
{
    clearArray(tableLevels);
    initTexture();

    vtLogComment("New PageIndirectionTable RGB-5:6:5 instance created. log2VirtPagesWide = " << log2VirtPagesWide);
}

void PageIndirectionTableRgb565::initTexture()
{
    assert(indirectionTextureId == 0 && "Duplicate initialization!");
    vtLogComment("Initializing page indirection texture with " << numLevels << " levels...");

    // Count total texture size, including all mip-levels:
    totalTableEntries = 0;
    for (int l = 0; l < numLevels; ++l)
    {
        totalTableEntries += numPagesX[l] * numPagesY[l];
    }

    tableEntryPool.reset(new TableEntry[totalTableEntries]);

    // Set up the pointers:
    totalTableEntries = 0;
    for (int l = 0; l < numLevels; ++l)
    {
        tableLevels[l] = tableEntryPool.get() + totalTableEntries;
        totalTableEntries += numPagesX[l] * numPagesY[l]; // Move to the next level
    }

    glGenTextures(1, &indirectionTextureId);
    if (indirectionTextureId == 0)
    {
        vtFatalError("Failed to generate a non-zero GL texture id for the page indirection texture!");
    }

    gl::use2DTexture(indirectionTextureId);

    // Create/set all the mip-levels:
    for (int l = 0; l < numLevels; ++l)
    {
        assert(tableLevels[l] != nullptr);

        // Default initialize the entries:
        for (int e = 0; e < numPagesX[l] * numPagesY[l]; ++e)
        {
            TableEntry & entry = tableLevels[l][e];

            entry = 0;
            entry = ((log2VirtPagesWide - l) << 5);
        }

        glTexImage2D(GL_TEXTURE_2D, l, GL_RGB, numPagesX[l], numPagesY[l], 0, GL_RGB, GL_UNSIGNED_SHORT_5_6_5, tableLevels[l]);
        vtLogComment("Allocated indirection tex level #" << l << ". Size: " << numPagesX[l] << "x" << numPagesY[l] << " pixels.");

        #if VT_EXTRA_GL_ERROR_CHECKING
        gl::checkGLErrors(__FILE__, __LINE__);
        #endif // VT_EXTRA_GL_ERROR_CHECKING
    }

    // iOS specific: Set max level (would probably have to use glGenerateMipmap() otherwise...)
    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAX_LEVEL_APPLE, numLevels - 1);

    // Set addressing mode:
    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, indirectionTexAddressing);
    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, indirectionTexAddressing);

    // Set filtering:
    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, indirectionTexMinFilter);
    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, indirectionTexMagFilter);

    gl::use2DTexture(0);
    gl::checkGLErrors(__FILE__, __LINE__);

    vtLogComment("Page indirection texture #" << indirectionTextureId << " created. Num entries: "
            << totalTableEntries << ". " << numLevels << " levels.");
}

void PageIndirectionTableRgb565::updateIndirectionTexture(const CacheEntry * __restrict const pages)
{
    assert(pages != nullptr);

    // Texture must be already bound!
    // GL_UNPACK_ALIGNMENT should ideally be set to 4.
    assert(indirectionTextureId == gl::getCurrent2DTexture());

    // Assemble the indirection table, one mip-level at a time,
    // stating from the lowest resolution one:
    for (int l = (numLevels - 1); l >= 0; --l)
    {
        // Write all pages in a level:
        for (int p = 0; p < PageCacheMgr::TotalCachePages; ++p)
        {
            const CacheEntry & cacheEntry = pages[p];
            if ((pageIdExtractMipLevel(cacheEntry.pageId) != l) || (cacheEntry.pageId == InvalidPageId))
            {
                continue;
            }

            const int x = pageIdExtractPageX(cacheEntry.pageId);
            const int y = pageIdExtractPageY(cacheEntry.pageId);
            const int index = (x + y * numPagesX[l]);

            assert(index >= 0);
            assert(index < (numPagesX[l] * numPagesY[l]));
            TableEntry & entry = tableLevels[l][index];

            entry = ((cacheEntry.cacheCoord.x * 32 / PageTable::TableSizeInPages) << 11) |
                ((log2VirtPagesWide - l) << 5) | (cacheEntry.cacheCoord.y * 32 / PageTable::TableSizeInPages);
        }

        // Upsample for next level:
        if (l != 0)
        {
            TableEntry * __restrict src  = tableLevels[l];
            TableEntry * __restrict dest = tableLevels[l - 1];

            const int srcW  = numPagesX[l];
            const int destW = numPagesX[l - 1];
            const int destH = numPagesY[l - 1];

            for (int y = 0; y < destH; ++y)
            {
                for (int x = 0; x < destW; ++x)
                {
                    dest[x + y * destW] = src[(x >> 1) + (y >> 1) * srcW];
                }
            }
        }
    }

    for (int l = 0; l < numLevels; ++l)
    {
        glTexImage2D(GL_TEXTURE_2D, l, GL_RGB, numPagesX[l], numPagesY[l], 0, GL_RGB, GL_UNSIGNED_SHORT_5_6_5, tableLevels[l]);

        #if VT_EXTRA_GL_ERROR_CHECKING
        gl::checkGLErrors(__FILE__, __LINE__);
        #endif // VT_EXTRA_GL_ERROR_CHECKING
    }
}

bool PageIndirectionTableRgb565::writeIndirectionTextureToFile(const std::string & pathname, const bool recolor) const
{
    int levelsWritten = 0;
    std::string levelNameStr, result;
    std::vector<Pixel4b> tempImage(numPagesX[0] * numPagesY[0]);

    // Convert the 5:6:5 texture to 8bits RGBA first, to make writing the image simpler.
    auto makeRGBA = [&tempImage, recolor](const TableEntry * data, size_t numPixels) -> Pixel4b *
    {
        for (size_t p = 0; p < numPixels; ++p)
        {
            TableEntry src  = data[p];
            Pixel4b &  dest = tempImage[p];

            // Unpack RGB 565 to RGBA fixing alpha to 255:
            dest.r = ((src & 0x7800) >> 11);
            dest.g = ((src & 0x07E0) >>  5);
            dest.b = ((src & 0x001F) >>  0);
            dest.a = 0xFF;

            // Reverse the bits in the image pixels to make them stand out.
            // Most of the pixels would be very dark otherwise.
            if (recolor)
            {
                dest.r = reverseByte(dest.r);
                dest.g = reverseByte(dest.g);
                dest.b = reverseByte(dest.b);
            }
        }
        return tempImage.data();
    };

    for (int l = 0; l < numLevels; ++l)
    {
        const size_t numPixels = numPagesX[l] * numPagesY[l];
        const Pixel4b * pixels = makeRGBA(tableLevels[l], numPixels);

        levelNameStr = pathname + "_" + std::to_string(l) + ".tga";
        if (tool::writeTgaImage(levelNameStr, numPagesX[l], numPagesY[l], 4, reinterpret_cast<const uint8_t *>(pixels), true, &result))
        {
            vtLogComment(result);
            levelsWritten++;
        }
    }

    return levelsWritten == numLevels;
}

// ======================================================
// createIndirectionTable():
// ======================================================

PageIndirectionTablePtr createIndirectionTable(const int * vtPagesX, const int * vtPagesY, const int vtNumLevels)
{
    extern IndirectionTableFormat getIndirectionTableFormat() noexcept;

    switch (getIndirectionTableFormat())
    {
    case IndirectionTableFormat::Rgb565 :
        return std::make_shared<PageIndirectionTableRgb565>(vtPagesX, vtPagesY, vtNumLevels);

    case IndirectionTableFormat::Rgba8888 :
        return std::make_shared<PageIndirectionTableRgba8888>(vtPagesX, vtPagesY, vtNumLevels);

    default :
        vtFatalError("Invalid IndirectionTableFormat!");
    } // switch (getIndirectionTableFormat())
}

} // namespace vt {}

My main concern with it is that both implementations of PageIndirectionTable share a fair amount of similar code (too much copy-pasting), so I'm unsure of whether to factor out some code into helper methods, thus increasing complexity, or leaving it as it is and avoiding abstracting it further. Any suggestions on that?

Other suggestions and critique are always welcome.

Improve this question
\$\endgroup\$
2
  • \$\begingroup\$ Did you mean to put two private sections in each of the child classes? \$\endgroup\$
    – flakes
    Dec 4, 2014 at 14:53
  • \$\begingroup\$ @Calpratt, yes. Did it to separate the variables from the rest... Arguabilly silly, though. \$\endgroup\$
    – glampert
    Dec 4, 2014 at 17:18

1 Answer 1

Reset to default
4
+50
\$\begingroup\$

Generally, the code looks very well-written. Good job!

Small improvements:

Nitpicky improvements:

  • Get in the habit of writing for (int i = 0; end != i; ++i) instead of for (int i = 0; i != end; ++i) when end is const or an r-value. This way, you will get a compiler error if you accidentially write end = i in the loop (assignment instead of test). Same goes for <=, >=, and especially ==. I'd even prefer that style just for < and > just to stay consistent. This goes for all your raw for loops.

Also, have you considered static polymorphism via the CRTP? I can't say whether this applies without seeing the client code. If it does apply, then you can gain some performance by dropping the vtable. However, the real world effects are probably negligible.

Improve this answer
\$\endgroup\$
5
  • \$\begingroup\$ Thank you, good points! I shall read the discussion about the non-copyable pattern. \$\endgroup\$
    – glampert
    Dec 28, 2014 at 16:23
  • \$\begingroup\$ BTW, correct me if I'm wrong, but AFAIK, the only advantage of CRTP over normal polymorphism is if you know the implementation type at compile time, no? I my case, I need to decide the implementation based on a config parameter that is only known at runtime. \$\endgroup\$
    – glampert
    Dec 28, 2014 at 16:36
  • \$\begingroup\$ @glampert You're right about static vs dynamic polymorphism. In your case, dynamic polymorphism is indeed needed. Is the gl namespace your own abstraction? It seems like you could benefit from a more RAII-like abstraction around basic textures. Shameless plug: Such as my own basic_texture. As for the abstraction of the PageIndirectionTable itself, I have some suggestions that I'll share in a later edit (if I get the time). \$\endgroup\$
    – Frederik Aalund
    Dec 28, 2014 at 22:18
  • \$\begingroup\$ Yes, I added a few helpers in that gl namespace to group a few common calls, like glBindTexture and glActiveTexUnit under a single function, and also to facilitate the addition of state caching if needed. I'm reluctant about adding RAII & classes at that level because that is a much lower level API and its users are themselves full blown classes. It is not an API meant to be used outside the library internals... \$\endgroup\$
    – glampert
    Dec 28, 2014 at 22:36
  • 1
    \$\begingroup\$ @glampert That is very understandable. If performance is an absolutely critical metric that can not be compromised for any abstraction then I would also accept your current code during a review. However, for something like this where you are actually allocating just a single texture, I would strongly favor a small bit of RAII to handle the acquisition/release logic. With even the most basic inlining, there won't even be any overhead. It's also safer since you will never miss a glDeleteTextures call (and won't do double deletes either). \$\endgroup\$
    – Frederik Aalund
    Dec 29, 2014 at 1:18

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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