Stowage/ladybird
2
0
Fork 0
mirror of https://github.com/LadybirdBrowser/ladybird.git synced 2025-12-08 06:09:58 +00:00
Code Issues Projects Releases Packages Wiki Activity

Everywhere: Unify naming of RGBA-like colors

Browse source 
The `Bitmap` type was referring to to its internal pixel format by a
name that represents the order of the color components as they are layed
out in memory. Contrary, the `Color` type was using a naming that where
the name represents the order of the components from most to least
significant byte when viewed as a unsigned 32bit integer. This is
confusing as you have to keep remembering which mental model to use
depending on which code you work with.

To unify the two, the naming of RGBA-like colors in the `Color` type has
been adjusted to match the one from the Bitmap type. This seems to be
generally in line with how web APIs think about these types:
* `ImageData.pixelFormat` can be `rgba-8unorm` backed by a
  `Uint8ClamedArray`, but there is no pixel format backed by a 32bit
  unsigned type.
* WebGL can use format `RGBA` with type `UNSIGNED_BYTE`, but there is no
  such format with type `UNSIGNED_INT`.

Additionally, it appears that other browsers and browser-adjacent
libraries also think similarly about these types:
* Firefox:
  https://github.com/mozilla-firefox/firefox/blob/main/gfx/2d/Types.h
* WebKit:
  https://github.com/WebKit/WebKit/blob/main/Source/WebCore/platform/graphics/PixelFormat.h
* Skia:
  https://chromium.googlesource.com/skia/+/refs/heads/main/include/core/SkColorType.h

This has the not so nice side effect that APIs that interact with these
types through 32bit unsigned integers now have the component order
inverted due to little-endian byte order. E.g. specifying a color as hex
constant needs to be done as `0xAABBGGRR` if it is to be treated as
RGBA8888.

We could alleviate this by providing endian-independent APIs to callers.
But I suspect long-term we might want to think differently about bitmap
data anyway, e.g. to better support HDR in the future. However, such
changes would be more involved than just unifying the naming as done
here. So I considered that out of scope for now.
This commit is contained in:
InvalidUsernameException 2025-11-23 13:07:38 +01:00 • committed by Jelle Raaijmakers
parent 07f61f2cec
commit 7c315ef67f
Notes: github-actions[bot] 2025-11-28 17:34:55 +00:00
14 changed files with 130 additions and 128 deletions
Download patch file Download diff file Expand all files Collapse all files

2
Libraries/LibWeb/Painting/MediaPaintable.cpp
View file

@ -21,7 +21,7 @@
namespace Web::Painting {
static constexpr auto CONTROL_BOX_COLOR = Gfx::Color::from_rgb(0x26'26'26);
static constexpr auto CONTROL_BOX_COLOR = Gfx::Color::from_bgrx(0x26'26'26);
static constexpr auto CONTROL_BUTTON_COLOR = Gfx::Color::branded_color(Gfx::Color::BrandedColor::Violet);
static constexpr auto CONTROL_HIGHLIGHT_COLOR = Gfx::Color::branded_color(Gfx::Color::BrandedColor::Violet60);

6
Libraries/LibWeb/Painting/VideoPaintable.cpp
View file

@ -18,8 +18,8 @@
namespace Web::Painting {
static constexpr auto control_box_color = Gfx::Color::from_rgb(0x26'26'26);
static constexpr auto control_highlight_color = Gfx::Color::from_rgb(0x1d'99'f3);
static constexpr auto control_box_color = Gfx::Color::from_bgrx(0x26'26'26);
static constexpr auto control_highlight_color = Gfx::Color::from_bgrx(0x1d'99'f3);
GC_DEFINE_ALLOCATOR(VideoPaintable);
@ -134,7 +134,7 @@ void VideoPaintable::paint(DisplayListRecordingContext& context, PaintPhase phas
};
auto paint_transparent_black = [&]() {
static constexpr auto transparent_black = Gfx::Color::from_argb(0x00'00'00'00);
static constexpr auto transparent_black = Gfx::Color::from_bgra(0x00'00'00'00);
context.display_list_recorder().fill_rect(video_rect.to_type<int>(), transparent_black);
};