Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Use DisplayListMatrixClipTracker in DisplayListBuilder #38349

Merged
auto-submit merged 9 commits into from
Dec 20, 2022
Merged

Use DisplayListMatrixClipTracker in DisplayListBuilder #38349

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
0b94e62
Use DisplayListMatrixClipTracker in DisplayListBuilder
ColdPaleLight Dec 16, 2022
b201dad
Ignore is_aa
ColdPaleLight Dec 16, 2022
d71c31c
Revert "Ignore is_aa"
ColdPaleLight Dec 16, 2022
88aa3ce
Tweak code
ColdPaleLight Dec 19, 2022
24c72bd
Use content_culled
ColdPaleLight Dec 19, 2022
0810362
getLocalClipBounds without device clip bounds roundsOut
ColdPaleLight Dec 19, 2022
0aee025
Tweak code and add more tests
ColdPaleLight Dec 20, 2022
c6f674c
remove virtual
ColdPaleLight Dec 20, 2022
9a6e830
Merge branch 'flutter:main' into dlbuilder_refactor
ColdPaleLight Dec 20, 2022
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
83 changes: 29 additions & 54 deletions display_list/display_list_builder.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,16 +67,16 @@ sk_sp<DisplayList> DisplayListBuilder::Build() {
}

DisplayListBuilder::DisplayListBuilder(const SkRect& cull_rect,
bool prepare_rtree) {
bool prepare_rtree)
: tracker_(cull_rect.isEmpty() ? SkRect::MakeEmpty() : cull_rect,
SkMatrix::I()) {
Comment thread
ColdPaleLight marked this conversation as resolved.
Outdated
if (prepare_rtree) {
accumulator_ = std::make_unique<RTreeBoundsAccumulator>();
} else {
accumulator_ = std::make_unique<RectBoundsAccumulator>();
}

// isEmpty protects us against NaN as we normalize any empty cull rects
SkRect cull = cull_rect.isEmpty() ? SkRect::MakeEmpty() : cull_rect;
layer_stack_.emplace_back(SkM44(), SkMatrix::I(), cull);
layer_stack_.emplace_back(LayerInfo());
Comment thread
flar marked this conversation as resolved.
Outdated
current_layer_ = &layer_stack_.back();
}

Expand Down Expand Up @@ -443,6 +443,7 @@ void DisplayListBuilder::save() {
layer_stack_.emplace_back(current_layer_);
current_layer_ = &layer_stack_.back();
current_layer_->has_deferred_save_op_ = true;
tracker_.save();
accumulator()->save();
}

Expand All @@ -455,6 +456,7 @@ void DisplayListBuilder::restore() {
// on the stack.
LayerInfo layer_info = layer_stack_.back();

tracker_.restore();
layer_stack_.pop_back();
current_layer_ = &layer_stack_.back();
bool is_unbounded = layer_info.is_unbounded();
Expand All @@ -463,7 +465,7 @@ void DisplayListBuilder::restore() {
// current accumulator and adjust it as required based on the filter.
std::shared_ptr<const DlImageFilter> filter = layer_info.filter();
if (filter) {
const SkRect* clip = &current_layer_->clip_bounds();
const SkRect clip = tracker_.device_cull_rect();
if (!accumulator()->restore(
[filter = filter, matrix = getTransform()](const SkRect& input,
SkRect& output) {
Expand All @@ -473,7 +475,7 @@ void DisplayListBuilder::restore() {
output.set(output_bounds);
return ret;
},
clip)) {
&clip)) {
is_unbounded = true;
}
} else {
Expand Down Expand Up @@ -549,6 +551,7 @@ void DisplayListBuilder::saveLayer(const SkRect* bounds,
} else {
layer_stack_.emplace_back(current_layer_, save_layer_offset, true, nullptr);
}
tracker_.save();
accumulator()->save();
current_layer_ = &layer_stack_.back();
if (options.renders_with_attributes()) {
Expand All @@ -566,7 +569,7 @@ void DisplayListBuilder::saveLayer(const SkRect* bounds,
// use them as the temporary layer bounds during rendering the layer, so
// we set them as if a clip operation were performed.
if (bounds) {
intersect(*bounds);
tracker_.clipRect(*bounds, SkClipOp::kIntersect, false);
}
if (backdrop) {
// A backdrop will affect up to the entire surface, bounded by the clip
Expand All @@ -590,34 +593,30 @@ void DisplayListBuilder::translate(SkScalar tx, SkScalar ty) {
(tx != 0.0 || ty != 0.0)) {
checkForDeferredSave();
Push<TranslateOp>(0, 1, tx, ty);
current_layer_->matrix().preTranslate(tx, ty);
current_layer_->update_matrix33();
tracker_.translate(tx, ty);
}
}
void DisplayListBuilder::scale(SkScalar sx, SkScalar sy) {
if (SkScalarIsFinite(sx) && SkScalarIsFinite(sy) &&
(sx != 1.0 || sy != 1.0)) {
checkForDeferredSave();
Push<ScaleOp>(0, 1, sx, sy);
current_layer_->matrix().preScale(sx, sy);
current_layer_->update_matrix33();
tracker_.scale(sx, sy);
}
}
void DisplayListBuilder::rotate(SkScalar degrees) {
if (SkScalarMod(degrees, 360.0) != 0.0) {
checkForDeferredSave();
Push<RotateOp>(0, 1, degrees);
current_layer_->matrix().preConcat(SkMatrix::RotateDeg(degrees));
current_layer_->update_matrix33();
tracker_.rotate(degrees);
}
}
void DisplayListBuilder::skew(SkScalar sx, SkScalar sy) {
if (SkScalarIsFinite(sx) && SkScalarIsFinite(sy) &&
(sx != 0.0 || sy != 0.0)) {
checkForDeferredSave();
Push<SkewOp>(0, 1, sx, sy);
current_layer_->matrix().preConcat(SkMatrix::Skew(sx, sy));
current_layer_->update_matrix33();
tracker_.skew(sx, sy);
}
}

Expand All @@ -636,11 +635,10 @@ void DisplayListBuilder::transform2DAffine(
Push<Transform2DAffineOp>(0, 1,
mxx, mxy, mxt,
myx, myy, myt);
current_layer_->matrix().preConcat(SkM44(mxx, mxy, 0, mxt,
myx, myy, 0, myt,
0, 0, 1, 0,
0, 0, 0, 1));
current_layer_->update_matrix33();
tracker_.transform(SkM44(mxx, mxy, 0, mxt,
Comment thread
ColdPaleLight marked this conversation as resolved.
Outdated
myx, myy, 0, myt,
0, 0, 1, 0,
0, 0, 0, 1));
}
}
// full 4x4 transform in row major order
Expand All @@ -665,19 +663,17 @@ void DisplayListBuilder::transformFullPerspective(
myx, myy, myz, myt,
mzx, mzy, mzz, mzt,
mwx, mwy, mwz, mwt);
current_layer_->matrix().preConcat(SkM44(mxx, mxy, mxz, mxt,
myx, myy, myz, myt,
mzx, mzy, mzz, mzt,
mwx, mwy, mwz, mwt));
current_layer_->update_matrix33();
tracker_.transform(SkM44(mxx, mxy, mxz, mxt,
myx, myy, myz, myt,
mzx, mzy, mzz, mzt,
mwx, mwy, mwz, mwt));
}
}
// clang-format on
void DisplayListBuilder::transformReset() {
checkForDeferredSave();
Push<TransformResetOp>(0, 0);
current_layer_->matrix().setIdentity();
current_layer_->update_matrix33();
tracker_.setIdentity();
}
void DisplayListBuilder::transform(const SkMatrix* matrix) {
if (matrix != nullptr) {
Expand All @@ -704,12 +700,12 @@ void DisplayListBuilder::clipRect(const SkRect& rect,
switch (clip_op) {
case SkClipOp::kIntersect:
Push<ClipIntersectRectOp>(0, 1, rect, is_aa);
intersect(rect);
break;
case SkClipOp::kDifference:
Push<ClipDifferenceRectOp>(0, 1, rect, is_aa);
break;
}
tracker_.clipRect(rect, clip_op, is_aa);
}
void DisplayListBuilder::clipRRect(const SkRRect& rrect,
SkClipOp clip_op,
Expand All @@ -721,12 +717,12 @@ void DisplayListBuilder::clipRRect(const SkRRect& rrect,
switch (clip_op) {
case SkClipOp::kIntersect:
Push<ClipIntersectRRectOp>(0, 1, rrect, is_aa);
intersect(rrect.getBounds());
break;
case SkClipOp::kDifference:
Push<ClipDifferenceRRectOp>(0, 1, rrect, is_aa);
break;
}
tracker_.clipRRect(rrect, clip_op, is_aa);
}
}
void DisplayListBuilder::clipPath(const SkPath& path,
Expand All @@ -753,33 +749,12 @@ void DisplayListBuilder::clipPath(const SkPath& path,
switch (clip_op) {
case SkClipOp::kIntersect:
Push<ClipIntersectPathOp>(0, 1, path, is_aa);
if (!path.isInverseFillType()) {
intersect(path.getBounds());
}
break;
case SkClipOp::kDifference:
Push<ClipDifferencePathOp>(0, 1, path, is_aa);
// Map "kDifference of inverse path" to "kIntersect of the original path".
if (path.isInverseFillType()) {
intersect(path.getBounds());
}
break;
}
}
void DisplayListBuilder::intersect(const SkRect& rect) {
SkRect dev_clip_bounds = getTransform().mapRect(rect);
if (!current_layer_->clip_bounds().intersect(dev_clip_bounds)) {
current_layer_->clip_bounds().setEmpty();
}
}
SkRect DisplayListBuilder::getLocalClipBounds() {
SkM44 inverse;
if (current_layer_->matrix().invert(&inverse)) {
Comment thread
flar marked this conversation as resolved.
SkRect dev_bounds;
current_layer_->clip_bounds().roundOut(&dev_bounds);
return inverse.asM33().mapRect(dev_bounds);
}
return kMaxCullRect;
tracker_.clipPath(path, clip_op, is_aa);
}

bool DisplayListBuilder::quickReject(const SkRect& bounds) const {
Expand All @@ -794,7 +769,7 @@ bool DisplayListBuilder::quickReject(const SkRect& bounds) const {
}
SkRect dev_bounds;
matrix.mapRect(bounds).roundOut(&dev_bounds);
return !current_layer_->clip_bounds().intersects(dev_bounds);
return !tracker_.device_cull_rect().intersects(dev_bounds);
Comment thread
flar marked this conversation as resolved.
Outdated
}

void DisplayListBuilder::drawPaint() {
Expand Down Expand Up @@ -1357,7 +1332,7 @@ bool DisplayListBuilder::AdjustBoundsForPaint(SkRect& bounds,
}

void DisplayListBuilder::AccumulateUnbounded() {
accumulator()->accumulate(current_layer_->clip_bounds());
accumulator()->accumulate(tracker_.device_cull_rect());
}

void DisplayListBuilder::AccumulateOpBounds(SkRect& bounds,
Expand All @@ -1370,7 +1345,7 @@ void DisplayListBuilder::AccumulateOpBounds(SkRect& bounds,
}
void DisplayListBuilder::AccumulateBounds(SkRect& bounds) {
getTransform().mapRect(&bounds);
if (bounds.intersect(current_layer_->clip_bounds())) {
if (bounds.intersect(tracker_.device_cull_rect())) {
Comment thread
flar marked this conversation as resolved.
accumulator()->accumulate(bounds);
}
}
Expand Down
33 changes: 8 additions & 25 deletions display_list/display_list_builder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "flutter/display_list/display_list_dispatcher.h"
#include "flutter/display_list/display_list_flags.h"
#include "flutter/display_list/display_list_image.h"
#include "flutter/display_list/display_list_matrix_clip_tracker.h"
#include "flutter/display_list/display_list_paint.h"
#include "flutter/display_list/display_list_path_effect.h"
#include "flutter/display_list/display_list_sampling_options.h"
Expand Down Expand Up @@ -210,11 +211,11 @@ class DisplayListBuilder final : public virtual Dispatcher,
/// Returns the 4x4 full perspective transform representing all transform
/// operations executed so far in this DisplayList within the enclosing
/// save stack.
SkM44 getTransformFullPerspective() const { return current_layer_->matrix(); }
SkM44 getTransformFullPerspective() const { return tracker_.matrix_4x4(); }
/// Returns the 3x3 partial perspective transform representing all transform
/// operations executed so far in this DisplayList within the enclosing
/// save stack.
SkMatrix getTransform() const { return current_layer_->matrix33(); }
SkMatrix getTransform() const { return tracker_.matrix_3x3(); }

void clipRect(const SkRect& rect, SkClipOp clip_op, bool is_aa) override;
void clipRRect(const SkRRect& rrect, SkClipOp clip_op, bool is_aa) override;
Expand All @@ -223,11 +224,11 @@ class DisplayListBuilder final : public virtual Dispatcher,
/// Conservative estimate of the bounds of all outstanding clip operations
/// measured in the coordinate space within which this DisplayList will
/// be rendered.
SkRect getDestinationClipBounds() { return current_layer_->clip_bounds(); }
SkRect getDestinationClipBounds() { return tracker_.device_cull_rect(); }
/// Conservative estimate of the bounds of all outstanding clip operations
/// transformed into the local coordinate space in which currently
/// recorded rendering operations are interpreted.
SkRect getLocalClipBounds();
SkRect getLocalClipBounds() { return tracker_.local_cull_rect(); }

/// Return true iff the supplied bounds are easily shown to be outside
/// of the current clip bounds. This method may conservatively return
Expand Down Expand Up @@ -386,32 +387,21 @@ class DisplayListBuilder final : public virtual Dispatcher,

class LayerInfo {
public:
explicit LayerInfo(const SkM44& matrix,
const SkMatrix& matrix33,
const SkRect& clip_bounds,
size_t save_layer_offset = 0,
explicit LayerInfo(size_t save_layer_offset = 0,
bool has_layer = false,
std::shared_ptr<const DlImageFilter> filter = nullptr)
: save_layer_offset_(save_layer_offset),
has_layer_(has_layer),
cannot_inherit_opacity_(false),
has_compatible_op_(false),
matrix_(matrix),
matrix33_(matrix33),
clip_bounds_(clip_bounds),
filter_(filter),
is_unbounded_(false) {}

explicit LayerInfo(const LayerInfo* current_layer,
size_t save_layer_offset = 0,
bool has_layer = false,
std::shared_ptr<const DlImageFilter> filter = nullptr)
: LayerInfo(current_layer->matrix_,
current_layer->matrix33_,
current_layer->clip_bounds_,
save_layer_offset,
has_layer,
filter) {}
: LayerInfo(save_layer_offset, has_layer, filter) {}
Comment thread
ColdPaleLight marked this conversation as resolved.
Outdated

// The offset into the memory buffer where the saveLayer DLOp record
// for this saveLayer() call is placed. This may be needed if the
Expand All @@ -424,11 +414,6 @@ class DisplayListBuilder final : public virtual Dispatcher,
bool has_layer() const { return has_layer_; }
bool cannot_inherit_opacity() const { return cannot_inherit_opacity_; }
bool has_compatible_op() const { return cannot_inherit_opacity_; }
SkM44& matrix() { return matrix_; }
SkMatrix& matrix33() { return matrix33_; }
SkRect& clip_bounds() { return clip_bounds_; }

void update_matrix33() { matrix33_ = matrix_.asM33(); }

bool is_group_opacity_compatible() const {
return !cannot_inherit_opacity_;
Expand Down Expand Up @@ -486,9 +471,6 @@ class DisplayListBuilder final : public virtual Dispatcher,
bool has_layer_;
bool cannot_inherit_opacity_;
bool has_compatible_op_;
SkM44 matrix_;
SkMatrix matrix33_;
SkRect clip_bounds_;
std::shared_ptr<const DlImageFilter> filter_;
bool is_unbounded_;
bool has_deferred_save_op_ = false;
Expand Down Expand Up @@ -640,6 +622,7 @@ class DisplayListBuilder final : public virtual Dispatcher,
DlPaint current_;
// If |current_blender_| is set then ignore |current_.getBlendMode()|
sk_sp<SkBlender> current_blender_;
DisplayListMatrixClipTracker tracker_;
Comment thread
ColdPaleLight marked this conversation as resolved.
Outdated
};

} // namespace flutter
Expand Down
17 changes: 16 additions & 1 deletion display_list/display_list_matrix_clip_tracker.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,19 @@ void DisplayListMatrixClipTracker::clipRRect(const SkRRect& rrect,
void DisplayListMatrixClipTracker::clipPath(const SkPath& path,
SkClipOp op,
bool is_aa) {
// Map "kDifference of inverse path" to "kIntersect of the original path" and
// map "kIntersect of inverse path" to "kDifference of the original path"
if (path.isInverseFillType()) {
switch (op) {
case SkClipOp::kIntersect:
op = SkClipOp::kDifference;
break;
case SkClipOp::kDifference:
op = SkClipOp::kIntersect;
break;
}
}

SkRect bounds;
switch (op) {
case SkClipOp::kIntersect:
Expand Down Expand Up @@ -323,7 +336,9 @@ SkRect Data3x3::local_cull_rect() const {
// cull rect.
return DisplayListBuilder::kMaxCullRect;
}
return inverse.mapRect(cull_rect_);
SkRect expended_rect;

@flar flar Dec 16, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about the version in Data4x4?

@flar flar Dec 16, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is dubious, btw, because we could be computing local bounds for something that won't yet have its final transform, so the roundOut may be too much or not enough depending on a scale that happens outside the picture recording...

@ColdPaleLight ColdPaleLight Dec 19, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is roundOut logic in DLBuilder. If it is not change here, some tests in display_list_unittests.cc and canvas_test.dart will fail.

So we should change the tests instead of add roundOut here, right?

cull_rect_.roundOut(&expended_rect);
return inverse.mapRect(expended_rect);
}

} // namespace flutter
Loading