Skip to content

Commit ee25bed

Browse files
committed
[Impeller] fix line/polygon depth and GLES scissor state. (flutter#56494)
Two problems: 1. We were incorrectly clearing scissor state in the GLES render pass. I suspect this is the cause of all of the clipping problems in wonderous. 2. We were incrementing the depth value in drawPoints with line/polygon mode: these actually need to use the same depth values. Fixed in the same PR because the golden actually demonstrates both problems with GLES, and luckily I found 2. when I noticed 1. ![image](https://github.com/user-attachments/assets/a5541a51-bdf6-4a47-9638-610d9562df6f) ![image](https://github.com/user-attachments/assets/9f3ad7e1-8193-405d-98c0-49141c8ab662)
1 parent 83bacfc commit ee25bed

File tree

6 files changed

+86
-7
lines changed

6 files changed

+86
-7
lines changed

impeller/display_list/aiks_dl_unittests.cc

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,14 @@
33
// Use of this source code is governed by a BSD-style license that can be
44
// found in the LICENSE file.
55

6+
#include <vector>
67
#include "display_list/dl_sampling_options.h"
78
#include "display_list/dl_tile_mode.h"
89
#include "display_list/effects/dl_color_filter.h"
910
#include "display_list/effects/dl_color_source.h"
1011
#include "display_list/effects/dl_image_filter.h"
1112
#include "display_list/geometry/dl_geometry_types.h"
13+
#include "display_list/geometry/dl_path.h"
1214
#include "display_list/image/dl_image.h"
1315
#include "flutter/impeller/display_list/aiks_unittests.h"
1416

@@ -21,7 +23,9 @@
2123
#include "impeller/display_list/dl_dispatcher.h"
2224
#include "impeller/display_list/dl_image_impeller.h"
2325
#include "impeller/geometry/scalar.h"
26+
#include "include/core/SkCanvas.h"
2427
#include "include/core/SkMatrix.h"
28+
#include "include/core/SkPath.h"
2529
#include "include/core/SkRSXform.h"
2630
#include "include/core/SkRefCnt.h"
2731

@@ -974,5 +978,63 @@ TEST_P(AiksTest, CanEmptyPictureConvertToImage) {
974978
ASSERT_TRUE(OpenPlaygroundHere(recorder_builder.Build()));
975979
}
976980

981+
TEST_P(AiksTest, DepthValuesForLineMode) {
982+
// Ensures that the additional draws created by line/polygon mode all
983+
// have the same depth values.
984+
DisplayListBuilder builder;
985+
986+
SkPath path = SkPath::Circle(100, 100, 100);
987+
988+
builder.DrawPath(path, DlPaint()
989+
.setColor(DlColor::kRed())
990+
.setDrawStyle(DlDrawStyle::kStroke)
991+
.setStrokeWidth(5));
992+
builder.Save();
993+
builder.ClipPath(path);
994+
995+
std::vector<DlPoint> points = {
996+
DlPoint::MakeXY(0, -200), DlPoint::MakeXY(400, 200),
997+
DlPoint::MakeXY(0, -100), DlPoint::MakeXY(400, 300),
998+
DlPoint::MakeXY(0, 0), DlPoint::MakeXY(400, 400),
999+
DlPoint::MakeXY(0, 100), DlPoint::MakeXY(400, 500),
1000+
DlPoint::MakeXY(0, 150), DlPoint::MakeXY(400, 600)};
1001+
1002+
builder.DrawPoints(DisplayListBuilder::PointMode::kLines, points.size(),
1003+
points.data(),
1004+
DlPaint().setColor(DlColor::kBlue()).setStrokeWidth(10));
1005+
builder.Restore();
1006+
1007+
ASSERT_TRUE(OpenPlaygroundHere(builder.Build()));
1008+
}
1009+
1010+
TEST_P(AiksTest, DepthValuesForPolygonMode) {
1011+
// Ensures that the additional draws created by line/polygon mode all
1012+
// have the same depth values.
1013+
DisplayListBuilder builder;
1014+
1015+
SkPath path = SkPath::Circle(100, 100, 100);
1016+
1017+
builder.DrawPath(path, DlPaint()
1018+
.setColor(DlColor::kRed())
1019+
.setDrawStyle(DlDrawStyle::kStroke)
1020+
.setStrokeWidth(5));
1021+
builder.Save();
1022+
builder.ClipPath(path);
1023+
1024+
std::vector<DlPoint> points = {
1025+
DlPoint::MakeXY(0, -200), DlPoint::MakeXY(400, 200),
1026+
DlPoint::MakeXY(0, -100), DlPoint::MakeXY(400, 300),
1027+
DlPoint::MakeXY(0, 0), DlPoint::MakeXY(400, 400),
1028+
DlPoint::MakeXY(0, 100), DlPoint::MakeXY(400, 500),
1029+
DlPoint::MakeXY(0, 150), DlPoint::MakeXY(400, 600)};
1030+
1031+
builder.DrawPoints(DisplayListBuilder::PointMode::kPolygon, points.size(),
1032+
points.data(),
1033+
DlPaint().setColor(DlColor::kBlue()).setStrokeWidth(10));
1034+
builder.Restore();
1035+
1036+
ASSERT_TRUE(OpenPlaygroundHere(builder.Build()));
1037+
}
1038+
9771039
} // namespace testing
9781040
} // namespace impeller

impeller/display_list/canvas.cc

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -543,13 +543,17 @@ bool Canvas::AttemptDrawBlurredRRect(const Rect& rect,
543543
return true;
544544
}
545545

546-
void Canvas::DrawLine(const Point& p0, const Point& p1, const Paint& paint) {
546+
void Canvas::DrawLine(const Point& p0,
547+
const Point& p1,
548+
const Paint& paint,
549+
bool reuse_depth) {
547550
Entity entity;
548551
entity.SetTransform(GetCurrentTransform());
549552
entity.SetBlendMode(paint.blend_mode);
550553

551554
LineGeometry geom(p0, p1, paint.stroke_width, paint.stroke_cap);
552-
AddRenderEntityWithFiltersToCurrentPass(entity, &geom, paint);
555+
AddRenderEntityWithFiltersToCurrentPass(entity, &geom, paint,
556+
/*reuse_depth=*/reuse_depth);
553557
}
554558

555559
void Canvas::DrawRect(const Rect& rect, const Paint& paint) {

impeller/display_list/canvas.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,10 @@ class Canvas {
167167

168168
void DrawPaint(const Paint& paint);
169169

170-
void DrawLine(const Point& p0, const Point& p1, const Paint& paint);
170+
void DrawLine(const Point& p0,
171+
const Point& p1,
172+
const Paint& paint,
173+
bool reuse_depth = false);
171174

172175
void DrawRect(const Rect& rect, const Paint& paint);
173176

impeller/display_list/dl_dispatcher.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -675,15 +675,15 @@ void DlDispatcherBase::drawPoints(PointMode mode,
675675
for (uint32_t i = 1; i < count; i += 2) {
676676
Point p0 = points[i - 1];
677677
Point p1 = points[i];
678-
GetCanvas().DrawLine(p0, p1, paint);
678+
GetCanvas().DrawLine(p0, p1, paint, /*reuse_depth=*/i > 1);
679679
}
680680
break;
681681
case flutter::DlCanvas::PointMode::kPolygon:
682682
if (count > 1) {
683683
Point p0 = points[0];
684684
for (uint32_t i = 1; i < count; i++) {
685685
Point p1 = points[i];
686-
GetCanvas().DrawLine(p0, p1, paint);
686+
GetCanvas().DrawLine(p0, p1, paint, /*reuse_depth=*/i > 1);
687687
p0 = p1;
688688
}
689689
}

impeller/renderer/backend/gles/render_pass_gles.cc

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -338,8 +338,6 @@ struct RenderPassData {
338338
scissor.GetWidth(), // width
339339
scissor.GetHeight() // height
340340
);
341-
} else {
342-
gl.Disable(GL_SCISSOR_TEST);
343341
}
344342

345343
//--------------------------------------------------------------------------

testing/impeller_golden_tests_output.txt

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -548,6 +548,18 @@ impeller_Play_AiksTest_CoordinateConversionsAreCorrect_Vulkan.png
548548
impeller_Play_AiksTest_CoverageOriginShouldBeAccountedForInSubpasses_Metal.png
549549
impeller_Play_AiksTest_CoverageOriginShouldBeAccountedForInSubpasses_OpenGLES.png
550550
impeller_Play_AiksTest_CoverageOriginShouldBeAccountedForInSubpasses_Vulkan.png
551+
impeller_Play_AiksTest_DepthValuesForLineMode_Metal.png
552+
impeller_Play_AiksTest_DepthValuesForLineMode_OpenGLES.png
553+
impeller_Play_AiksTest_DepthValuesForLineMode_Vulkan.png
554+
impeller_Play_AiksTest_DepthValuesForPolygonMode_Metal.png
555+
impeller_Play_AiksTest_DepthValuesForPolygonMode_OpenGLES.png
556+
impeller_Play_AiksTest_DepthValuesForPolygonMode_Vulkan.png
557+
impeller_Play_AiksTest_DestructiveBlendColorFilterFloodsClip_Metal.png
558+
impeller_Play_AiksTest_DestructiveBlendColorFilterFloodsClip_OpenGLES.png
559+
impeller_Play_AiksTest_DestructiveBlendColorFilterFloodsClip_Vulkan.png
560+
impeller_Play_AiksTest_DifferenceClipsMustRenderIdenticallyAcrossBackends_Metal.png
561+
impeller_Play_AiksTest_DifferenceClipsMustRenderIdenticallyAcrossBackends_OpenGLES.png
562+
impeller_Play_AiksTest_DifferenceClipsMustRenderIdenticallyAcrossBackends_Vulkan.png
551563
impeller_Play_AiksTest_DispatcherDoesNotCullPerspectiveTransformedChildDisplayLists_Metal.png
552564
impeller_Play_AiksTest_DispatcherDoesNotCullPerspectiveTransformedChildDisplayLists_OpenGLES.png
553565
impeller_Play_AiksTest_DispatcherDoesNotCullPerspectiveTransformedChildDisplayLists_Vulkan.png

0 commit comments

Comments
 (0)