Various BufferDescription fixes (#1911)

* Basic asserts in geometry testing

* Fix incorrect error message + split tests

* BufferDescription: f1 should auto-normalize

arcade/context.py

@@ -146,7 +146,6 @@ def __init__(self, window: pyglet.window.Window, gc_mode: str = "context_gc", gl
                     self.generic_draw_line_strip_color,
                     "4f1",
                     ["in_color"],
-                    normalized=["in_color"],
                 ),
             ]
         )
@@ -159,7 +158,7 @@ def __init__(self, window: pyglet.window.Window, gc_mode: str = "context_gc", gl
         self.shape_line_geometry = self.geometry(
             [
                 BufferDescription(self.shape_line_buffer_pos, "2f", ["in_vert"]),
-                # BufferDescription(self.shape_line_buffer_color, '4f1', ['in_color'], normalized=['in_color'])
+                # BufferDescription(self.shape_line_buffer_color, '4f1', ['in_color'])
             ]
         )
         # ellipse/circle filled

arcade/experimental/lights.py

@@ -94,7 +94,6 @@ def __init__(self, width: int, height: int):
                 self._buffer,
                 '2f 1f 1f 3f',
                 ['in_vert', 'in_radius', 'in_attenuation', 'in_color'],
-                normalized=['in_color'],
             ),
         ])
         self._light_program = self.ctx.load_program(

arcade/gl/types.py

@@ -1,7 +1,7 @@
 from __future__ import annotations
 
 import re
-from typing import Dict, Optional, Iterable, List, Sequence, Tuple, Union
+from typing import Dict, Optional, Iterable, List, Sequence, Tuple, Union, Set
 from typing_extensions import TypeAlias
 
 from pyglet import gl
@@ -125,13 +125,15 @@ def gl_name(gl_type: Optional[PyGLenum]) -> Union[str, PyGLenum, None]:
 
 class AttribFormat:
     """"
-    Represents an attribute in a BufferDescription or a Program.
+    Represents a vertex attribute in a BufferDescription / Program.
+    This is attribute metadata used when attempting to map vertex
+    shader inputs.
 
     :param name: Name of the attribute
     :param gl_type: The OpenGL type such as GL_FLOAT, GL_HALF_FLOAT etc.
-    :param bytes_per_component: Number of bytes a single component takes
-    :param offset: (Optional offset for BufferDescription)
-    :param location: (Optional location for program attribute)
+    :param bytes_per_component: Number of bytes for a single component
+    :param offset: (Optional) Offset for BufferDescription
+    :param location: (Optional) Location for program attribute
     """
 
     __slots__ = (
@@ -224,6 +226,7 @@ class BufferDescription:
         "i2": (gl.GL_SHORT, 2),
         "i4": (gl.GL_INT, 4),
         # Padding (1, 2, 4, 8 bytes)
+        "x": (None, 1),
         "x1": (None, 1),
         "x2": (None, 2),
         "x4": (None, 4),
@@ -253,7 +256,7 @@ def __init__(
         #: List of string attributes
         self.attributes = attributes
         #: List of normalized attributes
-        self.normalized = set() if normalized is None else set(normalized)
+        self.normalized: Set[str] = set() if normalized is None else set(normalized)
         #: Instanced flag (bool)
         self.instanced: bool = instanced
         #: Formats of each attribute
@@ -266,9 +269,7 @@ def __init__(
         if not isinstance(buffer, Buffer):
             raise ValueError("buffer parameter must be an arcade.gl.Buffer")
 
-        if not isinstance(self.attributes, list) and not isinstance(
-            self.attributes, tuple
-        ):
+        if not isinstance(self.attributes, (list, tuple)):
             raise ValueError("Attributes must be a list or tuple")
 
         if self.normalized > set(self.attributes):
@@ -279,7 +280,7 @@ def __init__(
 
         if len(non_padded_formats) != len(self.attributes):
             raise ValueError(
-                f"Different lengths of formats ({len(formats_list)}) and "
+                f"Different lengths of formats ({len(non_padded_formats)}) and "
                 f"attributes ({len(self.attributes)})"
             )
 
@@ -295,6 +296,9 @@ def zip_attrs(formats: List[str], attributes: Sequence[str]):
 
         self.stride = 0
         for attr_fmt, attr_name in zip_attrs(formats_list, self.attributes):
+            # Automatically make f1 attributes normalized
+            if attr_name is not None and "f1" in attr_fmt:
+                self.normalized.add(attr_name)
             try:
                 components_str, data_type_str, data_size_str = re.split(
                     r"([fiux])", attr_fmt

arcade/sprite_list/sprite_list.py

@@ -207,7 +207,7 @@ def _init_deferred(self):
             gl.BufferDescription(self._sprite_angle_buf, "1f", ["in_angle"]),
             gl.BufferDescription(self._sprite_texture_buf, "1f", ["in_texture"]),
             gl.BufferDescription(
-                self._sprite_color_buf, "4f1", ["in_color"], normalized=["in_color"]
+                self._sprite_color_buf, "4f1", ["in_color"],
             ),
         ]
         self._geometry = self.ctx.geometry(

tests/unit/gl/test_opengl_buffer_description.py

@@ -0,0 +1,36 @@
+import pytest
+from arcade.gl import BufferDescription
+
+
+def test_buffer_description(ctx):
+    # TODO: components > 4
+    # TODO: padding
+    buffer = ctx.buffer(reserve=4 * 8)
+    attribute_names = ['in_vert', 'in_uv']
+    descr = BufferDescription(
+        buffer,
+        '2f 2f',
+        attribute_names,
+    )
+    assert descr.num_vertices == 2
+    assert descr.buffer == buffer
+    assert descr.attributes == attribute_names
+    assert descr.instanced is False
+    assert len(descr.formats) == 2
+    assert descr.stride == 16
+
+    # Buffer parameter not a buffer
+    with pytest.raises(ValueError):
+        BufferDescription("test", "2f", ["pos"])
+
+    # Different lengths of attribute names and formats
+    with pytest.raises(ValueError):
+        BufferDescription(buffer, "2f", ["pos", "uv"])
+
+    # Different lengths when padding is used
+    with pytest.raises(ValueError):
+        BufferDescription(buffer, "2x", ["pos"])
+
+    # Invalid format
+    with pytest.raises(ValueError):
+        BufferDescription(buffer, "2g", ["pos"])

tests/unit/gl/test_opengl_geometry.py

@@ -5,12 +5,21 @@
 
 
 def test_quad_2d_fs(ctx):
-    geometry.quad_2d_fs()
+    geo = geometry.quad_2d_fs()
+    assert geo.ctx == ctx
+    assert geo.num_vertices == 4
+    assert geo._mode == ctx.TRIANGLE_STRIP
 
 
 def test_quad_2d(ctx):
-    geometry.quad_2d()
+    geo = geometry.quad_2d()
+    assert geo.ctx == ctx
+    assert geo.num_vertices == 4
+    assert geo._mode == ctx.TRIANGLE_STRIP
 
 
 def test_screen_rectangle(ctx):
-    geometry.screen_rectangle(0, 100, 0, 100)
+    geo = geometry.screen_rectangle(0, 100, 0, 100)
+    assert geo.ctx == ctx
+    assert geo.num_vertices == 4
+    assert geo._mode == ctx.TRIANGLE_STRIP

tests/unit/gl/test_opengl_types.py

@@ -1,5 +1,4 @@
 import pytest
-import arcade
 from pyglet import gl
 from arcade.gl import types
 

tests/unit/gl/test_opengl_vertex_array.py

@@ -9,28 +9,6 @@
 from arcade.gl.program import Program
 
 
-def test_buffer_description(ctx):
-    # TODO: components > 4
-    # TODO: padding
-    buffer = ctx.buffer(reserve=4 * 8)
-    attribute_names = ['in_vert', 'in_uv']
-    descr = BufferDescription(
-        buffer,
-        '2f 2f',
-        attribute_names,
-    )
-    assert descr.num_vertices == 2
-    assert descr.buffer == buffer
-    assert descr.attributes == attribute_names
-    assert descr.instanced is False
-    assert len(descr.formats) == 2
-    assert descr.stride == 16
-
-    # Buffer parameter not a buffer
-    with pytest.raises(ValueError):
-        BufferDescription("test", "2f", ["pos"])
-
-
 def test_geometry(ctx):
     """Test vertex_array"""
     program = ctx.load_program(
@@ -43,7 +21,6 @@ def test_geometry(ctx):
             ctx.buffer(reserve=4 * num_vertices),
             '4f1',
             ['in_color'],
-            normalized=['in_color'],
         ),
         BufferDescription(
             ctx.buffer(reserve=8 * num_vertices),