Designing a Robust Texture and Framebuffer class for Game Engine

Question

As part of my ongoing development of a 3D game engine based on PyOpenGL, I am trying to do some refactoring to make working with textures and framebuffers as painless as possible. I feel this refactoring is needed so that my texture class can handle all of these (and more!) use-cases seamlessly:

1. Loading in image data from a file
2. Loading in generated pixel data
3. Applying a texture to a sprite/model in my scene
4. Accept data in a variety of color formats.
5. Accept not only color data, but also depth and stencil data.
6. Rendering the framebuffer to a texture for post-processing effects/passes.

I have created a simple demo that illustrates my code. There are three files: texture.py for my texture class (the main concern), framebuffer.py for handling creating framebuffers besides the default screen buffer, and main.py, which tests these two classes without relying on the rest of my engine (just renders a full-screen quad to an off-screen texture). Here is that minimal code that works to cover these uses:

texture.py

import numpy as np
from OpenGL.GL import *
from OpenGL.GL.ARB.texture_float import *
from OpenGL.GL.EXT.framebuffer_object import *
from PIL import Image

_valid_formats = {
    "RGB": GL_RGB, 
    "RGBA": GL_RGBA, 
    "FLOAT": GL_FLOAT,
    "RGB_FLOAT": GL_RGB32F_ARB,
    "RGBA_FLOAT": GL_RGBA32F_ARB,
}

_valid_filters = [GL_NEAREST, GL_LINEAR]

class Texture(object):

    def __init__(self):
        self._id = glGenTextures(1)

    def get_data(self):
        self.bind()
        raw_data = glGetTexImage(GL_TEXTURE_2D, 0, GL_RGBA, GL_UNSIGNED_BYTE)
        image = Image.frombytes("RGBA", (self.width, self.height), raw_data)
        image = image.transpose(Image.FLIP_TOP_BOTTOM)
        data = np.array(image)
        self.unbind()
        return data

    def get_id(self):
        return self._id

    def load_from_url(self, asset_url, linear_filter=True, texture_unit=0):
        asset = Image.open(asset_url)
        data = np.array(asset, dtype=np.uint8).flatten()
        width, height = asset.size
        item_size = len(data)/(width * height)
        image_format = "RGBA" if item_size == 4 else "RGB"
        self.load_from_data(width, height, image_format, data, linear_filter, texture_unit)

    def load_from_data(self, width, height, image_format, data=None, linear_filter=True, texture_unit=0):
        self.width = width
        self.height = height
        self.image_format = image_format
        self.linear_filter = linear_filter
        self.texture_unit = texture_unit
        gl_format = _valid_formats[self.image_format]
        self.bind()
        glTexParameterf(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, _valid_filters[self.linear_filter])
        glTexParameterf(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, _valid_filters[self.linear_filter])
        glTexImage2D(GL_TEXTURE_2D, 0, gl_format, self.width, self.height, 0, gl_format, GL_UNSIGNED_BYTE, data)
        self.unbind()

    def bind(self):
        glActiveTexture(GL_TEXTURE1 + self.texture_unit)
        glBindTexture(GL_TEXTURE_2D, self._id)

    def unbind(self):
        glActiveTexture(GL_TEXTURE0 + self.texture_unit)
        glBindTexture(GL_TEXTURE_2D, 0)
        glActiveTexture(GL_TEXTURE0)

framebuffer.py

import numpy as np
from OpenGL.GL import *
from OpenGL.GL.ARB.texture_float import *
from OpenGL.GL.EXT.framebuffer_object import *
from PIL import Image
from pyorama.graphics.texture import Texture

