From bd9989b08abdeb09a3db4f23258d4fd3ee568674 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Wed, 27 Dec 2023 10:37:13 +0100 Subject: [PATCH] LibWeb: Don't pass StringView to RecordingPainter, to avoid copy Instead, we now pass String if we have one. In particular, this fixes an issue where image elements with a data: URL src would copy the entire URL string every time we painted (before the image had been decoded). This was very noticeable on "fully downloaded" web pages where every single image has been turned into a data: URL. --- Userland/Libraries/LibWeb/Painting/ImagePaintable.cpp | 4 ++-- Userland/Libraries/LibWeb/Painting/MarkerPaintable.cpp | 2 +- Userland/Libraries/LibWeb/Painting/PaintableBox.cpp | 2 +- Userland/Libraries/LibWeb/Painting/RecordingPainter.cpp | 8 ++++---- Userland/Libraries/LibWeb/Painting/RecordingPainter.h | 4 ++-- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/Userland/Libraries/LibWeb/Painting/ImagePaintable.cpp b/Userland/Libraries/LibWeb/Painting/ImagePaintable.cpp index 1ab0b9a06a..1b75d56d5e 100644 --- a/Userland/Libraries/LibWeb/Painting/ImagePaintable.cpp +++ b/Userland/Libraries/LibWeb/Painting/ImagePaintable.cpp @@ -57,9 +57,9 @@ void ImagePaintable::paint(PaintContext& context, PaintPhase phase) const auto enclosing_rect = context.enclosing_device_rect(absolute_rect()).to_type(); context.recording_painter().set_font(Platform::FontPlugin::the().default_font()); context.recording_painter().paint_frame(enclosing_rect, context.palette(), Gfx::FrameStyle::SunkenContainer); - auto alt = image_element.alt(); + auto alt = image_element.get_attribute_value(HTML::AttributeNames::alt); if (alt.is_empty()) - alt = image_element.src(); + alt = image_element.get_attribute_value(HTML::AttributeNames::src); context.recording_painter().draw_text(enclosing_rect, alt, Gfx::TextAlignment::Center, computed_values().color(), Gfx::TextElision::Right); } else if (auto bitmap = layout_box().image_provider().current_image_bitmap(image_rect.size().to_type())) { ScopedCornerRadiusClip corner_clip { context, image_rect, normalized_border_radii_data(ShrinkRadiiForBorders::Yes) }; diff --git a/Userland/Libraries/LibWeb/Painting/MarkerPaintable.cpp b/Userland/Libraries/LibWeb/Painting/MarkerPaintable.cpp index 978dd3f45d..2edae6e3b1 100644 --- a/Userland/Libraries/LibWeb/Painting/MarkerPaintable.cpp +++ b/Userland/Libraries/LibWeb/Painting/MarkerPaintable.cpp @@ -117,7 +117,7 @@ void MarkerPaintable::paint(PaintContext& context, PaintPhase phase) const break; // FIXME: This should use proper text layout logic! // This does not line up with the text in the
  • element which looks very sad :( - context.recording_painter().draw_text(device_enclosing.to_type(), *text, layout_box().scaled_font(context), Gfx::TextAlignment::Center, color); + context.recording_painter().draw_text(device_enclosing.to_type(), MUST(String::from_byte_string(*text)), layout_box().scaled_font(context), Gfx::TextAlignment::Center, color); break; } case CSS::ListStyleType::None: diff --git a/Userland/Libraries/LibWeb/Painting/PaintableBox.cpp b/Userland/Libraries/LibWeb/Painting/PaintableBox.cpp index 51922db177..b160082ec0 100644 --- a/Userland/Libraries/LibWeb/Painting/PaintableBox.cpp +++ b/Userland/Libraries/LibWeb/Painting/PaintableBox.cpp @@ -281,7 +281,7 @@ void PaintableBox::paint(PaintContext& context, PaintPhase phase) const else builder.append(layout_box().debug_description()); builder.appendff(" {}x{} @ {},{}", border_rect.width(), border_rect.height(), border_rect.x(), border_rect.y()); - auto size_text = builder.to_byte_string(); + auto size_text = MUST(builder.to_string()); 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()); diff --git a/Userland/Libraries/LibWeb/Painting/RecordingPainter.cpp b/Userland/Libraries/LibWeb/Painting/RecordingPainter.cpp index 1b9bc259d0..871aa52d85 100644 --- a/Userland/Libraries/LibWeb/Painting/RecordingPainter.cpp +++ b/Userland/Libraries/LibWeb/Painting/RecordingPainter.cpp @@ -183,11 +183,11 @@ void RecordingPainter::draw_line(Gfx::IntPoint from, Gfx::IntPoint to, Color col }); } -void RecordingPainter::draw_text(Gfx::IntRect const& rect, StringView raw_text, Gfx::TextAlignment alignment, Color color, Gfx::TextElision elision, Gfx::TextWrapping wrapping) +void RecordingPainter::draw_text(Gfx::IntRect const& rect, String raw_text, Gfx::TextAlignment alignment, Color color, Gfx::TextElision elision, Gfx::TextWrapping wrapping) { push_command(DrawText { .rect = state().translation.map(rect), - .raw_text = String::from_utf8(raw_text).release_value_but_fixme_should_propagate_errors(), + .raw_text = move(raw_text), .alignment = alignment, .color = color, .elision = elision, @@ -195,11 +195,11 @@ void RecordingPainter::draw_text(Gfx::IntRect const& rect, StringView raw_text, }); } -void RecordingPainter::draw_text(Gfx::IntRect const& rect, StringView raw_text, Gfx::Font const& font, Gfx::TextAlignment alignment, Color color, Gfx::TextElision elision, Gfx::TextWrapping wrapping) +void RecordingPainter::draw_text(Gfx::IntRect const& rect, String raw_text, Gfx::Font const& font, Gfx::TextAlignment alignment, Color color, Gfx::TextElision elision, Gfx::TextWrapping wrapping) { push_command(DrawText { .rect = state().translation.map(rect), - .raw_text = String::from_utf8(raw_text).release_value_but_fixme_should_propagate_errors(), + .raw_text = move(raw_text), .alignment = alignment, .color = color, .elision = elision, diff --git a/Userland/Libraries/LibWeb/Painting/RecordingPainter.h b/Userland/Libraries/LibWeb/Painting/RecordingPainter.h index a26b808a19..937898d701 100644 --- a/Userland/Libraries/LibWeb/Painting/RecordingPainter.h +++ b/Userland/Libraries/LibWeb/Painting/RecordingPainter.h @@ -443,8 +443,8 @@ public: void draw_line(Gfx::IntPoint from, Gfx::IntPoint to, Color color, int thickness = 1, Gfx::Painter::LineStyle style = Gfx::Painter::LineStyle::Solid, Color alternate_color = Color::Transparent); - void draw_text(Gfx::IntRect const&, StringView, Gfx::Font const&, Gfx::TextAlignment = Gfx::TextAlignment::TopLeft, Color = Color::Black, Gfx::TextElision = Gfx::TextElision::None, Gfx::TextWrapping = Gfx::TextWrapping::DontWrap); - void draw_text(Gfx::IntRect const& rect, StringView raw_text, Gfx::TextAlignment alignment = Gfx::TextAlignment::TopLeft, Color color = Color::Black, Gfx::TextElision elision = Gfx::TextElision::None, Gfx::TextWrapping wrapping = Gfx::TextWrapping::DontWrap); + void draw_text(Gfx::IntRect const&, String, Gfx::Font const&, Gfx::TextAlignment = Gfx::TextAlignment::TopLeft, Color = Color::Black, Gfx::TextElision = Gfx::TextElision::None, Gfx::TextWrapping = Gfx::TextWrapping::DontWrap); + void draw_text(Gfx::IntRect const& rect, String raw_text, Gfx::TextAlignment alignment = Gfx::TextAlignment::TopLeft, Color color = Color::Black, Gfx::TextElision elision = Gfx::TextElision::None, Gfx::TextWrapping wrapping = Gfx::TextWrapping::DontWrap); void draw_signed_distance_field(Gfx::IntRect const& dst_rect, Color color, Gfx::GrayscaleBitmap const& sdf, float smoothing);