Designing a Robust Texture and Framebuffer class for Game Engine

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:

• Loading in image data from a file

• Loading in generated pixel data

• Applying a texture to a sprite/model in my scene

• Accept data in a variety of color formats.

• Accept not only color data, but also depth and stencil data.

• 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(

I'm just going to review texture.py.

-
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.

-
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.

-
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.

-
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.

-
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)

-
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)

StackExchange Code Review Q#156260, answer score: 6