_valid_attachments = {
    "COLOR_0": GL_COLOR_ATTACHMENT0, 
    "COLOR_1": GL_COLOR_ATTACHMENT1, 
    "COLOR_2": GL_COLOR_ATTACHMENT2, 
    "COLOR_3": GL_COLOR_ATTACHMENT3, 
    "COLOR_4": GL_COLOR_ATTACHMENT4, 
    "COLOR_5": GL_COLOR_ATTACHMENT5, 
    "COLOR_6": GL_COLOR_ATTACHMENT6, 
    "COLOR_7": GL_COLOR_ATTACHMENT7, 
    "COLOR_8": GL_COLOR_ATTACHMENT8, 
    "COLOR_9": GL_COLOR_ATTACHMENT9, 
    "COLOR_10": GL_COLOR_ATTACHMENT10, 
    "COLOR_11": GL_COLOR_ATTACHMENT11, 
    "COLOR_12": GL_COLOR_ATTACHMENT12, 
    "COLOR_13": GL_COLOR_ATTACHMENT13, 
    "COLOR_14": GL_COLOR_ATTACHMENT14, 
    "COLOR_15": GL_COLOR_ATTACHMENT15,
    "DEPTH": GL_DEPTH_ATTACHMENT,
    "STENCIL": GL_STENCIL_ATTACHMENT,
}

class Framebuffer(object):

    def __init__(self):
        self._id = glGenFramebuffersEXT(1)

    def bind(self):
        glBindFramebufferEXT(GL_FRAMEBUFFER_EXT, self._id)

    def attach_texture(self, attachment, texture):
        self.bind()
        glFramebufferTexture2DEXT(GL_FRAMEBUFFER_EXT, _valid_attachments[attachment], GL_TEXTURE_2D, texture.get_id(), 0)
        self.unbind()

    def unbind(self):
        glBindFramebufferEXT(GL_FRAMEBUFFER_EXT, 0)

    def detach_texture(self, texture):
        self.bind()
        glFramebufferTexture2DEXT(GL_FRAMEBUFFER_EXT, GL_DEPTH_ATTACHMENT, GL_TEXTURE_2D, 0, 0)
        self.unbind()

main.py

from pyorama.graphics.texture import Texture
from pyorama.graphics.framebuffer import Framebuffer
import pygame
import sys

width, height = (800, 600)
pygame.init()
pygame.display.set_mode((width, height), pygame.OPENGL | pygame.DOUBLEBUF)
f = Framebuffer()
t = Texture()
t.load_from_data(width, height, "RGBA")
f.attach_texture("COLOR_0", t)

while True:
    for event in pygame.event.get():
        if event.type == pygame.QUIT:
            pygame.quit()
            sys.exit()
    f.bind()
    glClear(GL_COLOR_BUFFER_BIT)
    glClear(GL_DEPTH_BUFFER_BIT)
    glBegin(GL_QUADS)
    glColor(1, 0, 0, 1)
    glVertex(-1, -1)
    glVertex(-1, 1)
    glVertex(1, 1)
    glVertex(1, -1)
    glEnd()
    f.unbind()
    print t.get_data()[0][0]
    pygame.display.flip()

Somehow, while this code works, I can't help but feel that there are many different ways that the user can break the texture class and trigger some cryptic OpenGL errors. Here is what I am worried about:

1. I have tried to abstract away lots of OpenGL constants. Should these constants be placed at the module level, or should I place them at the class level instead? I also marked them as internal with a leading underscore. Is that all I need to do to dissuade users from modifying these constants and breaking behavior?
2. It does not seem clear from the name of load_from_data() that data can be null (in order to accept incoming framebuffer data). Is there a better name for this function that makes it clear that I can either pass in data for rendering to the screen or leave it blank so that data can be populated in by rendering to the offscreen framebuffer later? Furthermore, with the default of data=None, it is not even clear that a 3D numpy array is required if you need to pass in data.
3. I have lots of properties available to the user, such as width, height, texture_unit, etc. If the user creates a texture, loads some data, and then changes the width of the image later on, the data is not actually updated. What can I do to keep these user parameters and the actual OpenGL data in sync? Should I be doing some sort of validation of these texture parameters?

