fix a number of conditions for computing bounds from saveLayers (flutter/engine#27683)

This commit is contained in:
Jim Graham 2021-08-03 00:10:02 -07:00 committed by GitHub
parent f7fe697384
commit 4f7f65eb99
5 changed files with 265 additions and 22 deletions

View File

@ -867,7 +867,7 @@ static void DisposeOps(uint8_t* ptr, uint8_t* end) {
FOR_EACH_DISPLAY_LIST_OP(DL_OP_DISPOSE)
#undef DL_OP_DISPATCH
#undef DL_OP_DISPOSE
default:
FML_DCHECK(false);
@ -905,7 +905,7 @@ static bool CompareOps(uint8_t* ptrA,
FOR_EACH_DISPLAY_LIST_OP(DL_OP_EQUALS)
#undef DL_OP_DISPATCH
#undef DL_OP_EQUALS
default:
FML_DCHECK(false);
@ -1246,7 +1246,7 @@ void DisplayListBuilder::drawPoints(SkCanvas::PointMode mode,
uint32_t count,
const SkPoint pts[]) {
void* data_ptr;
FML_DCHECK(count < MaxDrawPointsCount);
FML_DCHECK(count < kMaxDrawPointsCount);
int bytes = count * sizeof(SkPoint);
switch (mode) {
case SkCanvas::PointMode::kPoints_PointMode:

View File

@ -222,7 +222,7 @@ class DisplayList : public SkRefCnt {
class Dispatcher {
public:
// MaxDrawPointsCount * sizeof(SkPoint) must be less than 1 << 32
static constexpr int MaxDrawPointsCount = ((1 << 29) - 1);
static constexpr int kMaxDrawPointsCount = ((1 << 29) - 1);
virtual void setAA(bool aa) = 0;
virtual void setDither(bool dither) = 0;
@ -334,7 +334,7 @@ class Dispatcher {
// the DisplayListCanvasRecorder class.
class DisplayListBuilder final : public virtual Dispatcher, public SkRefCnt {
public:
DisplayListBuilder(const SkRect& cull = SkRect::MakeEmpty());
DisplayListBuilder(const SkRect& cull = kMaxCull_);
~DisplayListBuilder();
void setAA(bool aa) override;
@ -451,6 +451,8 @@ class DisplayListBuilder final : public virtual Dispatcher, public SkRefCnt {
int save_level_ = 0;
SkRect cull_;
static constexpr SkRect kMaxCull_ =
SkRect::MakeLTRB(-1E9F, -1E9F, 1E9F, 1E9F);
template <typename T, typename... Args>
void* Push(size_t extra, Args&&... args);

View File

@ -863,5 +863,130 @@ TEST(DisplayList, DisplayListsWithVaryingOpComparisons) {
}
}
TEST(DisplayList, DisplayListSaveLayerBoundsWithAlphaFilter) {
SkRect build_bounds = SkRect::MakeLTRB(-100, -100, 200, 200);
SkRect save_bounds = SkRect::MakeWH(100, 100);
SkRect rect = SkRect::MakeLTRB(30, 30, 70, 70);
// clang-format off
const float color_matrix[] = {
0, 0, 0, 0, 0,
0, 1, 0, 0, 0,
0, 0, 1, 0, 0,
0, 0, 0, 1, 0,
};
// clang-format on
sk_sp<SkColorFilter> base_color_filter = SkColorFilters::Matrix(color_matrix);
// clang-format off
const float alpha_matrix[] = {
0, 0, 0, 0, 0,
0, 1, 0, 0, 0,
0, 0, 1, 0, 0,
0, 0, 0, 0, 1,
};
// clang-format on
sk_sp<SkColorFilter> alpha_color_filter =
SkColorFilters::Matrix(alpha_matrix);
{
DisplayListBuilder builder(build_bounds);
builder.saveLayer(&save_bounds, true);
builder.drawRect(rect);
builder.restore();
sk_sp<DisplayList> display_list = builder.Build();
ASSERT_EQ(display_list->bounds(), rect);
}
{
DisplayListBuilder builder(build_bounds);
builder.setColorFilter(base_color_filter);
builder.saveLayer(&save_bounds, true);
builder.setColorFilter(nullptr);
builder.drawRect(rect);
builder.restore();
sk_sp<DisplayList> display_list = builder.Build();
ASSERT_EQ(display_list->bounds(), rect);
}
{
DisplayListBuilder builder(build_bounds);
builder.setColorFilter(alpha_color_filter);
builder.saveLayer(&save_bounds, true);
builder.setColorFilter(nullptr);
builder.drawRect(rect);
builder.restore();
sk_sp<DisplayList> display_list = builder.Build();
ASSERT_EQ(display_list->bounds(), save_bounds);
}
{
DisplayListBuilder builder(build_bounds);
builder.setColorFilter(alpha_color_filter);
builder.saveLayer(nullptr, true);
builder.setColorFilter(nullptr);
builder.drawRect(rect);
builder.restore();
sk_sp<DisplayList> display_list = builder.Build();
ASSERT_EQ(display_list->bounds(), build_bounds);
}
{
DisplayListBuilder builder(build_bounds);
builder.setImageFilter(
SkImageFilters::ColorFilter(base_color_filter, nullptr));
builder.saveLayer(&save_bounds, true);
builder.setImageFilter(nullptr);
builder.drawRect(rect);
builder.restore();
sk_sp<DisplayList> display_list = builder.Build();
ASSERT_EQ(display_list->bounds(), rect);
}
{
DisplayListBuilder builder(build_bounds);
builder.setImageFilter(
SkImageFilters::ColorFilter(alpha_color_filter, nullptr));
builder.saveLayer(&save_bounds, true);
builder.setImageFilter(nullptr);
builder.drawRect(rect);
builder.restore();
sk_sp<DisplayList> display_list = builder.Build();
ASSERT_EQ(display_list->bounds(), save_bounds);
}
{
DisplayListBuilder builder(build_bounds);
builder.setImageFilter(
SkImageFilters::ColorFilter(alpha_color_filter, nullptr));
builder.saveLayer(nullptr, true);
builder.setImageFilter(nullptr);
builder.drawRect(rect);
builder.restore();
sk_sp<DisplayList> display_list = builder.Build();
ASSERT_EQ(display_list->bounds(), build_bounds);
}
{
DisplayListBuilder builder(build_bounds);
builder.setBlendMode(SkBlendMode::kClear);
builder.saveLayer(&save_bounds, true);
builder.setBlendMode(SkBlendMode::kSrcOver);
builder.drawRect(rect);
builder.restore();
sk_sp<DisplayList> display_list = builder.Build();
ASSERT_EQ(display_list->bounds(), save_bounds);
}
{
DisplayListBuilder builder(build_bounds);
builder.setBlendMode(SkBlendMode::kClear);
builder.saveLayer(nullptr, true);
builder.setBlendMode(SkBlendMode::kSrcOver);
builder.drawRect(rect);
builder.restore();
sk_sp<DisplayList> display_list = builder.Build();
ASSERT_EQ(display_list->bounds(), build_bounds);
}
}
} // namespace testing
} // namespace flutter

View File

@ -175,27 +175,25 @@ void DisplayListBoundsCalculator::saveLayer(const SkRect* bounds,
bool with_paint) {
SkMatrixDispatchHelper::save();
ClipBoundsDispatchHelper::save();
SaveInfo info =
with_paint ? SaveLayerWithPaintInfo(this, accumulator_, matrix(), paint())
: SaveLayerInfo(accumulator_, matrix());
saved_infos_.push_back(info);
accumulator_ = info.save();
saved_infos_.emplace_back(
with_paint ? std::make_unique<SaveLayerWithPaintInfo>(
this, accumulator_, matrix(), bounds, paint())
: std::make_unique<SaveLayerInfo>(accumulator_, matrix()));
accumulator_ = saved_infos_.back()->save();
SkMatrixDispatchHelper::reset();
}
void DisplayListBoundsCalculator::save() {
SkMatrixDispatchHelper::save();
ClipBoundsDispatchHelper::save();
SaveInfo info = SaveInfo(accumulator_);
saved_infos_.push_back(info);
accumulator_ = info.save();
saved_infos_.emplace_back(std::make_unique<SaveInfo>(accumulator_));
accumulator_ = saved_infos_.back()->save();
}
void DisplayListBoundsCalculator::restore() {
if (!saved_infos_.empty()) {
SkMatrixDispatchHelper::restore();
ClipBoundsDispatchHelper::restore();
SaveInfo info = saved_infos_.back();
accumulator_ = saved_infos_.back()->restore();
saved_infos_.pop_back();
accumulator_ = info.restore();
}
}
@ -388,6 +386,7 @@ BoundsAccumulator* DisplayListBoundsCalculator::SaveLayerInfo::save() {
}
BoundsAccumulator* DisplayListBoundsCalculator::SaveLayerInfo::restore() {
SkRect layer_bounds = layer_accumulator_.getBounds();
layer_bounds.roundOut(&layer_bounds);
matrix_.mapRect(&layer_bounds);
saved_accumulator_->accumulate(layer_bounds);
return saved_accumulator_;
@ -397,21 +396,118 @@ DisplayListBoundsCalculator::SaveLayerWithPaintInfo::SaveLayerWithPaintInfo(
DisplayListBoundsCalculator* calculator,
BoundsAccumulator* accumulator,
const SkMatrix& saveMatrix,
const SkRect* saveBounds,
const SkPaint& savePaint)
: SaveLayerInfo(accumulator, saveMatrix),
calculator_(calculator),
paint_(savePaint) {}
paint_(savePaint) {
if (saveBounds) {
bounds_.emplace(*saveBounds);
}
}
static bool PaintNopsOnTransparenBlack(const SkPaint& paint) {
SkImageFilter* image_filter = paint.getImageFilter();
// SkImageFilter::canComputeFastBounds tests for transparency behavior
// This test assumes that the blend mode checked down below will
// NOP on transparent black.
if (image_filter && !image_filter->canComputeFastBounds()) {
return false;
}
SkColorFilter* color_filter = paint.getColorFilter();
// We filter the transparent black that is used for the background of a
// saveLayer and make sure it returns transparent black. If it does, then
// the color filter will leave all area surrounding the contents of the
// save layer untouched out to the edge of the output surface.
// This test assumes that the blend mode checked down below will
// NOP on transparent black.
if (color_filter &&
color_filter->filterColor(SK_ColorTRANSPARENT) != SK_ColorTRANSPARENT) {
return false;
}
const auto blend_mode = paint.asBlendMode();
if (!blend_mode) {
return false; // can we query other blenders for this?
}
// Unusual blendmodes require us to process a saved layer
// even with operations outisde the clip.
// For example, DstIn is used by masking layers.
// https://code.google.com/p/skia/issues/detail?id=1291
// https://crbug.com/401593
switch (blend_mode.value()) {
// For each of the following transfer modes, if the source
// alpha is zero (our transparent black), the resulting
// blended pixel is not necessarily equal to the original
// destination pixel.
// Mathematically, any time in the following equations where
// the result is not d assuming source is 0
case SkBlendMode::kClear: // r = 0
case SkBlendMode::kSrc: // r = s
case SkBlendMode::kSrcIn: // r = s * da
case SkBlendMode::kDstIn: // r = d * sa
case SkBlendMode::kSrcOut: // r = s * (1-da)
case SkBlendMode::kDstATop: // r = d*sa + s*(1-da)
case SkBlendMode::kModulate: // r = s*d
return false;
break;
// And in these equations, the result must be d if the
// source is 0
case SkBlendMode::kDst: // r = d
case SkBlendMode::kSrcOver: // r = s + (1-sa)*d
case SkBlendMode::kDstOver: // r = d + (1-da)*s
case SkBlendMode::kDstOut: // r = d * (1-sa)
case SkBlendMode::kSrcATop: // r = s*da + d*(1-sa)
case SkBlendMode::kXor: // r = s*(1-da) + d*(1-sa)
case SkBlendMode::kPlus: // r = min(s + d, 1)
case SkBlendMode::kScreen: // r = s + d - s*d
case SkBlendMode::kOverlay: // multiply or screen, depending on dest
case SkBlendMode::kDarken: // rc = s + d - max(s*da, d*sa),
// ra = kSrcOver
case SkBlendMode::kLighten: // rc = s + d - min(s*da, d*sa),
// ra = kSrcOver
case SkBlendMode::kColorDodge: // brighten destination to reflect source
case SkBlendMode::kColorBurn: // darken destination to reflect source
case SkBlendMode::kHardLight: // multiply or screen, depending on source
case SkBlendMode::kSoftLight: // lighten or darken, depending on source
case SkBlendMode::kDifference: // rc = s + d - 2*(min(s*da, d*sa)),
// ra = kSrcOver
case SkBlendMode::kExclusion: // rc = s + d - two(s*d), ra = kSrcOver
case SkBlendMode::kMultiply: // r = s*(1-da) + d*(1-sa) + s*d
case SkBlendMode::kHue: // ra = kSrcOver
case SkBlendMode::kSaturation: // ra = kSrcOver
case SkBlendMode::kColor: // ra = kSrcOver
case SkBlendMode::kLuminosity: // ra = kSrcOver
return true;
break;
}
}
BoundsAccumulator*
DisplayListBoundsCalculator::SaveLayerWithPaintInfo::restore() {
SkRect layer_bounds = layer_accumulator_.getBounds();
if (paint_.canComputeFastBounds()) {
SkRect layer_bounds;
if (paint_.canComputeFastBounds() && PaintNopsOnTransparenBlack(paint_)) {
// The ideal situation. The paint can compute the bounds AND the
// surrounding transparent pixels will not affect the destination.
layer_bounds = layer_accumulator_.getBounds();
layer_bounds = paint_.computeFastBounds(layer_bounds, &layer_bounds);
matrix_.mapRect(&layer_bounds);
saved_accumulator_->accumulate(layer_bounds);
} else if (bounds_.has_value()) {
// Bounds were provided by the save layer, the operation will affect
// all of those bounds.
layer_bounds = bounds_.value();
} else {
// Bounds were not provided for the save layer. We will fill to the
// cull bounds provided to the original DisplayList.
calculator_->root_accumulator_.accumulate(calculator_->bounds_cull_);
// There is no need to process the layer bounds further as we just
// expanded bounds to the cull rect of the DisplayList.
return saved_accumulator_;
}
layer_bounds.roundOut(&layer_bounds);
matrix_.mapRect(&layer_bounds);
saved_accumulator_->accumulate(layer_bounds);
return saved_accumulator_;
}

View File

@ -5,7 +5,11 @@
#ifndef FLUTTER_FLOW_DISPLAY_LIST_UTILS_H_
#define FLUTTER_FLOW_DISPLAY_LIST_UTILS_H_
#include <optional>
#include "flutter/flow/display_list.h"
#include "flutter/fml/logging.h"
#include "flutter/fml/macros.h"
#include "third_party/skia/include/core/SkMaskFilter.h"
@ -313,7 +317,10 @@ class DisplayListBoundsCalculator final
bool occludes,
SkScalar dpr) override;
SkRect getBounds() { return accumulator_->getBounds(); }
SkRect getBounds() {
FML_DCHECK(accumulator_ == &root_accumulator_);
return root_accumulator_.getBounds();
}
private:
// current accumulator based on saveLayer history
@ -334,6 +341,9 @@ class DisplayListBoundsCalculator final
protected:
BoundsAccumulator* saved_accumulator_;
private:
FML_DISALLOW_COPY_AND_ASSIGN(SaveInfo);
};
class SaveLayerInfo : public SaveInfo {
@ -347,6 +357,9 @@ class DisplayListBoundsCalculator final
protected:
BoundsAccumulator layer_accumulator_;
const SkMatrix matrix_;
private:
FML_DISALLOW_COPY_AND_ASSIGN(SaveLayerInfo);
};
class SaveLayerWithPaintInfo : public SaveLayerInfo {
@ -354,6 +367,7 @@ class DisplayListBoundsCalculator final
SaveLayerWithPaintInfo(DisplayListBoundsCalculator* calculator,
BoundsAccumulator* accumulator,
const SkMatrix& save_matrix,
const SkRect* bounds,
const SkPaint& save_paint);
virtual ~SaveLayerWithPaintInfo() = default;
@ -362,10 +376,16 @@ class DisplayListBoundsCalculator final
protected:
DisplayListBoundsCalculator* calculator_;
std::optional<SkRect> bounds_;
SkPaint paint_;
private:
static constexpr SkRect kMissingBounds = SkRect::MakeWH(-1, -1);
FML_DISALLOW_COPY_AND_ASSIGN(SaveLayerWithPaintInfo);
};
std::vector<SaveInfo> saved_infos_;
std::vector<std::unique_ptr<SaveInfo>> saved_infos_;
void accumulateRect(const SkRect& rect, bool force_stroke = false);
};