pythonarcade
Eliminate slow CPU-based texture duplication & flipping
Enhancement request:
tl;dr:
What:
- Stop using
flipped_arguments from theSprite.__init__to trigger CPU-based flipping inload_texture - Multiply the coordinates instead, either via Sprite's scale or when returning a Texture object referring to the flipped data
- Make sure width, height, and size return absolute values instead of negatives
Why:
- Faster load times (#1159)
- Use less memory
- Eliminate bugs from nonsensical negative dimensions
Bonus: A step toward cleaning up Sprite's __init__ method
What should be added/changed?
Sprites currently have three flipped_ keyword arguments in their constructors:
https://github.com/pythonarcade/arcade/blob/8ad254f0e31fdb6d582e7dc50322732998a7caed/arcade/sprite.py#L157-L174
The sprite __init__ method passes these to texture.load_texture:
https://github.com/pythonarcade/arcade/blob/8ad254f0e31fdb6d582e7dc50322732998a7caed/arcade/sprite.py#L241-L253
This function uses CPU-bound transformation for flipping: https://github.com/pythonarcade/arcade/blob/8ad254f0e31fdb6d582e7dc50322732998a7caed/arcade/texture.py#L584-L591
We should eliminate that and use scale's x and y dimensions instead. If this is unpalatable, it may also be possible to create flipped textures that refer to the same portion of the atlas but perform math / GL tricks to interpret the texture backwards. I prefer the scale option because of the reasons listed below.
What would it help with?
1. Faster load times (#1159)
CPU-bound Pillow functions for creating flipped copies are slow compared to letting the GPU do it. In the worst case, the image can be transposed three times when all three booleans are true: https://github.com/pythonarcade/arcade/blob/8ad254f0e31fdb6d582e7dc50322732998a7caed/arcade/texture.py#L584-L591
This also creates additional garbage collection overhead that slows down load times.
2. Reduce memory usage on CPU and GPU
The main memory savings will come from not duplicating image data + mipmaps. There will also be a little bit of memory saved by using shorter strings and not growing dict spaces so much, but that is negligible unless people are loading a lot of different image files.
3. Eliminate bugs from nonsensical negative dimensions
Width and height are conceptually unsigned quantities. Forcing the user to call abs is annoying and may introduce some overhead. Additionally, reporting negative sizes can create some very interesting offset bugs when calculating layouts or movement. We can eliminate all of these problems returning absolute dimensions from getters, and optionally caching the results.
Bonus: A step toward cleaning up Sprite's __init__ method
We can also define orientation constants and helpers that define which orientation to give sprites by setting up scale in the constructor.
For example, constants might look like this:
FLIPPED_HORIZONTALLY = (-1.0, 1.0)
FLIPPED_VERTICALLY = (1.0, -1.0)
FLIPPED_DIAGONALLY = (-1.0, -1.0)
These might be easier to use if https://github.com/pyglet/pyglet/issues/659 is resolved first. We could then make the constants into FrozenVec2 instances that can be multiplied for convenience:
sprite = Sprite("path/to/file.png", scale = FLIPPED_HORIZONTALLY * 2.0)
We can also rotate and flip texture coordinates. The hit box points also need to be transformed.
That confirms what I was guessing at here:
it may also be possible to create flipped textures that refer to the same portion of the atlas but perform math / GL tricks to interpret the texture backwards.
I'm not yet sure of the pros and cons for each approach. Maybe changing the load_texture behavior and adding support for scale-based flipping are orthogonal and complimentary features.
You're definitely right about the hitbox transform either way.
Regardless of which approach we take, we'll also need to add warnings about how drawing into the atlas/texture will affect any references to the same data elsewhere.
The underlying texture atlas is definitely a bit "magic" for most users and needs to be explained better somewhere in the docs. Preferable the manual. I still think transforming texture coordinates will make it simpler for most use cases.
Transforming texture coordinates can also partly solve an important packer issue with the current row based atlas. Let's say a user makes very tall sprite. For example 4 x 1000. This will end up taking up a lot of vertical space in the atlas itself. In theory we could rotate tall textures to pack textures better, but rendering into them will be weird if rotated.
If my understanding is correct, this ticket overlaps heavily with #1102 because altering the texture mapping coordinates allows both flipping and rotation effects.
Yes, there is a lot of overlap here except that we're moving lower down in the system to the atlas itself to solve multiple problems. We would no longer store multiple versions of the same texture in the atlas in different transformed states. Instead we store the transforms in the Texture object and use that to tweak the hit box points and texture coordinates. The logic in the sprite itself stays unchanged.
Should we support something like a deepcopy argument that can be used to force copying the data instead of referencing the same portion of the atlas with different coordinates?
Should we support something like a
deepcopyargument that can be used to force copying the data instead of referencing the same portion of the atlas with different coordinates?
We could do a clone() or something that adds a suffix on the texture name?
We now have texture transforms. Transformations are very cheap and is referencing the same texture in the atlas. Scale can of course also be used in creating ways.