From 9d1025359c7034774d4c6545ba7cb7710541e993 Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Fri, 15 Sep 2023 06:15:51 -0400 Subject: [PATCH 01/17] Allow SpriteList._atlas to be None by default * Remove if wrapping setting _atlas in SpriteList.__init__ * Update conditional _atlas setting in SpriteList._init_deferred * Update type annotations to make `SpriteList.atlas` optional --- arcade/sprite_list/sprite_list.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/arcade/sprite_list/sprite_list.py b/arcade/sprite_list/sprite_list.py index b8de470e4..3777bd96b 100644 --- a/arcade/sprite_list/sprite_list.py +++ b/arcade/sprite_list/sprite_list.py @@ -105,8 +105,7 @@ def __init__( visible: bool = True, ): self.program = None - if atlas: - self._atlas: TextureAtlas = atlas + self._atlas: Optional[TextureAtlas] = atlas self._initialized = False self._lazy = lazy self._visible = visible @@ -190,9 +189,8 @@ def _init_deferred(self): self.ctx = get_window().ctx self.program = self.ctx.sprite_list_program_cull - self._atlas: TextureAtlas = ( - getattr(self, "_atlas", None) or self.ctx.default_atlas - ) + if not self._atlas: + self._atlas = self.ctx.default_atlas # Buffers for each sprite attribute (read by shader) with initial capacity self._sprite_pos_buf = self.ctx.buffer(reserve=self._buf_capacity * 12) # 3 x 32 bit floats @@ -371,7 +369,7 @@ def alpha_normalized(self, value: float): self._color = self._color[0], self._color[1], self._color[2], value @property - def atlas(self) -> "TextureAtlas": + def atlas(self) -> Optional["TextureAtlas"]: """Get the texture atlas for this sprite list""" return self._atlas From 4e41b98839528fb12554d984cb4c2d98fb2f86b3 Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Fri, 15 Sep 2023 06:25:16 -0400 Subject: [PATCH 02/17] Update unit tests for SpriteList with None atlas --- tests/unit/spritelist/test_spritelist_lazy.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/unit/spritelist/test_spritelist_lazy.py b/tests/unit/spritelist/test_spritelist_lazy.py index 8696ba2aa..ecfbc3fce 100644 --- a/tests/unit/spritelist/test_spritelist_lazy.py +++ b/tests/unit/spritelist/test_spritelist_lazy.py @@ -1,5 +1,6 @@ import pytest import arcade +from arcade import TextureAtlas def test_create(): @@ -14,6 +15,7 @@ def test_create(): assert len(spritelist) == 100 assert spritelist.spatial_hash is not None assert spritelist._initialized is False + assert spritelist.atlas is None arcade.set_window(None) with pytest.raises(RuntimeError): @@ -30,4 +32,5 @@ def test_create_2(window): assert spritelist._initialized assert spritelist._sprite_pos_buf assert spritelist._geometry + assert isinstance(spritelist.atlas, TextureAtlas) spritelist.draw() From 7fc4995fbaef0fd41adf9a3be549e71acecc4170 Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Fri, 15 Sep 2023 06:44:43 -0400 Subject: [PATCH 03/17] Rename, reorganize, and comment SpriteList lazy=True tests for clarity --- tests/unit/spritelist/test_spritelist_lazy.py | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/tests/unit/spritelist/test_spritelist_lazy.py b/tests/unit/spritelist/test_spritelist_lazy.py index ecfbc3fce..772c4dc37 100644 --- a/tests/unit/spritelist/test_spritelist_lazy.py +++ b/tests/unit/spritelist/test_spritelist_lazy.py @@ -3,11 +3,16 @@ from arcade import TextureAtlas -def test_create(): +def test_create_lazy_equals_true(): """Test lazy creation of spritelist""" spritelist = arcade.SpriteList(lazy=True, use_spatial_hash=True) + + # Make sure OpenGL abstractions are not created assert spritelist._sprite_pos_buf == None assert spritelist._geometry == None + assert spritelist.atlas is None + + # Make sure CPU-only behavior still works correctly for x in range(100): spritelist.append( arcade.Sprite(":resources:images/items/coinGold.png", center_x=x * 64) @@ -15,22 +20,30 @@ def test_create(): assert len(spritelist) == 100 assert spritelist.spatial_hash is not None assert spritelist._initialized is False - assert spritelist.atlas is None + # Verify that initialization will fail without a window arcade.set_window(None) with pytest.raises(RuntimeError): spritelist.initialize() -def test_create_2(window): +def test_manual_initialization_after_lazy_equals_true(window): + """Test manual initialization of lazy sprite lists.""" spritelist = arcade.SpriteList(lazy=True) + + # CPU-only actions which shouldn't affect initializing OpenGL resources sprite = arcade.SpriteSolidColor(10, 10, color=(255, 255, 255, 255)) spritelist.append(sprite) spritelist.remove(sprite) + # Make sure initialization still worked correctly. spritelist.initialize() assert spritelist._initialized assert spritelist._sprite_pos_buf assert spritelist._geometry assert isinstance(spritelist.atlas, TextureAtlas) + + # Uncomment the next line and set a breakpoint on it to + # spot-check the number of sprites drawn (it should be zero). spritelist.draw() + # pass \ No newline at end of file From 213b729b0d5b9437a3e655335610a427a874f838 Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Fri, 15 Sep 2023 06:53:39 -0400 Subject: [PATCH 04/17] Update lazy argument docstring with link to programming guide --- arcade/sprite_list/sprite_list.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/arcade/sprite_list/sprite_list.py b/arcade/sprite_list/sprite_list.py index 3777bd96b..f7e2cfb60 100644 --- a/arcade/sprite_list/sprite_list.py +++ b/arcade/sprite_list/sprite_list.py @@ -86,11 +86,15 @@ class SpriteList(Generic[SpriteType]): :param capacity: (Advanced) The initial capacity of the internal buffer. It's a suggestion for the maximum amount of sprites this list can hold. Can normally be left with default value. - :param lazy: (Advanced) Enabling lazy spritelists ensures no internal OpenGL - resources are created until the first draw call or ``initialize()`` - is called. This can be useful when making spritelists in threads - because only the main thread is allowed to interact with - OpenGL. + :param lazy: (Advanced) Delay creating OpenGL resources for the sprite list + until its first call of either of these methods: + + 1. py:meth:`~SpriteList.draw` + 2. :py:meth:`~SpriteList.initialize` + + This is useful for advanced techniques like parallel loading or world + generation. See :ref:`pg_spritelist_advanced_lazy_spritelists` to learn + more. :param visible: Setting this to False will cause the SpriteList to not be drawn. When draw is called, the method will just return without drawing. """ From f3fed5180aa719276790f886f9f6f205e2fb80c6 Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Fri, 15 Sep 2023 08:11:57 -0400 Subject: [PATCH 05/17] Link Lazy Spritelist from the lazy param docstring --- arcade/sprite_list/sprite_list.py | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/arcade/sprite_list/sprite_list.py b/arcade/sprite_list/sprite_list.py index f7e2cfb60..83f4d1a48 100644 --- a/arcade/sprite_list/sprite_list.py +++ b/arcade/sprite_list/sprite_list.py @@ -86,15 +86,10 @@ class SpriteList(Generic[SpriteType]): :param capacity: (Advanced) The initial capacity of the internal buffer. It's a suggestion for the maximum amount of sprites this list can hold. Can normally be left with default value. - :param lazy: (Advanced) Delay creating OpenGL resources for the sprite list - until its first call of either of these methods: - - 1. py:meth:`~SpriteList.draw` - 2. :py:meth:`~SpriteList.initialize` - - This is useful for advanced techniques like parallel loading or world - generation. See :ref:`pg_spritelist_advanced_lazy_spritelists` to learn - more. + :param lazy: (Advanced) ``True`` delays creating OpenGL resources + for the sprite list until either its :py:meth:`~SpriteList.draw` + or :py:meth:`~SpriteList.initialize` method is called. See + :ref:`pg_spritelist_advanced_lazy_spritelists` to learn more. :param visible: Setting this to False will cause the SpriteList to not be drawn. When draw is called, the method will just return without drawing. """ From 79813913edc3cbb3d3674f2f2b5be8f32b4c1883 Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Fri, 15 Sep 2023 08:53:36 -0400 Subject: [PATCH 06/17] Rewrite SpriteList.initialize docstring * Remove unecessary details * Link draw method * Link lazy sprite list section of the programming guide --- arcade/sprite_list/sprite_list.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/arcade/sprite_list/sprite_list.py b/arcade/sprite_list/sprite_list.py index 83f4d1a48..024427775 100644 --- a/arcade/sprite_list/sprite_list.py +++ b/arcade/sprite_list/sprite_list.py @@ -944,14 +944,17 @@ def _write_sprite_buffers_to_gpu(self): def initialize(self): """ - Create the internal OpenGL resources. - This can be done if the sprite list is lazy or was created before the window / context. - The initialization will happen on the first draw if this method is not called. - This is acceptable for most people, but this method gives you the ability to pre-initialize - to potentially void initial stalls during rendering. - - Calling this otherwise will have no effect. Calling this method in another thread - will result in an OpenGL error. + Request immediate creation of OpenGL resources for this list. + + Calling this method is optional. It only has an effect for lists + created with ``lazy=True``. If this method is not called, + uninitialized sprite lists will automatically initialize OpenGL + resources on their first :py:meth:`~SpriteList.draw` call instead. + + This method is useful for performance optimization, advanced + techniques, and writing tests. Do not call it across thread + boundaries. See :ref:`pg_spritelist_advanced_lazy_spritelists` + to learn more. """ self._init_deferred() From e632e0ebff104c18a53469759640916e58519a22 Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Fri, 15 Sep 2023 08:55:08 -0400 Subject: [PATCH 07/17] Add None type annotation on SpriteList.initialize --- arcade/sprite_list/sprite_list.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arcade/sprite_list/sprite_list.py b/arcade/sprite_list/sprite_list.py index 024427775..d1e0593ff 100644 --- a/arcade/sprite_list/sprite_list.py +++ b/arcade/sprite_list/sprite_list.py @@ -942,7 +942,7 @@ def _write_sprite_buffers_to_gpu(self): self._sprite_index_buf.write(self._sprite_index_data) self._sprite_index_changed = False - def initialize(self): + def initialize(self) -> None: """ Request immediate creation of OpenGL resources for this list. From 6e122a15b496943589405a7cda64e8744c0d92ab Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Fri, 15 Sep 2023 09:00:59 -0400 Subject: [PATCH 08/17] Update SpriteList.draw docstring with lazy info * Add brief overview of when there may be a performance hit * Link the lazy sprite list section --- arcade/sprite_list/sprite_list.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/arcade/sprite_list/sprite_list.py b/arcade/sprite_list/sprite_list.py index d1e0593ff..0c2b446ca 100644 --- a/arcade/sprite_list/sprite_list.py +++ b/arcade/sprite_list/sprite_list.py @@ -968,6 +968,16 @@ def draw( """ Draw this list of sprites. + Uninitialized sprite lists will first create OpenGL resources + before drawing. This may cause a performance stutter when the + following are true: + + 1. You created the sprite list with ``lazy=True`` + 2. You did not call :py:meth:`~SpriteList.initialize` before drawing + 3. You are initializing many sprites and/or lists at once + + See :ref:`pg_spritelist_advanced_lazy_spritelists` to learn more. + :param filter: Optional parameter to set OpenGL filter, such as `gl.GL_NEAREST` to avoid smoothing. :param pixelated: ``True`` for pixelated and ``False`` for smooth interpolation. From c4ba171b45972f43cb01f84631a470c2b69795cc Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Fri, 15 Sep 2023 09:13:26 -0400 Subject: [PATCH 09/17] Fix None type issue in perf graph --- arcade/perf_graph.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/arcade/perf_graph.py b/arcade/perf_graph.py index 440ac5c01..2d3300cfb 100644 --- a/arcade/perf_graph.py +++ b/arcade/perf_graph.py @@ -224,10 +224,13 @@ def update_graph(self, delta_time: float): return sprite_list = self.sprite_lists[0] + atlas = sprite_list.atlas # Clear and return if timings are disabled if not arcade.timings_enabled(): - with sprite_list.atlas.render_into(self.minimap_texture, projection=self.proj) as fbo: + # Please forgive the ugly spacing. It makes type checking work. + with atlas.render_into( # type: ignore + self.minimap_texture, projection=self.proj) as fbo: fbo.clear(color=(0, 0, 0, 255)) return From 9830d772053ce804723eb4babe5fea1434024092 Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Fri, 15 Sep 2023 09:14:08 -0400 Subject: [PATCH 10/17] Fix a None type issue in SpriteList.append --- arcade/sprite_list/sprite_list.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arcade/sprite_list/sprite_list.py b/arcade/sprite_list/sprite_list.py index 0c2b446ca..34d3779e9 100644 --- a/arcade/sprite_list/sprite_list.py +++ b/arcade/sprite_list/sprite_list.py @@ -621,7 +621,7 @@ def append(self, sprite: SpriteType): if self._initialized: if sprite.texture is None: raise ValueError("Sprite must have a texture when added to a SpriteList") - self._atlas.add(sprite.texture) + self._atlas.add(sprite.texture) # type: ignore def swap(self, index_1: int, index_2: int): """ From 619fd6846a1608609991b26da09ad9141e267139 Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Fri, 15 Sep 2023 09:15:39 -0400 Subject: [PATCH 11/17] Fix a second None type issue in perf graph --- arcade/perf_graph.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/arcade/perf_graph.py b/arcade/perf_graph.py index 2d3300cfb..2d8b10a70 100644 --- a/arcade/perf_graph.py +++ b/arcade/perf_graph.py @@ -287,7 +287,9 @@ def update_graph(self, delta_time: float): text_object.text = f"{int(index * view_y_legend_increment)}" # Render to the internal texture - with sprite_list.atlas.render_into(self.minimap_texture, projection=self.proj) as fbo: + # This ugly spacing is intentional to make type checking work. + with atlas.render_into( # type: ignore + self.minimap_texture, projection=self.proj) as fbo: # Set the background color fbo.clear(self.background_color) From 2554a5d8a5d30c476fc4e4ec89a0c64315e2deca Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Fri, 15 Sep 2023 09:18:04 -0400 Subject: [PATCH 12/17] Fix a None type issue in preload_textures --- arcade/sprite_list/sprite_list.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/arcade/sprite_list/sprite_list.py b/arcade/sprite_list/sprite_list.py index 34d3779e9..4a31e277c 100644 --- a/arcade/sprite_list/sprite_list.py +++ b/arcade/sprite_list/sprite_list.py @@ -880,7 +880,9 @@ def preload_textures(self, texture_list: List["Texture"]) -> None: raise ValueError("Cannot preload textures before the window is created") for texture in texture_list: - self._atlas.add(texture) + # Ugly spacing is a fast workaround for None type checking issues + self._atlas.add( # type: ignore + texture) def write_sprite_buffers_to_gpu(self) -> None: From a17224990a45b757b81cf38d657cc160135a998c Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Fri, 15 Sep 2023 09:23:15 -0400 Subject: [PATCH 13/17] Fix 4 None errors in SpriteList.draw --- arcade/sprite_list/sprite_list.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/arcade/sprite_list/sprite_list.py b/arcade/sprite_list/sprite_list.py index 4a31e277c..a36de1d4b 100644 --- a/arcade/sprite_list/sprite_list.py +++ b/arcade/sprite_list/sprite_list.py @@ -1000,22 +1000,25 @@ def draw( else: self.ctx.blend_func = self.ctx.BLEND_DEFAULT + # Workaround for Optional[TextureAtlas] + slow . lookup speed + atlas_texture: Texture2D = self.atlas.texture # type: ignore + # Set custom filter or reset to default if filter: if hasattr(filter, '__len__', ): # assume it's a collection if len(cast(Sized, filter)) != 2: raise ValueError("Can't use sequence of length != 2") - self.atlas.texture.filter = tuple(filter) # type: ignore + atlas_texture.filter = tuple(filter) # type: ignore else: # assume it's an int - self.atlas.texture.filter = cast(OpenGlFilter, (filter, filter)) + atlas_texture.filter = cast(OpenGlFilter, (filter, filter)) else: - self.atlas.texture.filter = self.ctx.LINEAR, self.ctx.LINEAR + atlas_texture.filter = self.ctx.LINEAR, self.ctx.LINEAR # Handle the pixelated shortcut if pixelated: - self.atlas.texture.filter = self.ctx.NEAREST, self.ctx.NEAREST + atlas_texture.filter = self.ctx.NEAREST, self.ctx.NEAREST else: - self.atlas.texture.filter = self.ctx.LINEAR, self.ctx.LINEAR + atlas_texture.filter = self.ctx.LINEAR, self.ctx.LINEAR if not self.program: raise ValueError("Attempting to render without 'program' field being set.") From 28a883762680a2d992c7e47ba041e95e90edee97 Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Fri, 15 Sep 2023 09:26:01 -0400 Subject: [PATCH 14/17] Extend workaround / speed tweak to the rest of SpriteList.draw --- arcade/sprite_list/sprite_list.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/arcade/sprite_list/sprite_list.py b/arcade/sprite_list/sprite_list.py index a36de1d4b..6ec6b641d 100644 --- a/arcade/sprite_list/sprite_list.py +++ b/arcade/sprite_list/sprite_list.py @@ -33,6 +33,7 @@ get_window, gl, ) +from arcade.gl import Texture2D from arcade.types import Color, RGBA255 from arcade.gl.types import OpenGlFilter, BlendFunction, PyGLenum from arcade.gl.buffer import Buffer @@ -1000,8 +1001,9 @@ def draw( else: self.ctx.blend_func = self.ctx.BLEND_DEFAULT - # Workaround for Optional[TextureAtlas] + slow . lookup speed - atlas_texture: Texture2D = self.atlas.texture # type: ignore + # Workarounds for Optional[TextureAtlas] + slow . lookup speed + atlas: TextureAtlas = self.atlas # type: ignore + atlas_texture: Texture2D = atlas.texture # Set custom filter or reset to default if filter: @@ -1025,8 +1027,8 @@ def draw( self.program["spritelist_color"] = self._color - self._atlas.texture.use(0) - self._atlas.use_uv_texture(1) + atlas_texture.use(0) + atlas.use_uv_texture(1) if not self._geometry: raise ValueError("Attempting to render without '_geometry' field being set.") self._geometry.render( From 3499fd768e677192d219e535c3a4f3a0143902ad Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Fri, 15 Sep 2023 09:29:15 -0400 Subject: [PATCH 15/17] Add None typing workaround to _update_texture --- arcade/sprite_list/sprite_list.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/arcade/sprite_list/sprite_list.py b/arcade/sprite_list/sprite_list.py index 6ec6b641d..f3b8fa481 100644 --- a/arcade/sprite_list/sprite_list.py +++ b/arcade/sprite_list/sprite_list.py @@ -1176,8 +1176,10 @@ def _update_texture(self, sprite) -> None: if not sprite._texture: return - - tex_slot, _ = self._atlas.add(sprite._texture) + atlas = self._atlas + # Ugly spacing makes type checking work with specificity + tex_slot: int = atlas.add( # type: ignore + sprite._texture)[0] slot = self.sprite_slot[sprite] self._sprite_texture_data[slot] = tex_slot From 936d2c0fb59e5f9328e72188348a09d4ffb597aa Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Fri, 15 Sep 2023 09:31:40 -0400 Subject: [PATCH 16/17] Add typing fix in SpriteList._update_all --- arcade/sprite_list/sprite_list.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/arcade/sprite_list/sprite_list.py b/arcade/sprite_list/sprite_list.py index f3b8fa481..903927166 100644 --- a/arcade/sprite_list/sprite_list.py +++ b/arcade/sprite_list/sprite_list.py @@ -1160,7 +1160,9 @@ def _update_all(self, sprite: SpriteType): if not sprite._texture: return - tex_slot, _ = self._atlas.add(sprite._texture) + # Ugly syntax makes type checking pass without perf hit from cast + tex_slot: int = self._atlas.add( # type: ignore + sprite._texture)[0] slot = self.sprite_slot[sprite] self._sprite_texture_data[slot] = tex_slot From e69b51c5f9b95e10b543fc9f45832f2c884eb784 Mon Sep 17 00:00:00 2001 From: pushfoo <36696816+pushfoo@users.noreply.github.com> Date: Fri, 15 Sep 2023 10:52:38 -0400 Subject: [PATCH 17/17] Add missing newline at end of file --- tests/unit/spritelist/test_spritelist_lazy.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/spritelist/test_spritelist_lazy.py b/tests/unit/spritelist/test_spritelist_lazy.py index 772c4dc37..c7b46959a 100644 --- a/tests/unit/spritelist/test_spritelist_lazy.py +++ b/tests/unit/spritelist/test_spritelist_lazy.py @@ -46,4 +46,4 @@ def test_manual_initialization_after_lazy_equals_true(window): # Uncomment the next line and set a breakpoint on it to # spot-check the number of sprites drawn (it should be zero). spritelist.draw() - # pass \ No newline at end of file + # pass