From c2eaa0eb1c3bb4d65f4054a5dd2f1a69d3a84638 Mon Sep 17 00:00:00 2001 From: Aliaksandr Kalenik Date: Tue, 17 Oct 2023 22:12:58 +0200 Subject: [PATCH] LibGfx: Fix crash during rasterizing glyphs containing only one point Fixes https://github.com/SerenityOS/serenity/issues/20179 --- Tests/LibGfx/TestFontHandling.cpp | 19 +++++ .../Libraries/LibGfx/Font/OpenType/Glyf.cpp | 75 +++++++------------ 2 files changed, 46 insertions(+), 48 deletions(-) diff --git a/Tests/LibGfx/TestFontHandling.cpp b/Tests/LibGfx/TestFontHandling.cpp index dfb3943b64..95daebbbfb 100644 --- a/Tests/LibGfx/TestFontHandling.cpp +++ b/Tests/LibGfx/TestFontHandling.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -172,3 +173,21 @@ TEST_CASE(test_character_set_masking) EXPECT(!masked_font->glyph_index(0x0100).has_value()); EXPECT(masked_font->glyph_index(0xFFFD).value() == 0x1FD); } + +TEST_CASE(rasterize_glyph_containing_single_off_curve_point) +{ + Vector glyph_data { + 0, 5, 0, 205, 255, 51, 7, 51, 6, 225, 0, 3, 0, 6, 0, 9, 0, 12, 0, 15, 0, 31, 64, 13, 13, 2, 15, 5, 7, 2, 8, 5, 10, 3, 0, + 5, 3, 0, 47, 47, 51, 17, 51, 17, 51, 17, 51, 17, 51, 17, 51, 48, 49, 19, 33, 17, 33, 1, 33, 1, 1, 17, 1, 1, 33, 9, 3, + 205, 6, 102, 249, 154, 5, 184, 250, 248, 2, 133, 2, 199, 253, 125, 253, 57, 5, 4, 253, 127, 253, 53, 2, 133, 253, + 123, 6, 225, 248, 82, 7, 68, 252, 231, 252, 145, 6, 50, 252, 231, 252, 149, 3, 23, 253, 57, 3, 27, 3, 29, 0, 1, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 6, 177, 2, 81, 43, 48, 49, 48, 0 + }; + OpenType::Glyf glyf(glyph_data.span()); + auto glyph = glyf.glyph(118); + EXPECT(glyph.has_value()); + EXPECT_NO_CRASH("rasterizing glyph containing single off-curve point should not crash", [&] { + (void)glyph->rasterize(0, 0, 1, 1, {}, [&](u16) -> Optional { VERIFY_NOT_REACHED(); }); + return Test::Crash::Failure::DidNotCrash; + }); +} diff --git a/Userland/Libraries/LibGfx/Font/OpenType/Glyf.cpp b/Userland/Libraries/LibGfx/Font/OpenType/Glyf.cpp index e658496568..76847d902f 100644 --- a/Userland/Libraries/LibGfx/Font/OpenType/Glyf.cpp +++ b/Userland/Libraries/LibGfx/Font/OpenType/Glyf.cpp @@ -262,60 +262,39 @@ void Glyf::Glyph::rasterize_impl(Gfx::Painter& painter, Gfx::AffineTransform con u32 current_point_index = 0; for (u16 contour_index = 0; contour_index < m_num_contours; contour_index++) { u32 current_contour_last_point_index = be_u16(m_slice.offset(contour_index * 2)); - Optional start_off_curve_point; - Optional start_on_curve_point; - Optional unprocessed_off_curve_point; + + Vector points; while (current_point_index <= current_contour_last_point_index) { - auto current_point = point_iterator.next(); + points.append(*point_iterator.next()); current_point_index++; - if (!current_point.has_value()) - break; - - if (current_point->on_curve) { - if (!start_on_curve_point.has_value()) { - start_on_curve_point = current_point->point; - path.move_to(current_point->point); - } - - if (unprocessed_off_curve_point.has_value()) { - path.quadratic_bezier_curve_to(unprocessed_off_curve_point.value(), current_point->point); - unprocessed_off_curve_point = {}; - } else { - path.line_to(current_point->point); - } - } else { - if (!start_on_curve_point.has_value() && !start_off_curve_point.has_value()) { - // If "off curve" point comes first it needs to be saved to use while closing the path - start_off_curve_point = current_point->point; - } - - if (unprocessed_off_curve_point.has_value()) { - // Two subsequent "off curve" points create implied "on-curve" point lying between them - auto implied_point = (unprocessed_off_curve_point.value() + current_point->point) * 0.5f; - if (!start_on_curve_point.has_value()) { - start_on_curve_point = implied_point; - path.move_to(implied_point); - } - path.quadratic_bezier_curve_to(unprocessed_off_curve_point.value(), implied_point); - } - unprocessed_off_curve_point = current_point->point; - } } - if (start_off_curve_point.has_value()) { - // Close the path creating "implied" point if both first and last points were "off curve" - if (unprocessed_off_curve_point.has_value()) { - auto implied_point = (start_off_curve_point.value() + unprocessed_off_curve_point.value()) * 0.5f; - path.quadratic_bezier_curve_to(unprocessed_off_curve_point.value(), implied_point); - } + if (points.is_empty()) + continue; - // Add bezier curve from new "implied point" to first "on curve" point in the path - path.quadratic_bezier_curve_to(start_off_curve_point.value(), start_on_curve_point.value()); - } else if (unprocessed_off_curve_point.has_value()) { - // Add bezier curve to first "on curve" point using last "off curve" point - path.quadratic_bezier_curve_to(unprocessed_off_curve_point.value(), start_on_curve_point.value()); + auto current = points.last(); + auto next = points.first(); + + if (current.on_curve) { + path.move_to(current.point); + } else if (next.on_curve) { + path.move_to(next.point); } else { - path.line_to(start_on_curve_point.value()); + auto implied_point = (current.point + next.point) * 0.5f; + path.move_to(implied_point); + } + + for (size_t i = 0; i < points.size(); i++) { + current = next; + next = points[(i + 1) % points.size()]; + if (current.on_curve) { + path.line_to(current.point); + } else if (next.on_curve) { + path.quadratic_bezier_curve_to(current.point, next.point); + } else { + auto implied_point = (current.point + next.point) * 0.5f; + path.quadratic_bezier_curve_to(current.point, implied_point); + } } }