I am sure that despite all of the considerations, there are even more issues that I can run into later down the line (e.g. "cube-mapping" with 6 textures comes to mind). What improvements can I make to the texture class to address all of the concerns above and make it flexible for future modifications? Should this texture class be split into multiple classes to handle, say, regular rendering versus framebuffer data or color versus depth data? Most other engines online I have seen are built on statically-typed languages and have method overloading and therefore do not seem to have to worry about passing in the wrong data or deal with missing arguments like in Python.

Answer 1

I'm just going to review texture.py.

1. There's no documentation. What kind of thing does an instance of the Texture class represent? How am I supposed to create and use one? What arguments do I pass to the load_from_data method? What does the get_data method return? Do I have to call the methods in a particular order for things to work? And so on.

2. Image formats are represented as strings like "RGBA". This makes it tricky to statically check that code is correct — tools like Pylint won't spot typos like "RBGA". If the code used an enum the it would be easier to check the correctness of code.

3. Image filters are represented by a Boolean: True represents bilinear and False represents nearest-neighbour. This representation makes code hard to understand (there's no particular reason why True represents bilinear rather than nearest-neighbour) and it seems like asking for trouble because in the future you may want to support mipmaps and so GL_TEXTURE_MIN_FILTER will need to take GL_NEAREST_MIPMAP_NEAREST or whatever. I recommend using an enumeration for filters.

4. The Texture class has load_from_url and load_from_data methods that suggest that users might want to reload an existing Texture instance with a new image. This seems contrary to the way that objects are normally used in Python (where if you wanted a new texture, you'd make a new Texture). So I would make load_from_url and load_from_data into class constructors.

5. The Texture class does not ensure that its instances are valid. For example it is an error to create a Texture object and then immediately call bind (you get a NameError). It is a good idea for classes to ensure that their instances are valid: this makes it harder to accidentally misuse them. In this case I would write:

   def __init__(self, width, height, image_format, data=None,
                filter=TextureFilter.LINEAR, texture_unit=0):
       """... docstring explaining the arguments ..."""
       self._id = glGenTextures(1)
       self.width = width
       self.height = height
       # ... and so on ...
   
   @classmethod
   def from_url(cls, asset_url, filter=TextureFilter.LINEAR, texture_unit=0):
       """... docstring explaining the arguments ..."""
       # ... implementation here ...
       return cls(width, height, image_format, data, filter, texture_unit)
   

6. It looks as if the bind method must always be followed by unbind. But the code does not ensure this happens — if the code between bind and unbind raises an exception, then unbind is not called. This looks like a job for a context manager, for example:

   import contextlib
   
   @contextlib.contextmanager
   def bind(self):
       """... docstring explaining the method ..."""
       glActiveTexture(GL_TEXTURE1 + self.texture_unit)
       glBindTexture(GL_TEXTURE_2D, self._id)
       try:
           yield
       finally:
           glActiveTexture(GL_TEXTURE0 + self.texture_unit)
           glBindTexture(GL_TEXTURE_2D, 0)
           glActiveTexture(GL_TEXTURE0)
   

   and then in load_from_data you'd write:

   with self.bind():
       glTexParameterf(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, self.filter)
       glTexParameterf(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, self.filter)
       glTexImage2D(GL_TEXTURE_2D, 0, gl_format, self.width, self.height, 0, gl_format, GL_UNSIGNED_BYTE, data)
   

Answer 2

I have finally made all of these changes both @GarethRees and @MathiasEttinger suggested! Here is my updated texture class:

texture.py

import contextlib
from enum import Enum
import numpy as np
from OpenGL.GL import *
from OpenGL.GL.ARB.texture_float import *
from OpenGL.GL.EXT.framebuffer_object import *
from PIL import Image

class TextureFilter(Enum):
    """Contain the different filter options supported by OpenGL 
    NEAREST: no filtering (use the texel value closest to the pixel of interest's center) - "blocky"
    LINEAR: use a weighted average of the four texels closest to the center of the pixel of interest - default
    """
    NEAREST = GL_NEAREST
    LINEAR = GL_LINEAR

class TextureFormat(Enum):
    """Contain the different internal texture formats supported by OpenGL
    Assumes floats are 32bit, ints are unsigned+8bit
    RGB: Each texel has a red, green, and blue int component (0-255)
    RGBA: Each texel has a red, green, blue, and alpha int component (0-255)
    FLOAT: Each texel has a single float component
    RGB_FLOAT: Each texel has a red, green, and blue float component 
    RGBA_FLOAT: Each texel has a red, green, blue, and alpha float component
    """
    RGB = GL_RGB
    RGBA = GL_RGBA
    FLOAT = GL_FLOAT
    RGB_FLOAT = GL_RGB32F_ARB
    RGBA_FLOAT = GL_RGBA32F_ARB

class Texture(object):

    def __init__(self, width, height, texture_format=TextureFormat.RGBA, data=None, texture_filter=TextureFilter.LINEAR, texture_unit=0):
        """Create a texture object
        width: texture width
        height: texture height
        texture_format: texture format (see TextureFormat)
        data: 1D numpy array containing texture's texel data (use None if the data is to be populated by a framebuffer)
        texture_filter: texture filter type (see TextureFilter)
        texture_unit: the OpenGL texture unit to which the texture should be bound
        Note that all of these properties are read-only (except for data, but only via the update() function)!
        """
        self._texture_id = glGenTextures(1)
        self._width = width
        self._height = height
        self._texture_format = texture_format
        self._texture_filter = texture_filter
        self._texture_unit = texture_unit
        self.update(data)

    @property
    def height(self):
        """Ensure height is a read-only property (no setter defined)"""
        return self._height

    @property
    def texture_filter(self):
        """Ensure texture_filter is a read-only property (no setter defined)"""
        return self._texture_filter

    @property
    def texture_format(self):
        """Ensure texture_format is a read-only property (no setter defined)"""
        return self._texture_format

    @property
    def texture_id(self):
        """Ensure texture_id is a read-only property (no setter defined)"""
        return self._texture_id

    @property
    def texture_unit(self):
        """Ensure texture_unit is a read-only property (no setter defined)"""
        return self._texture_unit

    @property
    def width(self):
        """Ensure width is a read-only property (no setter defined)"""
        return self._width

    def __del__(self):
        """Release the OpenGL texture id and associated GPU memory"""
        glDeleteTextures([self.texture_id])

    @classmethod
    def from_url(cls, asset_url, texture_filter=TextureFilter.LINEAR, texture_unit=0):
        """Create a texture object from an image file
        asset_url: path where image file is located
        texture_filter: texture filter type (see TextureFilter)
        texture_unit: the OpenGL texture unit to which the texture should be bound
        """
        asset = Image.open(asset_url).convert("RGBA")
        data = np.array(asset, dtype=np.uint8).flatten()
        width, height = asset.size
        return cls(width, height, TextureFormat.RGBA, data, texture_filter, texture_unit)

    @contextlib.contextmanager
    def bind(self):
        """Handle binding the texture to and from the correct texture unit
        Revert the OpenGL state when binding is terminated"""
        previous_active = glGetIntegerv(GL_ACTIVE_TEXTURE)
        glActiveTexture(GL_TEXTURE0 + self.texture_unit)
        glBindTexture(GL_TEXTURE_2D, self.texture_id)
        try:
            yield
        finally:
            glActiveTexture(GL_TEXTURE0 + self.texture_unit)
            glBindTexture(GL_TEXTURE_2D, 0)
            glActiveTexture(previous_active)

    def _get_gl_data_type(self):
        """Return the OpenGL data format appropriate for the texture's texture_format (INTERNAL)"""
        if self.texture_format in (TextureFormat.RGB, TextureFormat.RGBA):
            return GL_UNSIGNED_BYTE
        return GL_FLOAT

    def get_texels(self):
        """Retrieve the texel data of the texture from the GPU"""
        texels = None
        data_type = self._get_gl_data_type()
        np_data_type = np.uint8 if data_type == GL_UNSIGNED_BYTE else np.float32
        with self.bind():
            raw_data = glGetTexImage(GL_TEXTURE_2D, 0, self.texture_format.value, data_type)
            texels = np.frombuffer(bytes(raw_data), dtype=np_data_type)
            texels = np.reshape(texels, (self._width, self._height, texels.shape[0]/(self.width * self.height)))
        return texels

    def update(self, data):
        """Update the texture's data and upload the new data to the GPU
        data: new data to replace existing texture data (must match current data numpy array size and dtype)"""
        data_type = self._get_gl_data_type()
        np_data_type = np.uint8 if data_type == GL_UNSIGNED_BYTE else np.float32
        self._data = np.array(data, dtype=np_data_type).flatten() if data is not None else None
        with self.bind():
            glTexParameterf(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, self.texture_filter.value)
            glTexParameterf(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, self.texture_filter.value)
            glTexImage2D(GL_TEXTURE_2D, 0, self.texture_format.value, self.width, self.height, 0, self.texture_format.value, data_type, self._data)

I switched to the contextlib and bind() solution by @GarethRees as it turns out it is important to know the previously bound texture unit so that the OpenGL state can be completely reverted when unbinding, which you cannot achieve easily if the code is split between __enter__ and __exit__! I also went ahead and used properties to make all of the parameters defined in the constructor to be essentially read-only by defining getters without corresponding setters.

For completeness sake, I am also including the new framebuffer and test source code:

framebuffer.py

import contextlib
from enum import Enum
import numpy as np
from OpenGL.GL import *
from OpenGL.GL.ARB.texture_float import *
from OpenGL.GL.EXT.framebuffer_object import *
from pyorama.graphics.texture import Texture

class FramebufferAttachment(Enum):
    """Contain the different framebuffer attachment options supported by OpenGL 
    COLOR_#: OpenGL framebuffer color attachment (0-15)
    DEPTH: OpenGL depth attachment
    STENCIL: OpenGL stencil attachment
    """
    COLOR0 = GL_COLOR_ATTACHMENT0 
    COLOR1 = GL_COLOR_ATTACHMENT1 
    COLOR2 = GL_COLOR_ATTACHMENT2 
    COLOR3 = GL_COLOR_ATTACHMENT3 
    COLOR4 = GL_COLOR_ATTACHMENT4 
    COLOR5 = GL_COLOR_ATTACHMENT5 
    COLOR6 = GL_COLOR_ATTACHMENT6 
    COLOR7 = GL_COLOR_ATTACHMENT7 
    COLOR8 = GL_COLOR_ATTACHMENT8 
    COLOR9 = GL_COLOR_ATTACHMENT9 
    COLOR10 = GL_COLOR_ATTACHMENT10 
    COLOR11 = GL_COLOR_ATTACHMENT11 
    COLOR12 = GL_COLOR_ATTACHMENT12 
    COLOR13 = GL_COLOR_ATTACHMENT13 
    COLOR14 = GL_COLOR_ATTACHMENT14 
    COLOR15 = GL_COLOR_ATTACHMENT15
    DEPTH = GL_DEPTH_ATTACHMENT
    STENCIL = GL_STENCIL_ATTACHMENT

class Framebuffer(object):

    def __init__(self):
        """Create a framebuffer object
        Define textures dictionary, where each key:value pair is a texture:attachment
        """
        self._framebuffer_id = glGenFramebuffersEXT(1)
        self._textures = {}

    @property
    def framebuffer_id(self):
        """Ensure framebuffer_id is a read-only property (no setter defined)"""
        return self._framebuffer_id

    @property
    def textures(self):
        """Ensure textures is a read-only property (no setter defined)"""
        return self._textures

    @contextlib.contextmanager
    def bind(self):
        """Handle binding this framebuffer to replace the current framebuffer
        Revert the OpenGL state when binding is terminated
        Framebuffer must have textures attched to it to be bound"""
        previous_active = glGetIntegerv(GL_FRAMEBUFFER_BINDING)
        if self.textures:
            glBindFramebufferEXT(GL_FRAMEBUFFER_EXT, self.framebuffer_id)
        else:
            raise ValueError("Framebuffer has no textures attached to it")
        try:
            yield
        finally:
            glBindFramebufferEXT(GL_FRAMEBUFFER_EXT, previous_active)

    def attach_texture(self, attachment, texture):
        """Attach a texture to receive data from the framebuffer of the desired attachment type
        attachment: what type of framebuffer data the texture is to receive (see FramebufferAttachment)
        texture: the object to attach to this framebuffer
        """
        self.textures[texture] = attachment
        with self.bind():
            glFramebufferTexture2DEXT(GL_FRAMEBUFFER_EXT, attachment.value, GL_TEXTURE_2D, texture.texture_id, 0)

    def detach_texture(self, texture):
        """Detach a texture from this framebuffer to stop sending the texture data
        texture: the object to remove from this framebuffer
        """
        attachment = self.textures.pop(texture)
        with self.bind():
            glFramebufferTexture2DEXT(GL_FRAMEBUFFER_EXT, attachment.value, GL_TEXTURE_2D, 0, 0)

test.py

import numpy as np
import pygame
import sys
from pyorama.graphics.texture import Texture, TextureFilter, TextureFormat
from pyorama.graphics.framebuffer import Framebuffer, FramebufferAttachment
from OpenGL.GL import *

width, height = (800, 600)
pygame.init()
pygame.display.set_mode((width, height), pygame.OPENGL | pygame.DOUBLEBUF)
fbo = Framebuffer()
sprite = Texture.from_url("logo.png")
tex = Texture(width, height, TextureFormat.RGBA)
fbo.attach_texture(FramebufferAttachment.COLOR0, tex)
rotation = 0.0

while True:
    for event in pygame.event.get():
        if event.type == pygame.QUIT:
            del sprite, tex, fbo
            pygame.quit()
            sys.exit()

    rotation += 0.1
    with fbo.bind():
        glEnable(GL_TEXTURE_2D)
        glEnable(GL_DEPTH_TEST)
        glClear(GL_COLOR_BUFFER_BIT)
        glClear(GL_DEPTH_BUFFER_BIT)
        glMatrixMode(GL_PROJECTION)
        glLoadIdentity()
        glOrtho(0, width, 0, height, -1, 1)
        glMatrixMode(GL_MODELVIEW)
        glLoadIdentity()
        glTranslate(width/2.0, height/2.0, 0)
        glRotate(rotation, 0, 0, 1)
        glTranslate(-sprite.width/2.0, -sprite.height/2.0, 0)
        with sprite.bind():
            glBegin(GL_QUADS)
            glTexCoord(0, 1)
            glVertex(0, 0)
            glTexCoord(0, 0)
            glVertex(0, sprite.height)
            glTexCoord(1, 0)
            glVertex(sprite.width, sprite.height)
            glTexCoord(1, 1)
            glVertex(sprite.width, 0)
            glEnd()

    glEnable(GL_TEXTURE_2D)
    glClear(GL_COLOR_BUFFER_BIT)
    glClear(GL_DEPTH_BUFFER_BIT)
    glMatrixMode(GL_PROJECTION)
    glLoadIdentity()
    glOrtho(-1, 1, -1, 1, -1, 1)
    glMatrixMode(GL_MODELVIEW)
    glLoadIdentity()
    with tex.bind():
        glBegin(GL_QUADS)
        glTexCoord(0, 0)
        glVertex(-1, -1)
        glTexCoord(0, 1)
        glVertex(-1, 1)
        glTexCoord(1, 1)
        glVertex(1, 1)
        glTexCoord(1, 0)
        glVertex(1, -1)
        glEnd()
    pygame.display.flip()

So much nicer to use now, especially thanks to python's indentation visually showing the binding state of the OpenGL framebuffers and textures!