RGBCube/serenity
1
Fork 0
mirror of https://github.com/RGBCube/serenity synced 2025-07-27 23:57:44 +00:00
Code Issues Activity Actions

LibWeb: Remove implicit conversion from float and double to CSSPixels

Browse source 
In general it is not safe to convert any arbitrary floating-point value
to CSSPixels. CSSPixels has a resolution of 0.015625, which for small
values (e.g. scale factors between 0 and 1), can produce bad results
if converted to CSSPixels then scaled back up. In the worst case values
can underflow to zero and produce incorrect results.
This commit is contained in:
MacDue 2023-08-26 15:03:04 +01:00 • committed by Alexander Kalenik
parent 0f9c088302
commit 360c0eb509
43 changed files with 248 additions and 221 deletions
Download patch file Download diff file Expand all files Collapse all files

8
Userland/Libraries/LibWeb/Painting/BackgroundPainting.cpp
View file

@ -161,14 +161,14 @@ void paint_background(PaintContext& context, Layout::NodeWithStyleAndBoxModelMet
double max_width_ratio = (background_positioning_area.width() / natural_image_width).to_double();
double max_height_ratio = (background_positioning_area.height() / natural_image_height).to_double();
double ratio = min(max_width_ratio, max_height_ratio);
image_rect.set_size(natural_image_width * ratio, natural_image_height * ratio);
image_rect.set_size(natural_image_width.scaled(ratio), natural_image_height.scaled(ratio));
break;
}
case CSS::BackgroundSize::Cover: {
double max_width_ratio = (background_positioning_area.width() / natural_image_width).to_double();
double max_height_ratio = (background_positioning_area.height() / natural_image_height).to_double();
double ratio = max(max_width_ratio, max_height_ratio);
image_rect.set_size(natural_image_width * ratio, natural_image_height * ratio);
image_rect.set_size(natural_image_width.scaled(ratio), natural_image_height.scaled(ratio));
break;
}
case CSS::BackgroundSize::LengthPercentage: {
@ -263,7 +263,7 @@ void paint_background(PaintContext& context, Layout::NodeWithStyleAndBoxModelMet
repeat_x = false;
} else {
auto space = fmod(background_positioning_area.width().to_double(), image_rect.width().to_double());
x_step = image_rect.width() + (space / static_cast<double>(whole_images - 1));
x_step = image_rect.width() + CSSPixels(space / static_cast<double>(whole_images - 1));
repeat_x = true;
}
break;
@ -294,7 +294,7 @@ void paint_background(PaintContext& context, Layout::NodeWithStyleAndBoxModelMet
repeat_y = false;
} else {
auto space = fmod(background_positioning_area.height().to_float(), image_rect.height().to_float());
y_step = image_rect.height() + (static_cast<double>(space) / static_cast<double>(whole_images - 1));
y_step = image_rect.height() + CSSPixels(static_cast<double>(space) / static_cast<double>(whole_images - 1));
repeat_y = true;
}
break;

2
Userland/Libraries/LibWeb/Painting/GradientPainting.cpp
View file

@ -117,7 +117,7 @@ LinearGradientData resolve_linear_gradient_data(Layout::NodeWithStyleAndBoxModel
auto resolved_color_stops = resolve_color_stop_positions(
node, linear_gradient.color_stop_list(), [&](auto const& length_percentage) {
return length_percentage.to_px(node, gradient_length_px).to_float() / static_cast<float>(gradient_length_px);
return length_percentage.to_px(node, CSSPixels(gradient_length_px)).to_float() / static_cast<float>(gradient_length_px);
},
linear_gradient.is_repeating());

2
Userland/Libraries/LibWeb/Painting/MarkerPaintable.cpp
View file

@ -39,7 +39,7 @@ void MarkerPaintable::paint(PaintContext& context, PaintPhase phase) const
CSSPixelRect enclosing = absolute_rect().to_rounded<CSSPixels>();
auto device_enclosing = context.enclosing_device_rect(enclosing);
CSSPixels marker_width = enclosing.height() / 2.0;
CSSPixels marker_width = enclosing.height() / 2;
if (auto const* list_style_image = layout_box().list_style_image()) {
CSSPixelRect image_rect {

2
Userland/Libraries/LibWeb/Painting/PaintContext.cpp
View file

@ -96,7 +96,7 @@ DevicePixelSize PaintContext::rounded_device_size(CSSPixelSize size) const
CSSPixels PaintContext::scale_to_css_pixels(DevicePixels device_pixels) const
{
return device_pixels.value() / m_device_pixels_per_css_pixel;
return CSSPixels(device_pixels.value() / m_device_pixels_per_css_pixel);
}
CSSPixelPoint PaintContext::scale_to_css_point(DevicePixelPoint point) const

16
Userland/Libraries/LibWeb/Painting/PaintableBox.cpp
View file

@ -258,8 +258,8 @@ void PaintableBox::paint(PaintContext& context, PaintPhase phase) const
auto size_text_rect = border_rect;
size_text_rect.set_y(border_rect.y() + border_rect.height());
size_text_rect.set_top(size_text_rect.top());
size_text_rect.set_width((float)font.width(size_text) + 4);
size_text_rect.set_height(font.pixel_size() + 4);
size_text_rect.set_width(CSSPixels(font.width(size_text)) + 4);
size_text_rect.set_height(CSSPixels(font.pixel_size()) + 4);
auto size_text_device_rect = context.enclosing_device_rect(size_text_rect).to_type<int>();
context.painter().fill_rect(size_text_device_rect, context.palette().color(Gfx::ColorRole::Tooltip));
context.painter().draw_rect(size_text_device_rect, context.palette().threed_shadow1());
@ -503,7 +503,7 @@ static void paint_cursor_if_needed(PaintContext& context, Layout::TextNode const
auto fragment_rect = fragment.absolute_rect();
CSSPixelRect cursor_rect {
fragment_rect.x() + text_node.font().width(fragment.text().substring_view(0, text_node.browsing_context().cursor_position().offset() - fragment.start())),
fragment_rect.x() + CSSPixels(text_node.font().width(fragment.text().substring_view(0, text_node.browsing_context().cursor_position().offset() - fragment.start()))),
fragment_rect.top(),
1,
fragment_rect.height()
@ -518,7 +518,7 @@ static void paint_text_decoration(PaintContext& context, Gfx::Painter& painter,
{
auto& font = fragment.layout_node().font();
auto fragment_box = fragment.absolute_rect();
CSSPixels glyph_height = font.pixel_size();
CSSPixels glyph_height = CSSPixels(font.pixel_size());
auto baseline = fragment_box.height() / 2 - (glyph_height + 4) / 2 + glyph_height;
auto line_color = text_node.computed_values().text_decoration_color();
@ -526,9 +526,9 @@ static void paint_text_decoration(PaintContext& context, Gfx::Painter& painter,
CSSPixels css_line_thickness = [&] {
CSS::Length computed_thickness = text_node.computed_values().text_decoration_thickness().resolved(text_node, CSS::Length(1, CSS::Length::Type::Em));
if (computed_thickness.is_auto())
return max(glyph_height * 0.1, 1.);
return max(glyph_height.scaled(0.1), 1);
return computed_thickness.to_px(text_node).to_double();
return computed_thickness.to_px(text_node);
}();
auto device_line_thickness = context.rounded_device_pixels(css_line_thickness);
@ -550,8 +550,8 @@ static void paint_text_decoration(PaintContext& context, Gfx::Painter& painter,
break;
case CSS::TextDecorationLine::LineThrough: {
auto x_height = font.x_height();
line_start_point = context.rounded_device_point(fragment_box.top_left().translated(0, baseline - x_height * 0.5f));
line_end_point = context.rounded_device_point(fragment_box.top_right().translated(-1, baseline - x_height * 0.5f));
line_start_point = context.rounded_device_point(fragment_box.top_left().translated(0, baseline - x_height * CSSPixels(0.5f)));
line_end_point = context.rounded_device_point(fragment_box.top_right().translated(-1, baseline - x_height * CSSPixels(0.5f)));
break;
}
case CSS::TextDecorationLine::Blink:

6
Userland/Libraries/LibWeb/Painting/StackingContext.cpp
View file

@ -448,12 +448,12 @@ void StackingContext::paint(PaintContext& context) const
// to the size of the source (which could add some artefacts, though just scaling the bitmap already does that).
// We need to copy the background at the destination because a bunch of our rendering effects now rely on
// being able to sample the painter (see border radii, shadows, filters, etc).
CSSPixelPoint destination_clipped_fixup {};
Gfx::FloatPoint destination_clipped_fixup {};
auto try_get_scaled_destination_bitmap = [&]() -> ErrorOr<NonnullRefPtr<Gfx::Bitmap>> {
Gfx::IntRect actual_destination_rect;
auto bitmap = TRY(context.painter().get_region_bitmap(destination_rect, Gfx::BitmapFormat::BGRA8888, actual_destination_rect));
// get_region_bitmap() may clip to a smaller region if the requested rect goes outside the painter, so we need to account for that.
destination_clipped_fixup = CSSPixelPoint { destination_rect.location() - actual_destination_rect.location() };
destination_clipped_fixup = Gfx::FloatPoint { destination_rect.location() - actual_destination_rect.location() };
destination_rect = actual_destination_rect;
if (source_rect.size() != transformed_destination_rect.size()) {
auto sx = static_cast<float>(source_rect.width()) / transformed_destination_rect.width();
@ -469,7 +469,7 @@ void StackingContext::paint(PaintContext& context) const
return;
auto bitmap = bitmap_or_error.release_value_but_fixme_should_propagate_errors();
Gfx::Painter painter(bitmap);
painter.translate(context.rounded_device_point(-paintable_box().absolute_paint_rect().location() + destination_clipped_fixup).to_type<int>());
painter.translate(context.rounded_device_point(-paintable_box().absolute_paint_rect().location() + destination_clipped_fixup.to_type<CSSPixels>()).to_type<int>());
auto paint_context = context.clone(painter);
paint_internal(paint_context);