serenity icon indicating copy to clipboard operation
serenity copied to clipboard

LibWeb: Support CSS style `background-clip: text`

Open zacbrannelly opened this issue 1 year ago • 7 comments

Support for the background-clip: text CSS property, currently only supports the CPU renderer, will setup a machine that supports the GPU renderer (I'm on a Mac M1) and implement in either this PR or a subsequent one. Also probably have to fix a bunch of linting errors since I haven't got that setup on my machine properly.

zacbrannelly avatar Feb 27 '24 15:02 zacbrannelly

Hi there! If you could split the LibGfx changes into their own commit before the LibWeb commit, that would be appreciated.

AtkinsSJ avatar Feb 27 '24 17:02 AtkinsSJ

also some ref-tests would be much appreciated. you can find examples of the once we already have here: https://github.com/SerenityOS/serenity/tree/master/Tests/LibWeb/Ref

kalenikaliaksandr avatar Feb 27 '24 18:02 kalenikaliaksandr

I took another look at the spec for the definition of how text clip should be handled https://drafts.csswg.org/css-backgrounds-4/#background-clip

The background is painted within (clipped to) the intersection of the border box and the geometry of the text in the element and its in-flow and floated descendants

so what if we tried to implement in the following way?

  • if element has background-clip: text then we calculate intersection of its border box and all glyphs from descendants. I think we can start by collecting a vector of Gfx::Path for all descendant glyphs. this way we won't need to literally calculate the intersection.
  • vector with all glyph paths that are supposed to used for masking is saved in DrawScaledImmutableBitmap command that does background painting
  • executor is updated to apply a mask of DrawScaledImmutableBitmap command: if vector with glyph paths that are supposed to be used for a mask is not empty then we use them to paint a separate bitmap with a mask that is applied to background's bitmap.

this way we can completely get rid of alpha mask ids, push/pop mask states, having separate painters for background and glyph commands. wouldn't it be cool to make way simpler? 🤓

kalenikaliaksandr avatar Feb 29 '24 08:02 kalenikaliaksandr

I took another look at the spec for the definition of how text clip should be handled https://drafts.csswg.org/css-backgrounds-4/#background-clip

The background is painted within (clipped to) the intersection of the border box and the geometry of the text in the element and its in-flow and floated descendants

so what if we tried to implement in the following way?

  • if element has background-clip: text then we calculate intersection of its border box and all glyphs from descendants. I think we can start by collecting a vector of Gfx::Path for all descendant glyphs. this way we won't need to literally calculate the intersection.

  • vector with all glyph paths that are supposed to used for masking is saved in DrawScaledImmutableBitmap command that does background painting

  • executor is updated to apply a mask of DrawScaledImmutableBitmap command: if vector with glyph paths that are supposed to be used for a mask is not empty then we use them to paint a separate bitmap with a mask that is applied to background's bitmap.

this way we can completely get rid of alpha mask ids, push/pop mask states, having separate painters for background and glyph commands. wouldn't it be cool to make way simpler? 🤓

Sounds much better! Only additional thing is the fill rect command will also need the glyph paths since it's not always an image that is drawn for the background. Will give it a shot when I've got some more time, Thanks heaps! :)

zacbrannelly avatar Feb 29 '24 10:02 zacbrannelly

@kalenikaliaksandr I've implemented your suggestion with just the FillRectWithRoundedCorners command to test it out and running into an issue where the text isn't lining up between the background and the foreground text: Screenshot 2024-03-02 at 4 04 43 pm

Where the red colour is the background thats clipped via Gfx::Path's, and the green text is the foreground text being painted on top of the background.

This is how I'm collecting the paths:

        auto add_text_clip_path = [&](Layout::TextNode const& text_node, PaintableFragment const& fragment) {
            // Draw the glyphs to an instance of Gfx::Path.
            auto text = text_node.text_for_rendering();

            // Scale to the device pixels.
            Gfx::Path glyph_run_path;
            for (auto glyph : fragment.glyph_run()) {
                glyph.visit([&](auto& glyph) {
                    glyph.font = *glyph.font->with_size(glyph.font->point_size() * static_cast<float>(context.device_pixels_per_css_pixel()));
                    glyph.position = glyph.position.scaled(context.device_pixels_per_css_pixel());
                });

                if (glyph.has<Gfx::DrawGlyph>()) {
                    auto const& draw_glyph = glyph.get<Gfx::DrawGlyph>();

                    // Get the path for the glyph.
                    Gfx::Path glyph_path;
                    auto const& scaled_font = static_cast<Gfx::ScaledFont const&>(*draw_glyph.font);
                    auto glyph_id = scaled_font.glyph_id_for_code_point(draw_glyph.code_point);
                    scaled_font.append_glyph_path_to(glyph_path, glyph_id);

                    // Transform the path to the fragment's position.
                    auto transform = Gfx::AffineTransform {}.translate(draw_glyph.position);
                    glyph_run_path.append_path(glyph_path.copy_transformed(transform));
                }
            }

            // Calculate the baseline start position.
            auto fragment_absolute_rect = fragment.absolute_rect();
            auto fragment_absolute_device_rect = context.enclosing_device_rect(fragment_absolute_rect);
            DevicePixelPoint baseline_start { fragment_absolute_device_rect.x(), fragment_absolute_device_rect.y() + context.rounded_device_pixels(fragment.baseline()) };

            // Add the path to text_clip_paths.
            auto transform = Gfx::AffineTransform {}.translate(baseline_start.to_type<int>().to_type<float>());
            text_clip_paths.append(glyph_run_path.copy_transformed(transform));
        };

