Commit 0fde9a92 authored by Denis Bessonov's avatar Denis Bessonov Committed by Commit Bot

Revert "Fix 0,0-0x0 turning into PaintOp::kUnsetRect."

This reverts commit 0156c949.

Reason for revert: series of review fixes resulted in a no-op code. The completely different approach should be used to fix the bug correctly.

Original change's description:
> Fix 0,0-0x0 turning into PaintOp::kUnsetRect.
>
> When a paint chunk having 0,0-0x0 rect is processed inside an effect, an early
> return prevents the effect_bounds_stack_.back() update in a
> ConversionContext::UpdateEffectBounds call. This results in keeping default
> PaintOp::kUnsetRect in SaveLayerOp::bounds instead of 0,0-0x0 rect.
>
> This is probably a reason for a related bug 918240 (see bug thread Comment 4 for
> details). Application of this patch fixes the lagging print preview.
>
> R=​fmalita@chromium.org, samans@chromium.org
>
> Bug: 918240
> Change-Id: I022ae01418e83184421cff7e22566228a2f7c6d3
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1396958
> Commit-Queue: Denis Bessonov <dbessonov@yandex-team.ru>
> Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#714293}

Bug: 918240
Change-Id: I89a55d39a53559b37168b91adf3f331dbbf8b106
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2007409
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Auto-Submit: Denis Bessonov <dbessonov@yandex-team.ru>
Reviewed-by: default avatarXianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#733491}
parent ae37f0f8
...@@ -91,8 +91,6 @@ class CC_PAINT_EXPORT DisplayItemList ...@@ -91,8 +91,6 @@ class CC_PAINT_EXPORT DisplayItemList
return offset; return offset;
} }
UsageHint GetUsageHint() const { return usage_hint_; }
// Called by blink::PaintChunksToCcLayer when an effect ends, to update the // Called by blink::PaintChunksToCcLayer when an effect ends, to update the
// bounds of a SaveLayer[Alpha]Op which was emitted when the effect started. // bounds of a SaveLayer[Alpha]Op which was emitted when the effect started.
// This is needed because blink doesn't know the bounds when an effect starts. // This is needed because blink doesn't know the bounds when an effect starts.
......
...@@ -172,8 +172,7 @@ class ConversionContext { ...@@ -172,8 +172,7 @@ class ConversionContext {
// Ends the effect on the top of the state stack if the stack is not empty, // Ends the effect on the top of the state stack if the stack is not empty,
// and update the bounds of the SaveLayer[Alpha]Op of the effect. // and update the bounds of the SaveLayer[Alpha]Op of the effect.
void EndEffect(); void EndEffect();
void UpdateEffectBounds(const base::Optional<FloatRect>&, void UpdateEffectBounds(const FloatRect&, const TransformPaintPropertyNode&);
const TransformPaintPropertyNode&);
// Starts a clip state by adjusting the transform state, applying // Starts a clip state by adjusting the transform state, applying
// |combined_clip_rect| which is combined from one or more consecutive clips, // |combined_clip_rect| which is combined from one or more consecutive clips,
...@@ -244,7 +243,7 @@ class ConversionContext { ...@@ -244,7 +243,7 @@ class ConversionContext {
// Records the bounds of the effect which initiated the entry. Note that // Records the bounds of the effect which initiated the entry. Note that
// the effect is not |this->effect| (which is the previous effect), but the // the effect is not |this->effect| (which is the previous effect), but the
// |current_effect_| when this entry is the top of the stack. // |current_effect_| when this entry is the top of the stack.
base::Optional<FloatRect> bounds; FloatRect bounds;
}; };
Vector<EffectBoundsInfo> effect_bounds_stack_; Vector<EffectBoundsInfo> effect_bounds_stack_;
...@@ -589,25 +588,22 @@ void ConversionContext::StartEffect(const EffectPaintPropertyNode& effect) { ...@@ -589,25 +588,22 @@ void ConversionContext::StartEffect(const EffectPaintPropertyNode& effect) {
const ClipPaintPropertyNode* input_clip = current_clip_; const ClipPaintPropertyNode* input_clip = current_clip_;
PushState(StateEntry::kEffect, saved_count); PushState(StateEntry::kEffect, saved_count);
effect_bounds_stack_.emplace_back( effect_bounds_stack_.emplace_back(
EffectBoundsInfo{save_layer_id, current_transform_, base::nullopt}); EffectBoundsInfo{save_layer_id, current_transform_});
current_clip_ = input_clip; current_clip_ = input_clip;
current_effect_ = &effect; current_effect_ = &effect;
} }
void ConversionContext::UpdateEffectBounds( void ConversionContext::UpdateEffectBounds(
const base::Optional<FloatRect>& bounds, const FloatRect& bounds,
const TransformPaintPropertyNode& transform) { const TransformPaintPropertyNode& transform) {
if (effect_bounds_stack_.IsEmpty() || !bounds) if (effect_bounds_stack_.IsEmpty() || bounds.IsEmpty())
return; return;
auto& effect_bounds_info = effect_bounds_stack_.back(); auto& effect_bounds_info = effect_bounds_stack_.back();
FloatRect mapped_bounds = *bounds; FloatRect mapped_bounds = bounds;
GeometryMapper::SourceToDestinationRect( GeometryMapper::SourceToDestinationRect(
transform, *effect_bounds_info.transform, mapped_bounds); transform, *effect_bounds_info.transform, mapped_bounds);
if (effect_bounds_info.bounds) effect_bounds_info.bounds.Unite(mapped_bounds);
effect_bounds_info.bounds->Unite(mapped_bounds);
else
effect_bounds_info.bounds = mapped_bounds;
} }
void ConversionContext::EndEffect() { void ConversionContext::EndEffect() {
...@@ -621,20 +617,20 @@ void ConversionContext::EndEffect() { ...@@ -621,20 +617,20 @@ void ConversionContext::EndEffect() {
DCHECK(effect_bounds_stack_.size()); DCHECK(effect_bounds_stack_.size());
const auto& bounds_info = effect_bounds_stack_.back(); const auto& bounds_info = effect_bounds_stack_.back();
base::Optional<FloatRect> bounds = bounds_info.bounds; FloatRect bounds = bounds_info.bounds;
if (bounds) { if (!bounds.IsEmpty()) {
if (current_effect_->Filter().IsEmpty()) { if (current_effect_->Filter().IsEmpty()) {
cc_list_.UpdateSaveLayerBounds(bounds_info.save_layer_id, *bounds); cc_list_.UpdateSaveLayerBounds(bounds_info.save_layer_id, bounds);
} else { } else {
// The bounds for the SaveLayer[Alpha]Op should be the source bounds // The bounds for the SaveLayer[Alpha]Op should be the source bounds
// before the filter is applied, in the space of the TranslateOp which was // before the filter is applied, in the space of the TranslateOp which was
// emitted before the SaveLayer[Alpha]Op. // emitted before the SaveLayer[Alpha]Op.
auto save_layer_bounds = *bounds; auto save_layer_bounds = bounds;
save_layer_bounds.MoveBy(-current_effect_->FiltersOrigin()); save_layer_bounds.MoveBy(-current_effect_->FiltersOrigin());
cc_list_.UpdateSaveLayerBounds(bounds_info.save_layer_id, cc_list_.UpdateSaveLayerBounds(bounds_info.save_layer_id,
save_layer_bounds); save_layer_bounds);
// We need to propagate the filtered bounds to the parent. // We need to propagate the filtered bounds to the parent.
bounds = current_effect_->MapRect(*bounds); bounds = current_effect_->MapRect(bounds);
} }
} }
...@@ -753,17 +749,7 @@ void ConversionContext::Convert(const PaintChunkSubset& paint_chunks, ...@@ -753,17 +749,7 @@ void ConversionContext::Convert(const PaintChunkSubset& paint_chunks,
cc_list_.EndPaintOfUnpaired( cc_list_.EndPaintOfUnpaired(
chunk_to_layer_mapper_.MapVisualRect(item.VisualRect())); chunk_to_layer_mapper_.MapVisualRect(item.VisualRect()));
} }
UpdateEffectBounds(FloatRect(chunk.bounds), chunk_state.Transform());
// Chunk bounds are only important when we are actually drawing. There may
// also be cases when we only generate a paint record and do not draw,
// for example, to implement an SVG clip. In such cases, we can safely
// ignore effect bounds.
base::Optional<FloatRect> chunk_bounds;
if (cc_list_.GetUsageHint() ==
cc::DisplayItemList::kTopLevelDisplayItemList) {
chunk_bounds = FloatRect(chunk.bounds);
}
UpdateEffectBounds(chunk_bounds, chunk_state.Transform());
} }
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment