Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1402)

Issue 2172503002: Fix subpixel accumulation for composited content layers

Created:
4 years, 5 months ago by trchen
Modified:
4 years, 5 months ago
Reviewers:
chrishtr
CC:
blink-reviews, blink-reviews-layout_chromium.org, chromium-reviews, eae+blinkwatch, jchaffraix+rendering, leviw+renderwatch, pdr+renderingwatchlist_chromium.org, szager+layoutwatch_chromium.org, zoltan1
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Fix subpixel accumulation for composited content layers The content box rect was improperly snapped in the local space. We should have snapped in the layout space instead, including subpixel accumulation. BUG=627683

Patch Set 1 #

Patch Set 2 : add test expectation #

Total comments: 5
Unified diffs Side-by-side diffs Delta from patch set Stats (+33 lines, -69 lines) Patch
M third_party/WebKit/LayoutTests/TestExpectations View 1 1 chunk +3 lines, -0 lines 1 comment Download
M third_party/WebKit/Source/core/layout/LayoutTextTrackContainer.h View 1 chunk +0 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/layout/LayoutTextTrackContainer.cpp View 1 chunk +2 lines, -5 lines 1 comment Download
M third_party/WebKit/Source/core/layout/LayoutVideo.h View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/layout/LayoutVideo.cpp View 1 chunk +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.h View 2 chunks +0 lines, -5 lines 0 comments Download
M third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp View 9 chunks +25 lines, -55 lines 3 comments Download

Messages

Total messages: 7 (5 generated)
trchen
4 years, 5 months ago (2016-07-21 00:07:04 UTC) #2
chrishtr
4 years, 5 months ago (2016-07-21 18:02:31 UTC) #7
This CL needs a new test that exercises what's in the bug.

https://codereview.chromium.org/2172503002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/TestExpectations (right):

https://codereview.chromium.org/2172503002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/TestExpectations:1358: crbug.com/627683
virtual/gpu/fast/canvas/canvas-toDataURL-webp.html [ NeedsRebaseline ]
Please link to bot results on this so I can see the change. Or upload the change
on one platform.

https://codereview.chromium.org/2172503002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutTextTrackContainer.cpp (left):

https://codereview.chromium.org/2172503002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutTextTrackContainer.cpp:62: IntSize
videoSize = videoLayoutObject.videoBox().size();
Why the changes here? videoBox() has integer size.

https://codereview.chromium.org/2172503002/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
(right):

https://codereview.chromium.org/2172503002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp:93:
// FrameView are somehow pre-snapped. This is to match behavior of
LayoutPart::updateWidgetGeometry().
PartPainter::paintContents does rounded snapping that looks the same as this.
This is now the third place that this code appears.
Please refactor to have it centralized in a method on LayoutPart.

https://codereview.chromium.org/2172503002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp:354:
innerCompositor->frameViewDidChangeLocation(roundedIntPoint(contentsBox().location()));
Yet another instance of frame rounding to be refactored?

https://codereview.chromium.org/2172503002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp:1988:
return m_owningLayer.subpixelAccumulation() -
LayoutSize(m_graphicsLayer->offsetFromLayoutObject());
why subpixelAccumulation - offsetFromLayoutObject? Why not:

-(offsetFromLayoutObject + subpixelAccumulation)

Powered by Google App Engine
This is Rietveld 408576698