Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

encode DPR for shadows into DisplayList #27289

Merged
merged 1 commit into from
Jul 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
encode DPR for shadows into DisplayList
  • Loading branch information
flar committed Jul 9, 2021
commit 5074a5a056dd3ac2f6f9b544b34586d982d5db49
47 changes: 27 additions & 20 deletions flow/display_list.cc
Original file line number Diff line number Diff line change
Expand Up @@ -809,24 +809,28 @@ struct DrawTextBlobOp final : DLOp {
};

// 4 byte header + 28 byte payload packs evenly into 32 bytes
struct DrawShadowOp final : DLOp {
static const auto kType = DisplayListOpType::kDrawShadow;

DrawShadowOp(const SkPath& path,
SkColor color,
SkScalar elevation,
bool occludes)
: color(color), elevation(elevation), occludes(occludes), path(path) {}

const SkColor color;
const SkScalar elevation;
const bool occludes;
const SkPath path;

void dispatch(Dispatcher& dispatcher) const {
dispatcher.drawShadow(path, color, elevation, occludes);
}
};
#define DEFINE_DRAW_SHADOW_OP(name, occludes) \
struct Draw##name##Op final : DLOp { \
static const auto kType = DisplayListOpType::kDraw##name; \
\
Draw##name##Op(const SkPath& path, \
SkColor color, \
SkScalar elevation, \
SkScalar dpr) \
: color(color), elevation(elevation), dpr(dpr), path(path) {} \
\
const SkColor color; \
const SkScalar elevation; \
const SkScalar dpr; \
const SkPath path; \
\
void dispatch(Dispatcher& dispatcher) const { \
dispatcher.drawShadow(path, color, elevation, occludes, dpr); \
} \
};
DEFINE_DRAW_SHADOW_OP(Shadow, false)
DEFINE_DRAW_SHADOW_OP(ShadowOccludes, true)
#undef DEFINE_DRAW_SHADOW_OP

#pragma pack(pop, DLOp_Alignment)

Expand Down Expand Up @@ -1367,8 +1371,11 @@ void DisplayListBuilder::drawTextBlob(const sk_sp<SkTextBlob> blob,
void DisplayListBuilder::drawShadow(const SkPath& path,
const SkColor color,
const SkScalar elevation,
bool occludes) {
Push<DrawShadowOp>(0, path, color, elevation, occludes);
bool occludes,
SkScalar dpr) {
occludes //
? Push<DrawShadowOccludesOp>(0, path, color, elevation, dpr)
: Push<DrawShadowOp>(0, path, color, elevation, dpr);
}

} // namespace flutter
9 changes: 6 additions & 3 deletions flow/display_list.h
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,8 @@ namespace flutter {
V(DrawDisplayList) \
V(DrawTextBlob) \
\
V(DrawShadow)
V(DrawShadow) \
V(DrawShadowOccludes)

#define DL_OP_TO_ENUM_VALUE(name) k##name,
enum class DisplayListOpType { FOR_EACH_DISPLAY_LIST_OP(DL_OP_TO_ENUM_VALUE) };
Expand Down Expand Up @@ -321,7 +322,8 @@ class Dispatcher {
virtual void drawShadow(const SkPath& path,
const SkColor color,
const SkScalar elevation,
bool occludes) = 0;
bool occludes,
SkScalar dpr) = 0;
};

// The primary class used to build a display list. The list of methods
Expand Down Expand Up @@ -435,7 +437,8 @@ class DisplayListBuilder final : public virtual Dispatcher, public SkRefCnt {
void drawShadow(const SkPath& path,
const SkColor color,
const SkScalar elevation,
bool occludes) override;
bool occludes,
SkScalar dpr) override;

sk_sp<DisplayList> Build();

Expand Down
5 changes: 3 additions & 2 deletions flow/display_list_canvas.cc
Original file line number Diff line number Diff line change
Expand Up @@ -176,9 +176,10 @@ void DisplayListCanvasDispatcher::drawTextBlob(const sk_sp<SkTextBlob> blob,
void DisplayListCanvasDispatcher::drawShadow(const SkPath& path,
const SkColor color,
const SkScalar elevation,
bool occludes) {
bool occludes,
SkScalar dpr) {
flutter::PhysicalShapeLayer::DrawShadow(canvas_, path, color, elevation,
occludes, 1.0);
occludes, dpr);
}

DisplayListCanvasRecorder::DisplayListCanvasRecorder(const SkRect& bounds)
Expand Down
3 changes: 2 additions & 1 deletion flow/display_list_canvas.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,8 @@ class DisplayListCanvasDispatcher : public virtual Dispatcher,
void drawShadow(const SkPath& path,
const SkColor color,
const SkScalar elevation,
bool occludes) override;
bool occludes,
SkScalar dpr) override;

private:
SkCanvas* canvas_;
Expand Down
27 changes: 25 additions & 2 deletions flow/display_list_canvas_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1352,7 +1352,7 @@ TEST(DisplayListCanvas, DrawShadow) {
1.0);
},
[=](DisplayListBuilder& builder) { //
builder.drawShadow(path, color, elevation, false);
builder.drawShadow(path, color, elevation, false, 1.0);
});
CanvasCompareTester::UsingShadows = false;
}
Expand All @@ -1375,7 +1375,30 @@ TEST(DisplayListCanvas, DrawOccludingShadow) {
1.0);
},
[=](DisplayListBuilder& builder) { //
builder.drawShadow(path, color, elevation, true);
builder.drawShadow(path, color, elevation, true, 1.0);
});
CanvasCompareTester::UsingShadows = false;
}

