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)
   

• 301_Moved_Permanently: Wouldn't it be better to rename bind into __enter__ and unbind into __exit__ rather than using contextlib? I have the feeling that, in the main, using with framebuffer: would be nicer than with framebuffer.bind():.
• CodeSurgeon: @GarethRees Thank you so much for this answer, very thorough! I noticed that in point 5, the __init__ has one of the arguments as filter. Is it fine that this is a defined keyword, should I try to awkwardly work around that with texture_filter, or should I just append a trailing underscore like filter_?
• CodeSurgeon: @MathiasEttinger That does seem convenient for the framebuffer use case! I will take a look at __enter__ and __exit__ too. When I make all of these changes, do I just update the question with the new code at the bottom, or write this new code as an answer?
• 301_Moved_Permanently: @CodeSurgeon What to do when someone answers
• Gareth Rees: @CodeSurgeon: filter is a built-in function (not a keyword) so if you don't need to use the function of that name, then nothing will go wrong if you use the name for parameter. (But if you're worried about possible confusion, then by all means use texture_filter.)

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!