And this is how it is clipped in the executor:

template<typename Callback>
void apply_clip_paths_to_painter(Gfx::IntRect const& rect, Callback callback, Vector<Gfx::Path> const& clip_paths, Gfx::Painter& target_painter)
{
    // Setup a painter for a background canvas that we will paint to first.
    auto background_canvas = Gfx::Bitmap::create(Gfx::BitmapFormat::BGRA8888, rect.size()).release_value_but_fixme_should_propagate_errors();
    Gfx::Painter painter(*background_canvas);

    // Offset the painter to paint in the correct location.
    painter.translate(-rect.location());

    // Paint the background canvas.
    callback(painter);

    // Apply the clip path to the target painter.
    Gfx::AntiAliasingPainter aa_painter(target_painter);
    for (auto const& clip_path : clip_paths) {
        auto fill_offset = clip_path.bounding_box().location().to_type<int>() - rect.location();
        auto paint_style = Gfx::BitmapPaintStyle::create(*background_canvas, fill_offset).release_value_but_fixme_should_propagate_errors();
        aa_painter.fill_path(clip_path, paint_style);
    }
}

CommandResult CommandExecutorCPU::fill_rect_with_rounded_corners(Gfx::IntRect const& rect, Color const& color, Gfx::AntiAliasingPainter::CornerRadius const& top_left_radius, Gfx::AntiAliasingPainter::CornerRadius const& top_right_radius, Gfx::AntiAliasingPainter::CornerRadius const& bottom_left_radius, Gfx::AntiAliasingPainter::CornerRadius const& bottom_right_radius, Vector<Gfx::Path> const& clip_paths)
{
    auto paint_op = [&](Gfx::Painter& painter) {
        Gfx::AntiAliasingPainter aa_painter(painter);
        aa_painter.fill_rect_with_rounded_corners(
            rect,
            color,
            top_left_radius,
            top_right_radius,
            bottom_right_radius,
            bottom_left_radius);
    };
    if (clip_paths.is_empty()) {
        paint_op(painter());
    } else {
        apply_clip_paths_to_painter(rect, paint_op, clip_paths, painter());
    }
    return CommandResult::Continue;
}

Any idea's where I'm going wrong?

zacbrannelly avatar Mar 02 '24 05:03 zacbrannelly

I think I've fixed it by doing:

// Transform the path to the fragment's position.
auto top_left = draw_glyph.position + Gfx::FloatPoint(scaled_font.glyph_left_bearing(draw_glyph.code_point), 0);
auto glyph_position = Gfx::GlyphRasterPosition::get_nearest_fit_for(top_left);
auto transform = Gfx::AffineTransform {}.translate(glyph_position.blit_position.to_type<float>());
glyph_run_path.append_path(glyph_path.copy_transformed(transform));
Screenshot 2024-03-02 at 4 35 49 pm

Still some of the background leaking through but not as bad as before.

zacbrannelly avatar Mar 02 '24 05:03 zacbrannelly

it seems that the rendering discrepancy comes from from the difference between rasterizing Gfx::Path "as is", and how Gfx::Painter accounts for the subpixel offset (we produce couple bitmaps for each glyph and use the best one, depending on the glyph's position).

https://github.com/SerenityOS/serenity/blob/master/Userland/Libraries/LibGfx/Painter.cpp#L1359-L1363

I think the easiest way to fix that would be to save array of (font, code_point) instead of Gfx::Path and use Painter::draw_glyph() so it is always the same code used for glyph painting. BUT, feel free to leave a FIXME, instead of doing that!

great progress btw! 🤓

kalenikaliaksandr avatar Mar 02 '24 10:03 kalenikaliaksandr

it seems that the rendering discrepancy comes from from the difference between rasterizing Gfx::Path "as is", and how Gfx::Painter accounts for the subpixel offset (we produce couple bitmaps for each glyph and use the best one, depending on the glyph's position).

https://github.com/SerenityOS/serenity/blob/master/Userland/Libraries/LibGfx/Painter.cpp#L1359-L1363

I think the easiest way to fix that would be to save array of (font, code_point) instead of Gfx::Path and use Painter::draw_glyph() so it is always the same code used for glyph painting. BUT, feel free to leave a FIXME, instead of doing that!

great progress btw! 🤓

Yeahh I've just added a FIXME for now :) I've added a test and fixed all the linting issues, so should be good for a proper look.

The only thing I'm concerned about is the performance, when you load the test case I've added and highlight some text it is noticeably slower at repainting. Trying to do some profiling to see where its slowing down the most.

zacbrannelly avatar Mar 03 '24 05:03 zacbrannelly