-
Notifications
You must be signed in to change notification settings - Fork 358
Description
Enhancement / Request for Feedback: Color API
tl;dr
- Our current color handling and
Colortype have multiple flaws as shown by Make Color constants usable without affecting alpha #1838 - Much of item 1 is due to trying to nudge users away from using RGB colors
- Some of our color property and argument names are inconsistent with both our prior code and pyglet's
- This is a GitHub issue rather than a Discord thread because the users most concerned with Color (@bunny-therapist) do not appear to use Discord
My current (and loosely held) idea for fixing this while preserving our "keep the alpha unless replaced" behavior:
- Fulfill Make Color constants usable without affecting alpha #1838 as follows (mostly per @bunny-therapist's original suggestions):
- Use
RGBOrA255instead ofRGB255for color arguments throughout the codebase - Make the
Colortype 3-length except when an alpha value is provided - Add a
Color.rgbproperty as beginner-friendly shorthand forcolor_instance[:3] - Do not provide alpha values for our current color constants, except for
TRANSPARENT_BLACK - TBD: When someone uses
alphaon a 3-lengthColor, which happens:raise IndexError()orreturn None?
- Use
- Add an
OPAQUE_WHITEcolor constant and use it - Add
replace(self, r = None, g = None, b = None, alpha = None) -> Colorandopaque() -> Colorinstance methods toColor - Add
opacityproperties throughout the codebase to match pyglet's API - Add an
alphaproperty toColorto be consistent with our own properties
Please comment on known issues or mention ones not yet covered.
Once we reach rough consensus, we can turn the items above into a checkboxes and start making PRs.
Known Problems
As pointed out in #1838, there are three main sources of confusion:
- Although we still support RGB arguments, we use
RGBA255as a type hint - Users may expect arcade's color constants to leave the alpha of objects alone
Colordoesn't provide a beginner-friendly way to use only the RGB components
These are not minor issues. Many users will think of color and opacity as separate because:
- Many programs present them as separate (image editors, etc)
- Our API and pyglet's API present separate properties for color and opacity
- Support for setting color to RGB values further reinforces this idea
While reviewing arcade and pyglet's code, I also noticed pyglet uses opacity for its alpha-related property, while we use alpha and a.
What we have now
To my understanding, Arcade's color handling includes alpha because:
- In almost all places, pyglet accepts RGBA colors (
pyglet.Spriteis the only exception, and it will be changed shortly) - It does this because OpenGL represents colors as RGBA internally
Since this is supposed to be a beginner-friendly library, it was a mistake to try to force users to use RGBA. Additionally, pyglet does not seem to be planning on dropping RGB support any time soon, so we have no reason to drop it.
| Type + Link to Source | Intended Usage | Current Usage | Comments |
|---|---|---|---|
ChannelType = TypeVar('ChannelType') |
Allow defining generic color type aliases | Only used in types.py |
|
RGB = Tuple[ChannelType, ChannelType, ChannelType] |
Generic RGB type allowing specifying channel type | Only used directly in types.py |
|
RGBA = Tuple[ChannelType, ChannelType, ChannelType, ChannelType] |
Generic RGBA type allowing specifying channel type | Only used directly in types.py |
|
RGBOrA = Union[RGB[ChannelType], RGBA[ChannelType]] |
A generic for either 3 or 4 tuples of a channel type | Only used directly in types.py |
|
RGBOrA255 = RGBOrA[int] |
Specify either 3 or 4 length 0-255 color | Only in arcade.text |
We should probably have used this instead of trying to encourage RGBA. |
RGBOrANormalized = RGBOrA[float] |
Specify either 3 or 4 length float colors | Only in arcade.types |
|
RGBA255OrNormalized = Union[RGBA255, RGBANormalized] |
An RGBA color of either int or float channel type. | Only in Window.clear() and View.clear() |
Per @einarf's comments on Discord, clear shouldn't use this. |
class Color[RGBA255]: |
Backward-compatible RGBA color definition type | arcade.color and arcade.csscolor |
There is also a paused PR for adding a float version of the Color class (#1772). It would also help with #1794, among other issues. I've paused for multiple reasons, some of which include uncertainty about the color API.
Top Color Questions
2. How Should Color Handle Length & Alpha?
Length
I see the following as worth considering for our Color type and constants:
- 4-length only, with a helper
.rgbproperty to get the rgb channels, as suggested by Make Color constants usable without affecting alpha #1838 - Either 3 or 4 length
In the latter case, we could still have constants like arcade.color.TRANSPARENT_BLACK. However, we'd have to make the following changes:
- Make most constants 3-length
- Add
with_alpha(self, alpha: int) -> ColorandColor.opaque(self) -> Colorinstance methods - Update
Color.from_hex_string()
We can also mix it with an .rgb property since users may find it helpful if they write / use color mixing methods.
At the moment, the 3 or 4 length option seems best to me.
Alpha
How should accessing alpha on a 3-length color be handled?
- Raise a clearly worded
IndexError - Return
None
3. Should We Support Non-RGB(A)? If so, how?
@cspotcode has brought up non-RGBA support. It has also been brought up on Discord at least once.
I've argued in favor of RGB/RGBA only for the following reasons:
- Compatibility with pyglet and prior arcade-dependent frameworks
- Performance
- Simplicity / ease of implementation
If we want to enable support for non-RGBA, we have the following options:
- Subclasses
- Helper methods (ex:
def from_hsv255(self, h: int, s: int: v: int) -> Self:)- The standard library already includes HSV conversion helpers in
colorsys - Our helper methods could provide type conversion
- The standard library already includes HSV conversion helpers in
Helper methods seem like the best option since they can be added at any time without performance or complexity penalties.
4. Should We Support Subclassing? If so, how?
Doing so may preclude optimizations such as #1841. However, we may be able to side-step this issue entirely if we define color as a Protocol type as outlined below.
Wish List Ideas
These are potentially thorny issues and probably best avoided.
I've mentioned them in case someone else can think of ways around their problems.
1. Color as a Protocol Type?
Using Protocol types could open the door to additional benefits.
Pros:
- Potentially allow anything iterable or with the right properties to be a subtype.
Cons:
- We lose static analysis of color length
- Subscripted generic
Protocoltypes marked with@runtime_checkabledo not check types when used withisinstancebecause the decorator does not enable checking signatures of methods
1. Assignment to Slices of Color Properties?
As of now, I haven't found a good way to support syntax for the following:
sprite_instance.color[:3] = (1, 2, 3)By "good", I mean the following:
- Performant
- Easy to implement
If we build this on the Protocol concept above, we might be able get the following:
- Enable support for both mutable proxy objects and immutable color types
- Enable more expressive syntax such as setting color values from arbitrary iterables
- May also enable syntax such as
sprite_instance.color.rgb = 128, 0, 0
However, it would have the following downsides:
- Length and channel type checks become a run-time issue since
Protocolsdo not appear to support it - Entirely new problems due to returning mutable objects: somehow limiting view lifetime, or accepting risks from persisting mutable references
- Additional complexity due to needing to update GPU values after changing the returned mutable object
- Performance hit from added run-time checking
Metadata
Metadata
Assignees
Labels
Type
Projects
Status