Browse Source

LibGfx: Fix crash during rasterizing glyphs containing only one point

Fixes https://github.com/SerenityOS/serenity/issues/20179
Aliaksandr Kalenik 1 năm trước cách đây
mục cha
commit
c2eaa0eb1c

+ 19 - 0
Tests/LibGfx/TestFontHandling.cpp

@@ -9,6 +9,7 @@
 #include <LibCore/ResourceImplementationFile.h>
 #include <LibGfx/Font/BitmapFont.h>
 #include <LibGfx/Font/FontDatabase.h>
+#include <LibGfx/Font/OpenType/Glyf.h>
 #include <LibTest/TestCase.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -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<u8> 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<OpenType::Glyf::Glyph> { VERIFY_NOT_REACHED(); });
+        return Test::Crash::Failure::DidNotCrash;
+    });
+}

+ 25 - 46
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<Gfx::FloatPoint> start_off_curve_point;
-        Optional<Gfx::FloatPoint> start_on_curve_point;
-        Optional<Gfx::FloatPoint> unprocessed_off_curve_point;
+
+        Vector<PointIterator::Item> 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 (points.is_empty())
+            continue;
 
-                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;
-                }
+        auto current = points.last();
+        auto next = points.first();
 
-                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 (current.on_curve) {
+            path.move_to(current.point);
+        } else if (next.on_curve) {
+            path.move_to(next.point);
+        } else {
+            auto implied_point = (current.point + next.point) * 0.5f;
+            path.move_to(implied_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);
+        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);
             }
-
-            // 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());
-        } else {
-            path.line_to(start_on_curve_point.value());
         }
     }