Skip to content

GitLab

T
tangled
Commit ec37d4cc authored by Xianzhu Wang's avatar Xianzhu Wang Committed by Commit Bot
Browse files
Options

[PE] Fix subpixel accumulation for composited content layers 

The content box rect was improperly snapped in the local space.
We should snap in the layout space instead, including subpixel
accumulation.

This is a duplicate of trchen@'s old CL
https://codereview.chromium.org/2172503002/ rebased on the current
trunk.

Bug: 627683
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_layout_tests_slimming_paint_v2;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I737871c4bafead28f1040d486a59f3203c0b0ead
Reviewed-on: https://chromium-review.googlesource.com/c/1271582
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: default avatarKenneth Russell <kbr@chromium.org>
Reviewed-by: default avatarChris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600811}
parent 431f4144
Hide whitespace changes
Inline Side-by-side
Showing with 89 additions and 120 deletions
content/test/gpu/gpu_tests/pixel_expectations.py
View file @ ec37d4cc
...@@ -131,6 +131,11 @@ class PixelExpectations(GpuTestExpectations): ...@@ -131,6 +131,11 @@ class PixelExpectations(GpuTestExpectations):
self.Fail('Pixel_OffscreenCanvasWebGLPaintAfterResize', self.Fail('Pixel_OffscreenCanvasWebGLPaintAfterResize',
['android', 'nvidia'], bug=868596) ['android', 'nvidia'], bug=868596)
# TODO(wangxianzhu): temporarily suppress these two tests until new
# reference images are generated.
self.Fail('Pixel_OffscreenCanvasTransferToImageBitmap', bug=627683)
self.Fail('Pixel_OffscreenCanvasTransferToImageBitmapWorker', bug=627683)
# Fails on Nexus 5, 6 and 6P # Fails on Nexus 5, 6 and 6P
self.Fail('Pixel_BackgroundImage', self.Fail('Pixel_BackgroundImage',
['android', ('qualcomm', 'Adreno (TM) 330')], bug=883500) ['android', ('qualcomm', 'Adreno (TM) 330')], bug=883500)
......
content/test/gpu/gpu_tests/pixel_test_pages.py
View file @ ec37d4cc
...@@ -445,14 +445,14 @@ def ExperimentalCanvasFeaturesPages(base_name): ...@@ -445,14 +445,14 @@ def ExperimentalCanvasFeaturesPages(base_name):
'pixel_offscreenCanvas_transferToImageBitmap_main.html', 'pixel_offscreenCanvas_transferToImageBitmap_main.html',
base_name + '_OffscreenCanvasTransferToImageBitmap', base_name + '_OffscreenCanvasTransferToImageBitmap',
test_rect=[0, 0, 300, 300], test_rect=[0, 0, 300, 300],
revision=5, revision=6,
browser_args=browser_args), browser_args=browser_args),
PixelTestPage( PixelTestPage(
'pixel_offscreenCanvas_transferToImageBitmap_worker.html', 'pixel_offscreenCanvas_transferToImageBitmap_worker.html',
base_name + '_OffscreenCanvasTransferToImageBitmapWorker', base_name + '_OffscreenCanvasTransferToImageBitmapWorker',
test_rect=[0, 0, 300, 300], test_rect=[0, 0, 300, 300],
revision=5, revision=6,
browser_args=browser_args), browser_args=browser_args),
PixelTestPage( PixelTestPage(
......
third_party/WebKit/LayoutTests/compositing/composited-replaced-subpixel-location-expected.html 0 → 100644
View file @ ec37d4cc
<!DOCTYPE html>
<style>
img {
width: 40.4px;
height: 30.3px;
}
</style>
<div>
Passes if each image is at the exact same location when they are composited or not.
</div>
<img src="../images/resources/blue-100.png">
<img src="../images/resources/blue-100.png">
<img src="../images/resources/blue-100.png">
<img src="../images/resources/blue-100.png">
<img src="../images/resources/blue-100.png">
<img src="../images/resources/blue-100.png">
<img src="../images/resources/blue-100.png">
<img src="../images/resources/blue-100.png">
<img src="../images/resources/blue-100.png">
<img src="../images/resources/blue-100.png">
<img src="../images/resources/blue-100.png">
third_party/WebKit/LayoutTests/compositing/composited-replaced-subpixel-location.html 0 → 100644
View file @ ec37d4cc
<!DOCTYPE html>
<style>
img {
width: 40.4px;
height: 30.3px;
will-change: transform;
}
</style>
<div>
Passes if each image is at the exact same location when they are composited or not.
</div>
<img src="../images/resources/blue-100.png">
<img src="../images/resources/blue-100.png">
<img src="../images/resources/blue-100.png">
<img src="../images/resources/blue-100.png">
<img src="../images/resources/blue-100.png">
<img src="../images/resources/blue-100.png">
<img src="../images/resources/blue-100.png">
<img src="../images/resources/blue-100.png">
<img src="../images/resources/blue-100.png">
<img src="../images/resources/blue-100.png">
<img src="../images/resources/blue-100.png">
third_party/blink/renderer/core/paint/compositing/composited_layer_mapping.cc
View file @ ec37d4cc
...@@ -84,28 +84,20 @@ namespace blink { ...@@ -84,28 +84,20 @@ namespace blink {
using namespace HTMLNames; using namespace HTMLNames;
static IntRect ContentsRect(const LayoutObject& layout_object) { static LayoutRect ContentsRect(const LayoutObject& layout_object) {
if (!layout_object.IsBox()) if (!layout_object.IsBox())
return IntRect(); return LayoutRect();
if (layout_object.IsCanvas()) { if (layout_object.IsLayoutReplaced())
return PixelSnappedIntRect( return ToLayoutReplaced(layout_object).ReplacedContentRect();
ToLayoutHTMLCanvas(layout_object).ReplacedContentRect()); return ToLayoutBox(layout_object).PhysicalContentBoxRect();
}
if (layout_object.IsVideo()) {
return PixelSnappedIntRect(
ToLayoutVideo(layout_object).ReplacedContentRect());
}
return PixelSnappedIntRect(
ToLayoutBox(layout_object).PhysicalContentBoxRect());
} }
static IntRect BackgroundRect(const LayoutObject& layout_object) { static LayoutRect BackgroundRect(const LayoutObject& layout_object) {
if (!layout_object.IsBox()) if (!layout_object.IsBox())
return IntRect(); return LayoutRect();
const LayoutBox& box = ToLayoutBox(layout_object); const LayoutBox& box = ToLayoutBox(layout_object);
return PixelSnappedIntRect(box.PhysicalBackgroundRect(kBackgroundClipRect)); return box.PhysicalBackgroundRect(kBackgroundClipRect);
} }
static inline bool IsTextureLayerCanvas(const LayoutObject& layout_object) { static inline bool IsTextureLayerCanvas(const LayoutObject& layout_object) {
...@@ -198,7 +190,6 @@ static FloatPoint StickyPositionOffsetForLayer(PaintLayer& layer) { ...@@ -198,7 +190,6 @@ static FloatPoint StickyPositionOffsetForLayer(PaintLayer& layer) {
CompositedLayerMapping::CompositedLayerMapping(PaintLayer& layer) CompositedLayerMapping::CompositedLayerMapping(PaintLayer& layer)
: owning_layer_(layer), : owning_layer_(layer),
content_offset_in_compositing_layer_dirty_(false),
pending_update_scope_(kGraphicsLayerUpdateNone), pending_update_scope_(kGraphicsLayerUpdateNone),
is_main_frame_layout_view_layer_(false), is_main_frame_layout_view_layer_(false),
scrolling_contents_are_empty_(false), scrolling_contents_are_empty_(false),
...@@ -518,7 +509,6 @@ void CompositedLayerMapping::UpdateCompositedBounds() { ...@@ -518,7 +509,6 @@ void CompositedLayerMapping::UpdateCompositedBounds() {
// FIXME: if this is really needed for performance, it would be better to // FIXME: if this is really needed for performance, it would be better to
// store it on Layer. // store it on Layer.
composited_bounds_ = owning_layer_.BoundingBoxForCompositing(); composited_bounds_ = owning_layer_.BoundingBoxForCompositing();
content_offset_in_compositing_layer_dirty_ = true;
} }
GraphicsLayer* CompositedLayerMapping::FrameContentsGraphicsLayer() const { GraphicsLayer* CompositedLayerMapping::FrameContentsGraphicsLayer() const {
...@@ -565,7 +555,7 @@ void CompositedLayerMapping::UpdateAfterPartResize() { ...@@ -565,7 +555,7 @@ void CompositedLayerMapping::UpdateAfterPartResize() {
child_containment_layer_ child_containment_layer_
? FloatPoint(child_containment_layer_->GetPosition()) ? FloatPoint(child_containment_layer_->GetPosition())
: FloatPoint(); : FloatPoint();
document_layer->SetPosition(FloatPoint(FlooredIntPoint( document_layer->SetPosition(FloatPoint(RoundedIntSize(
FloatPoint(ContentsBox().Location()) - parent_position))); FloatPoint(ContentsBox().Location()) - parent_position)));
} }
} }
...@@ -1218,8 +1208,6 @@ void CompositedLayerMapping::UpdateGraphicsLayerGeometry( ...@@ -1218,8 +1208,6 @@ void CompositedLayerMapping::UpdateGraphicsLayerGeometry(
UpdateOverflowControlsHostLayerGeometry(compositing_stacking_context, UpdateOverflowControlsHostLayerGeometry(compositing_stacking_context,
compositing_container, compositing_container,
graphics_layer_parent_location); graphics_layer_parent_location);
UpdateContentsOffsetInCompositingLayer(
snapped_offset_from_composited_ancestor, graphics_layer_parent_location);
UpdateStickyConstraints(GetLayoutObject().StyleRef()); UpdateStickyConstraints(GetLayoutObject().StyleRef());
UpdateSquashingLayerGeometry( UpdateSquashingLayerGeometry(
graphics_layer_parent_location, compositing_container, graphics_layer_parent_location, compositing_container,
...@@ -1606,16 +1594,13 @@ void CompositedLayerMapping::UpdateChildContainmentLayerGeometry() { ...@@ -1606,16 +1594,13 @@ void CompositedLayerMapping::UpdateChildContainmentLayerGeometry() {
void CompositedLayerMapping::UpdateChildTransformLayerGeometry() { void CompositedLayerMapping::UpdateChildTransformLayerGeometry() {
if (!child_transform_layer_) if (!child_transform_layer_)
return; return;
const IntRect border_box =
ToLayoutBox(owning_layer_.GetLayoutObject()) LayoutRect border_box =
.PixelSnappedBorderBoxRect(SubpixelAccumulation()); ToLayoutBox(owning_layer_.GetLayoutObject()).BorderBoxRect();
child_transform_layer_->SetSize(gfx::Size(border_box.Size())); border_box.Move(ContentOffsetInCompositingLayer());
child_transform_layer_->SetOffsetFromLayoutObject( child_transform_layer_->SetSize(gfx::Size(border_box.PixelSnappedSize()));
ToIntSize(border_box.Location())); child_transform_layer_->SetOffsetFromLayoutObject(IntSize());
IntPoint parent_location( child_transform_layer_->SetPosition(FloatPoint(border_box.Location()));
child_transform_layer_->Parent()->OffsetFromLayoutObject());
child_transform_layer_->SetPosition(
FloatPoint(border_box.Location() - parent_location));
} }
void CompositedLayerMapping::UpdateMaskLayerGeometry() { void CompositedLayerMapping::UpdateMaskLayerGeometry() {
...@@ -1913,59 +1898,6 @@ void CompositedLayerMapping::UpdateContentsRect() { ...@@ -1913,59 +1898,6 @@ void CompositedLayerMapping::UpdateContentsRect() {
graphics_layer_->SetContentsRect(PixelSnappedIntRect(ContentsBox())); graphics_layer_->SetContentsRect(PixelSnappedIntRect(ContentsBox()));
} }
void CompositedLayerMapping::UpdateContentsOffsetInCompositingLayer(
const IntPoint& snapped_offset_from_composited_ancestor,
const IntPoint& graphics_layer_parent_location) {
// m_graphicsLayer is positioned relative to our compositing ancestor
// PaintLayer, but it's not positioned at the origin of m_owningLayer, it's
// offset by m_contentBounds.location(). This is what
// contentOffsetInCompositingLayer is meant to capture, roughly speaking
// (ignoring rounding and subpixel accumulation).
//
// Our ancestor graphics layers in this CLM (m_graphicsLayer and potentially
// m_ancestorClippingLayer) have pixel snapped, so if we don't adjust this
// offset, we'll see accumulated rounding errors due to that snapping.
//
// In order to ensure that we account for this rounding, we compute
// contentsOffsetInCompositingLayer in a somewhat roundabout way.
//
// our position = (desired position) - (inherited graphics layer offset).
//
// Precisely,
// Offset = snappedOffsetFromCompositedAncestor -
// offsetDueToAncestorGraphicsLayers (See code below)
// = snappedOffsetFromCompositedAncestor -
// (m_graphicsLayer->position() + graphicsLayerParentLocation)
// = snappedOffsetFromCompositedAncestor -
// (relativeCompositingBounds.location() -
// graphicsLayerParentLocation +
// graphicsLayerParentLocation)
// (See updateMainGraphicsLayerGeometry)
// = snappedOffsetFromCompositedAncestor -
// relativeCompositingBounds.location()
// = snappedOffsetFromCompositedAncestor -
// (pixelSnappedIntRect(contentBounds.location()) +
// snappedOffsetFromCompositedAncestor)
// (See computeBoundsOfOwningLayer)
// = -pixelSnappedIntRect(contentBounds.location())
//
// As you can see, we've ended up at the same spot
// (-contentBounds.location()), but by subtracting off our ancestor graphics
// layers positions, we can be sure we've accounted correctly for any pixel
// snapping due to ancestor graphics layers.
//
// And drawing of composited children takes into account the subpixel
// accumulation of this CLM already (through its own
// graphicsLayerParentLocation it appears).
FloatPoint offset_due_to_ancestor_graphics_layers =
FloatPoint(graphics_layer_->GetPosition()) +
graphics_layer_parent_location;
content_offset_in_compositing_layer_ =
LayoutSize(snapped_offset_from_composited_ancestor -
offset_due_to_ancestor_graphics_layers);
content_offset_in_compositing_layer_dirty_ = false;
}
void CompositedLayerMapping::UpdateDrawsContent() { void CompositedLayerMapping::UpdateDrawsContent() {
bool in_overlay_fullscreen_video = false; bool in_overlay_fullscreen_video = false;
if (GetLayoutObject().IsVideo()) { if (GetLayoutObject().IsVideo()) {
...@@ -2958,12 +2890,12 @@ FloatPoint3D CompositedLayerMapping::ComputeTransformOrigin( ...@@ -2958,12 +2890,12 @@ FloatPoint3D CompositedLayerMapping::ComputeTransformOrigin(
// Return the offset from the top-left of this compositing layer at which the // Return the offset from the top-left of this compositing layer at which the
// LayoutObject's contents are painted. // LayoutObject's contents are painted.
LayoutSize CompositedLayerMapping::ContentOffsetInCompositingLayer() const { LayoutSize CompositedLayerMapping::ContentOffsetInCompositingLayer() const {
DCHECK(!content_offset_in_compositing_layer_dirty_); return owning_layer_.SubpixelAccumulation() -
return content_offset_in_compositing_layer_; LayoutSize(graphics_layer_->OffsetFromLayoutObject());
} }
LayoutRect CompositedLayerMapping::ContentsBox() const { LayoutRect CompositedLayerMapping::ContentsBox() const {
LayoutRect contents_box = LayoutRect(ContentsRect(GetLayoutObject())); LayoutRect contents_box = ContentsRect(GetLayoutObject());
contents_box.Move(ContentOffsetInCompositingLayer()); contents_box.Move(ContentOffsetInCompositingLayer());
return contents_box; return contents_box;
} }
......
third_party/blink/renderer/core/paint/compositing/composited_layer_mapping.h
View file @ ec37d4cc
...@@ -455,9 +455,6 @@ class CORE_EXPORT CompositedLayerMapping final : public GraphicsLayerClient { ...@@ -455,9 +455,6 @@ class CORE_EXPORT CompositedLayerMapping final : public GraphicsLayerClient {
Color LayoutObjectBackgroundColor() const; Color LayoutObjectBackgroundColor() const;
void UpdateBackgroundColor(); void UpdateBackgroundColor();
void UpdateContentsRect(); void UpdateContentsRect();
void UpdateContentsOffsetInCompositingLayer(
const IntPoint& snapped_offset_from_composited_ancestor,
const IntPoint& graphics_layer_parent_location);
void UpdateAfterPartResize(); void UpdateAfterPartResize();
void UpdateCompositingReasons(); void UpdateCompositingReasons();
...@@ -666,8 +663,6 @@ class CORE_EXPORT CompositedLayerMapping final : public GraphicsLayerClient { ...@@ -666,8 +663,6 @@ class CORE_EXPORT CompositedLayerMapping final : public GraphicsLayerClient {
LayoutRect composited_bounds_; LayoutRect composited_bounds_;
LayoutSize content_offset_in_compositing_layer_;
// We keep track of the scrolling contents offset, so that when it changes we // We keep track of the scrolling contents offset, so that when it changes we
// can notify the ScrollingCoordinator, which passes on main-thread scrolling // can notify the ScrollingCoordinator, which passes on main-thread scrolling
// updates to the compositor. // updates to the compositor.
...@@ -675,8 +670,6 @@ class CORE_EXPORT CompositedLayerMapping final : public GraphicsLayerClient { ...@@ -675,8 +670,6 @@ class CORE_EXPORT CompositedLayerMapping final : public GraphicsLayerClient {
const PaintLayer* clip_inheritance_ancestor_; const PaintLayer* clip_inheritance_ancestor_;
unsigned content_offset_in_compositing_layer_dirty_ : 1;
unsigned pending_update_scope_ : 2; unsigned pending_update_scope_ : 2;
unsigned is_main_frame_layout_view_layer_ : 1; unsigned is_main_frame_layout_view_layer_ : 1;
......
third_party/blink/renderer/core/paint/compositing/composited_layer_mapping_test.cc
View file @ ec37d4cc
...@@ -1177,35 +1177,35 @@ TEST_F(CompositedLayerMappingTest, ...@@ -1177,35 +1177,35 @@ TEST_F(CompositedLayerMappingTest,
// direction to compensate. To make this a little clearer, for the first // direction to compensate. To make this a little clearer, for the first
// example here the layer positions are calculated as: // example here the layer positions are calculated as:
// //
// m_graphicsLayer x = left_pos - shadow_spread + shadow_x_offset // graphics_layer_ x = left_pos - shadow_spread + shadow_x_offset
// = 50 - 10 - 10 // = 50 - 10 - 10
// = 30 // = 30
// //
// m_graphicsLayer y = top_pos - shadow_spread + shadow_y_offset // graphics_layer_ y = top_pos - shadow_spread + shadow_y_offset
// = 50 - 10 + 0 // = 50 - 10 + 0
// = 40 // = 40
// //
// contents x = 50 - m_graphicsLayer x = 50 - 30 = 20 // contents x = 50 - graphics_layer_ x = 50 - 30 = 20
// contents y = 50 - m_graphicsLayer y = 50 - 40 = 10 // contents y = 50 - graphics_layer_ y = 50 - 40 = 10
// //
// The reason that perspective matters is that it affects which 'contents' // The reason that perspective matters is that it affects which 'contents'
// layer is offset; m_childTransformLayer when using perspective, or // layer is offset; child_transform_layer_ when using perspective, or
// m_scrollingLayer when there is no perspective. // scrolling_layer_ when there is no perspective.
SetBodyInnerHTML( SetBodyInnerHTML(R"HTML(
"<div id='scroller' style='position: absolute; top: 50px; left: 50px; " <div id='scroller' style='position: absolute; top: 50px; left: 50px;
"width: 400px; height: 245px; overflow: auto; will-change: transform; " width: 400px; height: 245px; overflow: auto; will-change: transform;
"box-shadow: -10px 0 0 10px; perspective: 1px;'>" box-shadow: -10px 0 0 10px; perspective: 1px;'>
" <div style='position: absolute; top: 50px; bottom: 0; width: 200px; " <div style='position: absolute; top: 50px; bottom: 0; width: 200px;
"height: 200px;'></div>" height: 200px;'></div>
"</div>" </div>
<div id='scroller2' style='position: absolute; top: 400px; left: 50px;
"<div id='scroller2' style='position: absolute; top: 400px; left: 50px; " width: 400px; height: 245px; overflow: auto; will-change: transform;
"width: 400px; height: 245px; overflow: auto; will-change: transform; " box-shadow: -10px 0 0 10px;'>
"box-shadow: -10px 0 0 10px;'>" <div style='position: absolute; top: 50px; bottom: 0; width: 200px;
" <div style='position: absolute; top: 50px; bottom: 0; width: 200px; " height: 200px;'></div>
"height: 200px;'></div>" </div>
"</div>"); )HTML");
CompositedLayerMapping* mapping = CompositedLayerMapping* mapping =
ToLayoutBlock(GetLayoutObjectByElementId("scroller")) ToLayoutBlock(GetLayoutObjectByElementId("scroller"))
......
third_party/blink/renderer/core/paint/embedded_content_painter.cc
View file @ ec37d4cc
...@@ -30,10 +30,6 @@ void EmbeddedContentPainter::PaintReplaced(const PaintInfo& paint_info, ...@@ -30,10 +30,6 @@ void EmbeddedContentPainter::PaintReplaced(const PaintInfo& paint_info,
paint_offset + paint_offset +
layout_embedded_content_.ReplacedContentRect().Location())); layout_embedded_content_.ReplacedContentRect().Location()));
// Views don't support painting with a paint offset, but instead
// offset themselves using the frame rect location. To paint Views at
// our desired location, we need to apply paint offset as a transform, with
// the frame rect neutralized.
IntSize view_paint_offset = IntSize view_paint_offset =
paint_location - embedded_content_view->FrameRect().Location(); paint_location - embedded_content_view->FrameRect().Location();
CullRect adjusted_cull_rect(paint_info.GetCullRect(), -view_paint_offset); CullRect adjusted_cull_rect(paint_info.GetCullRect(), -view_paint_offset);
......
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