[CP][Impeller] fix line/polygon depth and GLES scissor state. (#56494) (

#57248)
flutter · Jan 8, 2025 · d11906b · d11906b
1 parent c3ea4be
commit d11906b
Show file tree

Hide file tree

Showing 6 changed files with 78 additions and 7 deletions.
diff --git a/impeller/display_list/aiks_dl_unittests.cc b/impeller/display_list/aiks_dl_unittests.cc
@@ -3,12 +3,14 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <vector>
 #include "display_list/dl_sampling_options.h"
 #include "display_list/dl_tile_mode.h"
 #include "display_list/effects/dl_color_filter.h"
 #include "display_list/effects/dl_color_source.h"
 #include "display_list/effects/dl_image_filter.h"
 #include "display_list/geometry/dl_geometry_types.h"
+#include "display_list/geometry/dl_path.h"
 #include "display_list/image/dl_image.h"
 #include "flutter/impeller/display_list/aiks_unittests.h"
 
@@ -21,7 +23,9 @@
 #include "impeller/display_list/dl_dispatcher.h"
 #include "impeller/display_list/dl_image_impeller.h"
 #include "impeller/geometry/scalar.h"
+#include "include/core/SkCanvas.h"
 #include "include/core/SkMatrix.h"
+#include "include/core/SkPath.h"
 #include "include/core/SkRSXform.h"
 #include "include/core/SkRefCnt.h"
 
@@ -974,5 +978,61 @@ TEST_P(AiksTest, CanEmptyPictureConvertToImage) {
   ASSERT_TRUE(OpenPlaygroundHere(recorder_builder.Build()));
 }
 
+TEST_P(AiksTest, DepthValuesForLineMode) {
+  // Ensures that the additional draws created by line/polygon mode all
+  // have the same depth values.
+  DisplayListBuilder builder;
+
+  SkPath path = SkPath::Circle(100, 100, 100);
+
+  builder.DrawPath(path, DlPaint()
+                             .setColor(DlColor::kRed())
+                             .setDrawStyle(DlDrawStyle::kStroke)
+                             .setStrokeWidth(5));
+  builder.Save();
+  builder.ClipPath(path);
+
+  std::vector<SkPoint> points = {
+      SkPoint::Make(0, -200),  SkPoint::Make(400, 200), SkPoint::Make(0, -100),
+      SkPoint::Make(400, 300), SkPoint::Make(0, 0),     SkPoint::Make(400, 400),
+      SkPoint::Make(0, 100),   SkPoint::Make(400, 500), SkPoint::Make(0, 150),
+      SkPoint::Make(400, 600)};
+
+  builder.DrawPoints(DisplayListBuilder::PointMode::kLines, points.size(),
+                     points.data(),
+                     DlPaint().setColor(DlColor::kBlue()).setStrokeWidth(10));
+  builder.Restore();
+
+  ASSERT_TRUE(OpenPlaygroundHere(builder.Build()));
+}
+
+TEST_P(AiksTest, DepthValuesForPolygonMode) {
+  // Ensures that the additional draws created by line/polygon mode all
+  // have the same depth values.
+  DisplayListBuilder builder;
+
+  SkPath path = SkPath::Circle(100, 100, 100);
+
+  builder.DrawPath(path, DlPaint()
+                             .setColor(DlColor::kRed())
+                             .setDrawStyle(DlDrawStyle::kStroke)
+                             .setStrokeWidth(5));
+  builder.Save();
+  builder.ClipPath(path);
+
+  std::vector<SkPoint> points = {
+      SkPoint::Make(0, -200),  SkPoint::Make(400, 200), SkPoint::Make(0, -100),
+      SkPoint::Make(400, 300), SkPoint::Make(0, 0),     SkPoint::Make(400, 400),
+      SkPoint::Make(0, 100),   SkPoint::Make(400, 500), SkPoint::Make(0, 150),
+      SkPoint::Make(400, 600)};
+
+  builder.DrawPoints(DisplayListBuilder::PointMode::kPolygon, points.size(),
+                     points.data(),
+                     DlPaint().setColor(DlColor::kBlue()).setStrokeWidth(10));
+  builder.Restore();
+
+  ASSERT_TRUE(OpenPlaygroundHere(builder.Build()));
+}
+
 }  // namespace testing
 }  // namespace impeller
diff --git a/impeller/display_list/canvas.cc b/impeller/display_list/canvas.cc
@@ -543,13 +543,17 @@ bool Canvas::AttemptDrawBlurredRRect(const Rect& rect,
   return true;
 }
 
-void Canvas::DrawLine(const Point& p0, const Point& p1, const Paint& paint) {
+void Canvas::DrawLine(const Point& p0,
+                      const Point& p1,
+                      const Paint& paint,
+                      bool reuse_depth) {
   Entity entity;
   entity.SetTransform(GetCurrentTransform());
   entity.SetBlendMode(paint.blend_mode);
 
   LineGeometry geom(p0, p1, paint.stroke_width, paint.stroke_cap);
-  AddRenderEntityWithFiltersToCurrentPass(entity, &geom, paint);
+  AddRenderEntityWithFiltersToCurrentPass(entity, &geom, paint,
+                                          /*reuse_depth=*/reuse_depth);
 }
 
 void Canvas::DrawRect(const Rect& rect, const Paint& paint) {

diff --git a/impeller/display_list/canvas.h b/impeller/display_list/canvas.h
@@ -167,7 +167,10 @@ class Canvas {
 
   void DrawPaint(const Paint& paint);
 
-  void DrawLine(const Point& p0, const Point& p1, const Paint& paint);
+  void DrawLine(const Point& p0,
+                const Point& p1,
+                const Paint& paint,
+                bool reuse_depth = false);
 
   void DrawRect(const Rect& rect, const Paint& paint);
 

diff --git a/impeller/display_list/dl_dispatcher.cc b/impeller/display_list/dl_dispatcher.cc
@@ -675,15 +675,15 @@ void DlDispatcherBase::drawPoints(PointMode mode,
       for (uint32_t i = 1; i < count; i += 2) {
         Point p0 = points[i - 1];
         Point p1 = points[i];
-        GetCanvas().DrawLine(p0, p1, paint);
+        GetCanvas().DrawLine(p0, p1, paint, /*reuse_depth=*/i > 1);
       }
       break;
     case flutter::DlCanvas::PointMode::kPolygon:
       if (count > 1) {
         Point p0 = points[0];
         for (uint32_t i = 1; i < count; i++) {
           Point p1 = points[i];
-          GetCanvas().DrawLine(p0, p1, paint);
+          GetCanvas().DrawLine(p0, p1, paint, /*reuse_depth=*/i > 1);
           p0 = p1;
         }
       }

diff --git a/impeller/renderer/backend/gles/render_pass_gles.cc b/impeller/renderer/backend/gles/render_pass_gles.cc
@@ -338,8 +338,6 @@ struct RenderPassData {
           scissor.GetWidth(),                                         // width
           scissor.GetHeight()                                         // height
       );
-    } else {
-      gl.Disable(GL_SCISSOR_TEST);
     }
 
     //--------------------------------------------------------------------------

diff --git a/testing/impeller_golden_tests_output.txt b/testing/impeller_golden_tests_output.txt
@@ -548,6 +548,12 @@ impeller_Play_AiksTest_CoordinateConversionsAreCorrect_Vulkan.png
 impeller_Play_AiksTest_CoverageOriginShouldBeAccountedForInSubpasses_Metal.png
 impeller_Play_AiksTest_CoverageOriginShouldBeAccountedForInSubpasses_OpenGLES.png
 impeller_Play_AiksTest_CoverageOriginShouldBeAccountedForInSubpasses_Vulkan.png
+impeller_Play_AiksTest_DepthValuesForLineMode_Metal.png
+impeller_Play_AiksTest_DepthValuesForLineMode_OpenGLES.png
+impeller_Play_AiksTest_DepthValuesForLineMode_Vulkan.png
+impeller_Play_AiksTest_DepthValuesForPolygonMode_Metal.png
+impeller_Play_AiksTest_DepthValuesForPolygonMode_OpenGLES.png
+impeller_Play_AiksTest_DepthValuesForPolygonMode_Vulkan.png
 impeller_Play_AiksTest_DispatcherDoesNotCullPerspectiveTransformedChildDisplayLists_Metal.png
 impeller_Play_AiksTest_DispatcherDoesNotCullPerspectiveTransformedChildDisplayLists_OpenGLES.png
 impeller_Play_AiksTest_DispatcherDoesNotCullPerspectiveTransformedChildDisplayLists_Vulkan.png