Skip to content

Commit 722a9a6

Browse files
chinmaygardechinmaygarde
committed
Revert "[Impeller] fix line/polygon depth and GLES scissor state. (flutter#56494)"
This reverts commit 950e240. This caused a regression in a customer application (@lyceel). I am not sure I should revert this whole patch. The goldens don't change if I just revert the patches to `render_pass_gles.cc`. So while that part was untested in the original commit, the other patches should be good. The original commit message did mention two separate issues were being addressed. I am working on reduced test case with just the scissor regression with the customer.
1 parent f98cefb commit 722a9a6

File tree

6 files changed

+8
-80
lines changed

6 files changed

+8
-80
lines changed

‎impeller/display_list/aiks_dl_unittests.cc

Lines changed: 0 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,12 @@
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>
76
#include "display_list/dl_sampling_options.h"
87
#include "display_list/dl_tile_mode.h"
98
#include "display_list/effects/dl_color_filter.h"
109
#include "display_list/effects/dl_color_source.h"
1110
#include "display_list/effects/dl_image_filter.h"
1211
#include "display_list/geometry/dl_geometry_types.h"
13-
#include "display_list/geometry/dl_path.h"
1412
#include "display_list/image/dl_image.h"
1513
#include "flutter/impeller/display_list/aiks_unittests.h"
1614

@@ -23,9 +21,7 @@
2321
#include "impeller/display_list/dl_dispatcher.h"
2422
#include "impeller/display_list/dl_image_impeller.h"
2523
#include "impeller/geometry/scalar.h"
26-
#include "include/core/SkCanvas.h"
2724
#include "include/core/SkMatrix.h"
28-
#include "include/core/SkPath.h"
2925
#include "include/core/SkRSXform.h"
3026
#include "include/core/SkRefCnt.h"
3127

@@ -986,63 +982,5 @@ TEST_P(AiksTest, CanEmptyPictureConvertToImage) {
986982
ASSERT_TRUE(OpenPlaygroundHere(recorder_builder.Build()));
987983
}
988984

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

‎impeller/display_list/canvas.cc

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -446,17 +446,13 @@ bool Canvas::AttemptDrawBlurredRRect(const Rect& rect,
446446
return true;
447447
}
448448

449-
void Canvas::DrawLine(const Point& p0,
450-
const Point& p1,
451-
const Paint& paint,
452-
bool reuse_depth) {
449+
void Canvas::DrawLine(const Point& p0, const Point& p1, const Paint& paint) {
453450
Entity entity;
454451
entity.SetTransform(GetCurrentTransform());
455452
entity.SetBlendMode(paint.blend_mode);
456453

457454
LineGeometry geom(p0, p1, paint.stroke_width, paint.stroke_cap);
458-
AddRenderEntityWithFiltersToCurrentPass(entity, &geom, paint,
459-
/*reuse_depth=*/reuse_depth);
455+
AddRenderEntityWithFiltersToCurrentPass(entity, &geom, paint);
460456
}
461457

462458
void Canvas::DrawRect(const Rect& rect, const Paint& paint) {
@@ -576,6 +572,7 @@ void Canvas::ClipGeometry(const Geometry& geometry,
576572
// See https://github.com/flutter/flutter/issues/147021
577573
FML_DCHECK(current_depth_ <= transform_stack_.back().clip_depth)
578574
<< current_depth_ << " <=? " << transform_stack_.back().clip_depth;
575+
579576
uint32_t clip_depth = transform_stack_.back().clip_depth;
580577

581578
const Matrix clip_transform =

‎impeller/display_list/canvas.h

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -187,10 +187,7 @@ class Canvas {
187187

188188
void DrawPaint(const Paint& paint);
189189

190-
void DrawLine(const Point& p0,
191-
const Point& p1,
192-
const Paint& paint,
193-
bool reuse_depth = false);
190+
void DrawLine(const Point& p0, const Point& p1, const Paint& paint);
194191

195192
void DrawRect(const Rect& rect, const Paint& paint);
196193

‎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, /*reuse_depth=*/i > 1);
678+
GetCanvas().DrawLine(p0, p1, paint);
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, /*reuse_depth=*/i > 1);
686+
GetCanvas().DrawLine(p0, p1, paint);
687687
p0 = p1;
688688
}
689689
}

‎impeller/renderer/backend/gles/render_pass_gles.cc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -369,6 +369,8 @@ static bool BindVertexBuffer(const ProcTableGLES& gl,
369369
scissor.GetWidth(), // width
370370
scissor.GetHeight() // height
371371
);
372+
} else {
373+
gl.Disable(GL_SCISSOR_TEST);
372374
}
373375

374376
//--------------------------------------------------------------------------

‎testing/impeller_golden_tests_output.txt

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -575,12 +575,6 @@ impeller_Play_AiksTest_CoordinateConversionsAreCorrect_Vulkan.png
575575
impeller_Play_AiksTest_CoverageOriginShouldBeAccountedForInSubpasses_Metal.png
576576
impeller_Play_AiksTest_CoverageOriginShouldBeAccountedForInSubpasses_OpenGLES.png
577577
impeller_Play_AiksTest_CoverageOriginShouldBeAccountedForInSubpasses_Vulkan.png
578-
impeller_Play_AiksTest_DepthValuesForLineMode_Metal.png
579-
impeller_Play_AiksTest_DepthValuesForLineMode_OpenGLES.png
580-
impeller_Play_AiksTest_DepthValuesForLineMode_Vulkan.png
581-
impeller_Play_AiksTest_DepthValuesForPolygonMode_Metal.png
582-
impeller_Play_AiksTest_DepthValuesForPolygonMode_OpenGLES.png
583-
impeller_Play_AiksTest_DepthValuesForPolygonMode_Vulkan.png
584578
impeller_Play_AiksTest_DestructiveBlendColorFilterFloodsClip_Metal.png
585579
impeller_Play_AiksTest_DestructiveBlendColorFilterFloodsClip_OpenGLES.png
586580
impeller_Play_AiksTest_DestructiveBlendColorFilterFloodsClip_Vulkan.png

0 commit comments

Comments
 (0)