Skip to content

Commit

Permalink
LibWeb+LibGfx: Serialize HTML canvas fill/strokeStyle colors correctly
Browse files Browse the repository at this point in the history
Before this change we were serializing them in a bogus 8-digit hex color
format that isn't actually recognized by HTML.

This code will need more work when we start supporting color spaces
other than sRGB.

(cherry picked from commit 4590c081c2eb257232463ed660498a02f9bbe7b8;
amended expected output because serenity does not yet have
LadybirdBrowser/ladybird#1603)
  • Loading branch information
awesomekling authored and nico committed Nov 17, 2024
1 parent 4597138 commit 0b57196
Show file tree
Hide file tree
Showing 7 changed files with 58 additions and 9 deletions.
5 changes: 5 additions & 0 deletions Tests/LibWeb/Text/expected/canvas/fillStyle-serialization.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
`green` -> `#008000`
`rgba(128, 128, 128, 0.4)` -> `rgba(128, 128, 128, 0.4)`
`rgba(128, 128, 128, 0)` -> `rgba(128, 128, 128, 0)`
`rgba(128, 128, 128, 1)` -> `#808080`
`rgb(128, 128, 128)` -> `#808080`
10 changes: 5 additions & 5 deletions Tests/LibWeb/Text/expected/canvas/fillstyle.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
1. "#00ff00ff"
2. "#ff0000ff"
3. "#0000ffff"
4. "#00ff00ff"
5. "#ff0000ff"
1. "#00ff00"
2. "#ff0000"
3. "#0000ff"
4. "#00ff00"
5. "#ff0000"
18 changes: 18 additions & 0 deletions Tests/LibWeb/Text/input/canvas/fillStyle-serialization.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<script src="../include.js"></script>
<canvas id="c" width=300 height=300></canvas>
<script>
test(() => {
let x = c.getContext("2d");

function go(color) {
x.fillStyle = color;
println("`" + color + "` -> `" + x.fillStyle + "`");
}

go("green");
go("rgba(128, 128, 128, 0.4)");
go("rgba(128, 128, 128, 0)");
go("rgba(128, 128, 128, 1)");
go("rgb(128, 128, 128)");
});
</script>
24 changes: 22 additions & 2 deletions Userland/Libraries/LibGfx/Color.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,29 @@

namespace Gfx {

String Color::to_string() const
String Color::to_string(HTMLCompatibleSerialization html_compatible_serialization) const
{
return MUST(String::formatted("#{:02x}{:02x}{:02x}{:02x}", red(), green(), blue(), alpha()));
// If the following conditions are all true:

// 1. The color space is sRGB
// NOTE: This is currently always true for Gfx::Color.

// 2. The alpha is 1
// NOTE: An alpha value of 1 will be stored as 255 currently.

// 3. The RGB component values are internally represented as integers between 0 and 255 inclusive (i.e. 8-bit unsigned integer)
// NOTE: This is currently always true for Gfx::Color.

// 4. HTML-compatible serialization is requested
if (alpha() == 255
&& html_compatible_serialization == HTMLCompatibleSerialization::Yes) {
return MUST(String::formatted("#{:02x}{:02x}{:02x}", red(), green(), blue()));
}

// Otherwise, for sRGB the CSS serialization of sRGB values is used and for other color spaces, the relevant serialization of the <color> value.
if (alpha() < 255)
return MUST(String::formatted("rgba({}, {}, {}, {})", red(), green(), blue(), alpha() / 255.0));
return MUST(String::formatted("rgb({}, {}, {})", red(), green(), blue()));
}

String Color::to_string_without_alpha() const
Expand Down
7 changes: 6 additions & 1 deletion Userland/Libraries/LibGfx/Color.h
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,12 @@ class Color {
return m_value == other.m_value;
}

String to_string() const;
enum class HTMLCompatibleSerialization {
No,
Yes,
};

[[nodiscard]] String to_string(HTMLCompatibleSerialization = HTMLCompatibleSerialization::No) const;
String to_string_without_alpha() const;

ByteString to_byte_string() const;
Expand Down
1 change: 1 addition & 0 deletions Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2850,6 +2850,7 @@ RefPtr<CSSStyleValue> Parser::parse_rgb_color_value(TokenStream<ComponentValue>&
inner_tokens.skip_whitespace();

alpha = parse_number_percentage_value(inner_tokens);

inner_tokens.skip_whitespace();

if (inner_tokens.has_next_token())
Expand Down
2 changes: 1 addition & 1 deletion Userland/Libraries/LibWeb/HTML/Canvas/CanvasState.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ class CanvasState {
{
return m_fill_or_stroke_style.visit(
[&](Gfx::Color color) -> JsFillOrStrokeStyle {
return color.to_string();
return color.to_string(Gfx::Color::HTMLCompatibleSerialization::Yes);
},
[&](auto handle) -> JsFillOrStrokeStyle {
return handle;
Expand Down

0 comments on commit 0b57196

Please sign in to comment.