From dbe229eba711f4adf8bcaa9820afbaec06eb5fff Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Tue, 2 Sep 2025 16:19:36 -0700 Subject: [PATCH] [Impeller] Fix overdraw in DrawRect geometry (#174735) Fixes https://github.com/flutter/flutter/issues/174579 Vertex lists for stroked rects were fixed by being more careful about the creation of the vertices so that they have no inherent overdraw. These operations continue to use the fastest kNormal vertex mode but no longer create rendering artifacts due to multiple triangles covering the same area. --- .../display_list/aiks_dl_basic_unittests.cc | 69 +++++ .../impeller/entity/geometry/rect_geometry.cc | 249 +++++++++++++----- .../impeller/entity/geometry/rect_geometry.h | 5 - 3 files changed, 247 insertions(+), 76 deletions(-) diff --git a/engine/src/flutter/impeller/display_list/aiks_dl_basic_unittests.cc b/engine/src/flutter/impeller/display_list/aiks_dl_basic_unittests.cc index 31328e984d7..176efef6637 100644 --- a/engine/src/flutter/impeller/display_list/aiks_dl_basic_unittests.cc +++ b/engine/src/flutter/impeller/display_list/aiks_dl_basic_unittests.cc @@ -35,6 +35,75 @@ TEST_P(AiksTest, CanRenderColoredRect) { ASSERT_TRUE(OpenPlaygroundHere(builder.Build())); } +namespace { +using DrawRectProc = + std::function; + +sk_sp MakeWideStrokedRects(Point scale, + const DrawRectProc& draw_rect) { + DisplayListBuilder builder; + builder.Scale(scale.x, scale.y); + builder.DrawColor(DlColor::kWhite(), DlBlendMode::kSrc); + + DlPaint paint; + paint.setColor(DlColor::kBlue().withAlphaF(0.5)); + paint.setDrawStyle(DlDrawStyle::kStroke); + paint.setStrokeWidth(30.0f); + + // Each of these 3 sets of rects includes (with different join types): + // - One rectangle with a gap in the middle + // - One rectangle with no gap because it is too narrow + // - One rectangle with no gap because it is too short + paint.setStrokeJoin(DlStrokeJoin::kBevel); + draw_rect(builder, DlRect::MakeXYWH(100.0f, 100.0f, 100.0f, 100.0f), paint); + draw_rect(builder, DlRect::MakeXYWH(250.0f, 100.0f, 10.0f, 100.0f), paint); + draw_rect(builder, DlRect::MakeXYWH(100.0f, 250.0f, 100.0f, 10.0f), paint); + + paint.setStrokeJoin(DlStrokeJoin::kRound); + draw_rect(builder, DlRect::MakeXYWH(350.0f, 100.0f, 100.0f, 100.0f), paint); + draw_rect(builder, DlRect::MakeXYWH(500.0f, 100.0f, 10.0f, 100.0f), paint); + draw_rect(builder, DlRect::MakeXYWH(350.0f, 250.0f, 100.0f, 10.0f), paint); + + paint.setStrokeJoin(DlStrokeJoin::kMiter); + draw_rect(builder, DlRect::MakeXYWH(600.0f, 100.0f, 100.0f, 100.0f), paint); + draw_rect(builder, DlRect::MakeXYWH(750.0f, 100.0f, 10.0f, 100.0f), paint); + draw_rect(builder, DlRect::MakeXYWH(600.0f, 250.0f, 100.0f, 10.0f), paint); + + // And now draw 3 rectangles with a stroke width so large that that it + // overlaps in the middle in both directions (horizontal/vertical). + paint.setStrokeWidth(110.0f); + + paint.setStrokeJoin(DlStrokeJoin::kBevel); + draw_rect(builder, DlRect::MakeXYWH(100.0f, 400.0f, 100.0f, 100.0f), paint); + + paint.setStrokeJoin(DlStrokeJoin::kRound); + draw_rect(builder, DlRect::MakeXYWH(350.0f, 400.0f, 100.0f, 100.0f), paint); + + paint.setStrokeJoin(DlStrokeJoin::kMiter); + draw_rect(builder, DlRect::MakeXYWH(600.0f, 400.0f, 100.0f, 100.0f), paint); + + return builder.Build(); +} +} // namespace + +TEST_P(AiksTest, CanRenderWideStrokedRectWithoutOverlap) { + ASSERT_TRUE(OpenPlaygroundHere(MakeWideStrokedRects( + GetContentScale(), [](DisplayListBuilder& builder, const DlRect& rect, + const DlPaint& paint) { + // Draw the rect directly + builder.DrawRect(rect, paint); + }))); +} + +TEST_P(AiksTest, CanRenderWideStrokedRectPathWithoutOverlap) { + ASSERT_TRUE(OpenPlaygroundHere(MakeWideStrokedRects( + GetContentScale(), [](DisplayListBuilder& builder, const DlRect& rect, + const DlPaint& paint) { + // Draw the rect as a Path + builder.DrawPath(DlPath::MakeRect(rect), paint); + }))); +} + TEST_P(AiksTest, CanRenderImage) { DisplayListBuilder builder; DlPaint paint; diff --git a/engine/src/flutter/impeller/entity/geometry/rect_geometry.cc b/engine/src/flutter/impeller/entity/geometry/rect_geometry.cc index 6e90e1ff960..5dc78f7a823 100644 --- a/engine/src/flutter/impeller/entity/geometry/rect_geometry.cc +++ b/engine/src/flutter/impeller/entity/geometry/rect_geometry.cc @@ -25,6 +25,7 @@ GeometryResult FillRectGeometry::GetPositionBuffer( .index_type = IndexType::kNone, }, .transform = entity.GetShaderTransform(pass), + .mode = GeometryResult::Mode::kNormal, }; } @@ -67,10 +68,12 @@ GeometryResult StrokeRectGeometry::GetPositionBuffer( } Scalar min_size = kMinStrokeSize / max_basis; - Scalar half_stroke_width = std::max(stroke_width_, min_size) * 0.5f; + Scalar stroke_width = std::max(stroke_width_, min_size); + Scalar half_stroke_width = stroke_width * 0.5f; auto& data_host_buffer = renderer.GetTransientsDataBuffer(); const Rect& rect = rect_; + bool interior_filled = (stroke_width >= rect.GetSize().MinDimension()); switch (stroke_join_) { case Join::kRound: { @@ -80,16 +83,13 @@ GeometryResult StrokeRectGeometry::GetPositionBuffer( FML_DCHECK(trigs.size() >= 2u); - // We use all but the first entry in trigs for each corner. - auto vertex_count = trigs.size() - 1; - // Every other point has a center vertex added. - vertex_count = vertex_count + (vertex_count >> 1); - // The loop also adds 3 points of its own. - vertex_count += 3; - // We do that for each of the 4 corners. - vertex_count = vertex_count * 4; - // We then add 2 more points at the end to close the last edge. - vertex_count += 2; + auto vertex_count = trigs.size() * 4; + if (!interior_filled) { + // If there is a hole in the interior (as with most stroked rects + // unless the stroke width is really really wide) then we need + // to perform some surgery to generate the hollowed-out interior. + vertex_count += 12; + } return GeometryResult{ .type = PrimitiveType::kTriangleStrip, @@ -97,29 +97,103 @@ GeometryResult StrokeRectGeometry::GetPositionBuffer( { .vertex_buffer = data_host_buffer.Emplace( vertex_count * sizeof(Point), alignof(Point), - [hsw = half_stroke_width, &rect, vertex_count, - &trigs](uint8_t* buffer) { + [hsw = half_stroke_width, &rect, vertex_count, &trigs, + interior_filled](uint8_t* buffer) { + Scalar left = rect.GetLeft(); + Scalar top = rect.GetTop(); + Scalar right = rect.GetRight(); + Scalar bottom = rect.GetBottom(); + auto vertices = reinterpret_cast(buffer); [[maybe_unused]] auto vertices_end = vertices + vertex_count; - vertices = - AppendRoundCornerJoin(vertices, rect.GetLeftTop(), - Vector2(-hsw, 0), trigs); - vertices = - AppendRoundCornerJoin(vertices, rect.GetRightTop(), - Vector2(0, -hsw), trigs); - vertices = AppendRoundCornerJoin( - vertices, rect.GetRightBottom(), Vector2(hsw, 0), - trigs); - vertices = AppendRoundCornerJoin( - vertices, rect.GetLeftBottom(), Vector2(0, hsw), - trigs); + // Traverse top down, left to right across slices. - // Repeat the first 2 points from the first corner to - // close the last edge. - *vertices++ = rect.GetLeftTop() - Vector2(hsw, 0); - *vertices++ = rect.GetLeftTop() + Vector2(hsw, 0); + // Slice 1: Draw across between top pair of round joins. + for (auto trig : trigs) { + // trig.sin goes from 0 to 1 + // trig.cos goes from 1 to 0 + *vertices++ = Point(left - trig.sin * hsw, + top - trig.cos * hsw); + *vertices++ = Point(right + trig.sin * hsw, + top - trig.cos * hsw); + } + // Ends up with vertices that draw across the bottom + // of the top curved section (left - hsw, top) to + // (right + hsw, top). This is the starting pair of + // vertices for the following square section. + + if (interior_filled) { + // If interior is filled, we can just let the bottom + // pair of vertices of the top edge connect to the + // top pair of vertices of the bottom edge generated + // in slice 5 below. They both go left-right so they + // will create a proper zig-zag box to connect the + // 2 sections. + } else { + // Slice 2: Draw the inner part of the top stroke. + // Simply extend down from the last horizontal pair + // of vertices to (top + hsw). + *vertices++ = Point(left - hsw, top + hsw); + *vertices++ = Point(right + hsw, top + hsw); + + // Slice 3: Draw the left and right edges. + + // Slice 3a: Draw the right edge first. + // Since we are already at the right edge from the + // previous slice, we just have to add 2 vertices + // to get to the bottom of that right edge, but we + // have to start with an additional vertex that + // connects to (right - hsw) instead of the left + // side of the rectangle to avoid a big triangle + // through the hollow interior section. + *vertices++ = Point(right - hsw, top + hsw); + *vertices++ = Point(right + hsw, bottom - hsw); + *vertices++ = Point(right - hsw, bottom - hsw); + + // Now we need to jump up for the left edge, but we + // need to dupliate the last point and the next point + // to avoid drawing anything connecting them. These + // 2 vertices end up generating 2 empty triangles. + *vertices++ = Point(right - hsw, bottom - hsw); + *vertices++ = Point(left + hsw, top + hsw); + + // Slice 3b: Now draw the left edge. + // We draw this in a specific zig zag order so that + // we end up at (left - hsw, bottom - hsw) to connect + // properly to the next section. + *vertices++ = Point(left + hsw, top + hsw); + *vertices++ = Point(left - hsw, top + hsw); + *vertices++ = Point(left + hsw, bottom - hsw); + *vertices++ = Point(left - hsw, bottom - hsw); + + // Slice 4: Draw the inner part of the bottom stroke. + // Since the next section starts by drawing across + // the width of the rect at Y=bottom, we simple have + // to make sure that we presently have a pair of + // vertices that span the top of that section. The + // last point was (left - hsw, bottom - hsw), so we + // just have to add its right side partner which + // is not the same as the vertex before that. This + // extra vertex ends up defining an empty triangle, + // but sets us up for the final slice to complete + // this interior part of the bottom stroke. + *vertices++ = Point(right + hsw, bottom - hsw); + // Now the first pair of vertices below will + // "complete the zig-zag box" for the inner part + // of the bottom stroke. + } + + // Slice 5: Draw between bottom pair of round joins. + for (auto trig : trigs) { + // trig.sin goes from 0 to 1 + // trig.cos goes from 1 to 0 + *vertices++ = Point(left - trig.cos * hsw, + bottom + trig.sin * hsw); + *vertices++ = Point(right + trig.cos * hsw, + bottom + trig.sin * hsw); + } // Make sure our estimate is always up to date. FML_DCHECK(vertices == vertices_end); @@ -128,10 +202,40 @@ GeometryResult StrokeRectGeometry::GetPositionBuffer( .index_type = IndexType::kNone, }, .transform = entity.GetShaderTransform(pass), + .mode = GeometryResult::Mode::kNormal, }; } case Join::kBevel: { + if (interior_filled) { + return GeometryResult{ + .type = PrimitiveType::kTriangleStrip, + .vertex_buffer = + { + .vertex_buffer = data_host_buffer.Emplace( + 8 * sizeof(Point), alignof(Point), + [hsw = half_stroke_width, &rect](uint8_t* buffer) { + Scalar left = rect.GetLeft(); + Scalar top = rect.GetTop(); + Scalar right = rect.GetRight(); + Scalar bottom = rect.GetBottom(); + auto vertices = reinterpret_cast(buffer); + vertices[0] = Point(left, top - hsw); + vertices[1] = Point(right, top - hsw); + vertices[2] = Point(left - hsw, top); + vertices[3] = Point(right + hsw, top); + vertices[4] = Point(left - hsw, bottom); + vertices[5] = Point(right + hsw, bottom); + vertices[6] = Point(left, bottom + hsw); + vertices[7] = Point(right, bottom + hsw); + }), + .vertex_count = 8u, + .index_type = IndexType::kNone, + }, + .transform = entity.GetShaderTransform(pass), + .mode = GeometryResult::Mode::kNormal, + }; + } return GeometryResult{ .type = PrimitiveType::kTriangleStrip, .vertex_buffer = @@ -144,32 +248,59 @@ GeometryResult StrokeRectGeometry::GetPositionBuffer( Scalar right = rect.GetRight(); Scalar bottom = rect.GetBottom(); auto vertices = reinterpret_cast(buffer); - vertices[0] = Point(left, top - hsw); - vertices[1] = Point(left, top + hsw); - vertices[2] = Point(right, top - hsw); - vertices[3] = Point(right, top + hsw); - vertices[4] = Point(right + hsw, top); - vertices[5] = Point(right - hsw, top); - vertices[6] = Point(right + hsw, bottom); - vertices[7] = Point(right - hsw, bottom); - vertices[8] = Point(right, bottom + hsw); - vertices[9] = Point(right, bottom - hsw); - vertices[10] = Point(left, bottom + hsw); - vertices[11] = Point(left, bottom - hsw); - vertices[12] = Point(left - hsw, bottom); - vertices[13] = Point(left + hsw, bottom); - vertices[14] = Point(left - hsw, top); - vertices[15] = Point(left + hsw, top); - vertices[16] = Point(left, top - hsw); + vertices[0] = Point(left - hsw, top); + vertices[1] = Point(left + hsw, top + hsw); + vertices[2] = Point(left, top - hsw); + vertices[3] = Point(right - hsw, top + hsw); + vertices[4] = Point(right, top - hsw); + vertices[5] = Point(right - hsw, top + hsw); + vertices[6] = Point(right + hsw, top); + vertices[7] = Point(right - hsw, bottom - hsw); + vertices[8] = Point(right + hsw, bottom); + vertices[9] = Point(right - hsw, bottom - hsw); + vertices[10] = Point(right, bottom + hsw); + vertices[11] = Point(left + hsw, bottom - hsw); + vertices[12] = Point(left, bottom + hsw); + vertices[13] = Point(left + hsw, bottom - hsw); + vertices[14] = Point(left - hsw, bottom); + vertices[15] = Point(left + hsw, top + hsw); + vertices[16] = Point(left - hsw, top); }), .vertex_count = 17u, .index_type = IndexType::kNone, }, .transform = entity.GetShaderTransform(pass), + .mode = GeometryResult::Mode::kNormal, }; } case Join::kMiter: { + if (interior_filled) { + return GeometryResult{ + .type = PrimitiveType::kTriangleStrip, + .vertex_buffer = + { + .vertex_buffer = data_host_buffer.Emplace( + 4 * sizeof(Point), alignof(Point), + [hsw = half_stroke_width, &rect](uint8_t* buffer) { + Scalar left = rect.GetLeft(); + Scalar top = rect.GetTop(); + Scalar right = rect.GetRight(); + Scalar bottom = rect.GetBottom(); + auto vertices = reinterpret_cast(buffer); + + vertices[0] = Point(left - hsw, top - hsw); + vertices[1] = Point(right + hsw, top - hsw); + vertices[2] = Point(left - hsw, bottom + hsw); + vertices[3] = Point(right + hsw, bottom + hsw); + }), + .vertex_count = 4u, + .index_type = IndexType::kNone, + }, + .transform = entity.GetShaderTransform(pass), + .mode = GeometryResult::Mode::kNormal, + }; + } return GeometryResult{ .type = PrimitiveType::kTriangleStrip, .vertex_buffer = @@ -197,6 +328,7 @@ GeometryResult StrokeRectGeometry::GetPositionBuffer( .index_type = IndexType::kNone, }, .transform = entity.GetShaderTransform(pass), + .mode = GeometryResult::Mode::kNormal, }; } } @@ -213,29 +345,4 @@ Join StrokeRectGeometry::AdjustStrokeJoin(const StrokeParameters& stroke) { : stroke.join; } -Point* StrokeRectGeometry::AppendRoundCornerJoin( - Point* buffer, - Point corner, - Vector2 offset, - const Tessellator::Trigs& trigs) { - // Close the edge box set up by the end of the last corner and - // set up the first wedge of this corner. - *buffer++ = corner + offset; - *buffer++ = corner - offset; - bool do_center = false; - auto trig = trigs.begin(); - auto end = trigs.end(); - while (++trig < end) { - if (do_center) { - *buffer++ = corner; - } - do_center = !do_center; - *buffer++ = corner + *trig * offset; - } - // Together with the last point pushed by the loop we set up to - // initiate the edge box connecting to the next corner. - *buffer++ = corner - end[-1] * offset; - return buffer; -} - } // namespace impeller diff --git a/engine/src/flutter/impeller/entity/geometry/rect_geometry.h b/engine/src/flutter/impeller/entity/geometry/rect_geometry.h index 14427d90c78..d73ed3b5ac5 100644 --- a/engine/src/flutter/impeller/entity/geometry/rect_geometry.h +++ b/engine/src/flutter/impeller/entity/geometry/rect_geometry.h @@ -54,11 +54,6 @@ class StrokeRectGeometry final : public Geometry { const Join stroke_join_; static Join AdjustStrokeJoin(const StrokeParameters& stroke); - - static Point* AppendRoundCornerJoin(Point* buffer, - Point corner, - Vector2 offset, - const Tessellator::Trigs& trigs); }; } // namespace impeller