TEST(DisplayListCanvas, DrawShadowDpr) {
CanvasCompareTester::UsingShadows = true;
SkPath path;
path.moveTo(RenderCenterX, RenderTop);
path.lineTo(RenderRight, RenderBottom);
path.lineTo(RenderLeft, RenderCenterY);
path.lineTo(RenderRight, RenderCenterY);
path.lineTo(RenderLeft, RenderBottom);
path.close();
const SkColor color = SK_ColorDKGRAY;
const SkScalar elevation = 10;

CanvasCompareTester::RenderNoAttributes(
[=](SkCanvas* canvas, SkPaint& paint) { //
PhysicalShapeLayer::DrawShadow(canvas, path, color, elevation, false,
2.5);
},
[=](DisplayListBuilder& builder) { //
builder.drawShadow(path, color, elevation, false, 2.5);
});
CanvasCompareTester::UsingShadows = false;
}
Expand Down
11 changes: 6 additions & 5 deletions flow/display_list_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -670,11 +670,12 @@ std::vector<DisplayListInvocationGroup> allGroups = {
// See: https://bugs.chromium.org/p/skia/issues/detail?id=12125
{ "DrawShadow", {
// cv shadows are turned into an opaque ShadowRec which is not exposed
{1, 32, -1, 32, [](DisplayListBuilder& b) {b.drawShadow(TestPath1, SK_ColorGREEN, 1.0, false);}},
{1, 32, -1, 32, [](DisplayListBuilder& b) {b.drawShadow(TestPath2, SK_ColorGREEN, 1.0, false);}},
{1, 32, -1, 32, [](DisplayListBuilder& b) {b.drawShadow(TestPath1, SK_ColorBLUE, 1.0, false);}},
{1, 32, -1, 32, [](DisplayListBuilder& b) {b.drawShadow(TestPath1, SK_ColorGREEN, 2.0, false);}},
{1, 32, -1, 32, [](DisplayListBuilder& b) {b.drawShadow(TestPath1, SK_ColorGREEN, 1.0, true);}},
{1, 32, -1, 32, [](DisplayListBuilder& b) {b.drawShadow(TestPath1, SK_ColorGREEN, 1.0, false, 1.0);}},
{1, 32, -1, 32, [](DisplayListBuilder& b) {b.drawShadow(TestPath2, SK_ColorGREEN, 1.0, false, 1.0);}},
{1, 32, -1, 32, [](DisplayListBuilder& b) {b.drawShadow(TestPath1, SK_ColorBLUE, 1.0, false, 1.0);}},
{1, 32, -1, 32, [](DisplayListBuilder& b) {b.drawShadow(TestPath1, SK_ColorGREEN, 2.0, false, 1.0);}},
{1, 32, -1, 32, [](DisplayListBuilder& b) {b.drawShadow(TestPath1, SK_ColorGREEN, 1.0, true, 1.0);}},
{1, 32, -1, 32, [](DisplayListBuilder& b) {b.drawShadow(TestPath1, SK_ColorGREEN, 1.0, false, 2.5);}},
}
},
};
Expand Down
9 changes: 5 additions & 4 deletions flow/display_list_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,8 @@ void DisplayListBoundsCalculator::drawTextBlob(const sk_sp<SkTextBlob> blob,
void DisplayListBoundsCalculator::drawShadow(const SkPath& path,
const SkColor color,
const SkScalar elevation,
bool occludes) {
bool occludes,
SkScalar dpr) {
// Constants from physical_shape_layer.cc
const SkScalar kLightHeight = 600;
const SkScalar kLightRadius = 800;
Expand All @@ -351,9 +352,9 @@ void DisplayListBoundsCalculator::drawShadow(const SkPath& path,
SkScalar shadow_y = bounds.top() - 600.0f;
SkRect shadow_bounds;
SkShadowUtils::GetLocalBounds(
matrix(), path, SkPoint3::Make(0, 0, elevation),
SkPoint3::Make(shadow_x, shadow_y, kLightHeight), kLightRadius, flags,
&shadow_bounds);
matrix(), path, SkPoint3::Make(0, 0, dpr * elevation),
SkPoint3::Make(shadow_x, shadow_y, dpr * kLightHeight),
dpr * kLightRadius, flags, &shadow_bounds);
accumulateRect(shadow_bounds);
}

Expand Down
3 changes: 2 additions & 1 deletion flow/display_list_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,8 @@ class DisplayListBoundsCalculator final
void drawShadow(const SkPath& path,
const SkColor color,
const SkScalar elevation,
bool occludes) override;
bool occludes,
SkScalar dpr) override;

SkRect getBounds() { return accumulator_->getBounds(); }

Expand Down
13 changes: 7 additions & 6 deletions lib/ui/painting/canvas.cc
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,11 @@ void Canvas::drawShadow(const CanvasPath* path,
ToDart("Canvas.drawShader called with non-genuine Path."));
return;
}
SkScalar dpr = UIDartState::Current()
->platform_configuration()
->get_window(0)
->viewport_metrics()
.device_pixel_ratio;
if (display_list_recorder_) {
// The DrawShadow mechanism results in non-public operations to be
// performed on the canvas involving an SkDrawShadowRec. Since we
Expand All @@ -507,13 +512,9 @@ void Canvas::drawShadow(const CanvasPath* path,
// that situation we bypass the canvas interface and inject the
// shadow parameters directly into the underlying DisplayList.
// See: https://bugs.chromium.org/p/skia/issues/detail?id=12125
builder()->drawShadow(path->path(), color, elevation, transparentOccluder);
builder()->drawShadow(path->path(), color, elevation, transparentOccluder,
dpr);
} else {
SkScalar dpr = UIDartState::Current()
->platform_configuration()
->get_window(0)
->viewport_metrics()
.device_pixel_ratio;
flutter::PhysicalShapeLayer::DrawShadow(
canvas_, path->path(), color, elevation, transparentOccluder, dpr);
}
Expand Down