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

Issue 11276060: Pass accurate contentsScale to LayerImpl. (Closed)

Created:
8 years, 1 month ago by Xianzhu
Modified:
8 years, 1 month ago
CC:
chromium-reviews, cc-bugs_chromium.org, klobag.chromium, aelias_OOO_until_Jul13, trchen
Visibility:
Public.

Description

Pass accurate contentsScale to LayerImpl. This is the first part of fixing fuzzy content issue in composited layers in scaled pages. There are two reasons of fuzzy content: 1) Inaccurate scale: The scale in transformation is calculated with contentBounds.width / bounds.width contentBounds.height / bounds.height in layer_tree_host_common.cc and other places. However, contentBounds is a IntSize which is calculated from bounds and contentsScale in layer.cc: IntRect(lroundf(bounds.width * contentsScale), lroundf(bounds.height * contentsScale)) This causes the scale like 0.993 or 1.007 in the drawTransformation etc. where identity transformation is expected. To resolve the issue, instead of calculating scale using contentBounds, pass the accurate contentsScale from Layer to LayerImpl. 2) Pixel on surfaces are not aligned. Will describe this in the CL for the second part. (See https://bugs.webkit.org/show_bug.cgi?id=84187 for more details). Change-Id: I8f59f0460e1b212223e2c8c551b4127d8239e5cc BUG=bugs.webkit.org/show_bug.cgi?id=84187 TEST=ContentsScalingLayerTest.checkContentBounds, LayerTreeHostCommonTest.verifyLayerTransformsInHighDPIAccurateScaleZeroPosition, LayerTreeHostCommonTest.verifyRenderSurfaceTransformsInHighDPIAccurateScaleZeroPosition Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=165269

Patch Set 1 #

Total comments: 15

Patch Set 2 : update #

Patch Set 3 : update #

Patch Set 4 : Fix clang errors #

Patch Set 5 : Fix layout test crash in scrollbarlayer #

Patch Set 6 : Fix build break on win #

Patch Set 7 : Rebased #

Unified diffs Side-by-side diffs Delta from patch set Stats (+474 lines, -189 lines) Patch
M cc/cc.gyp View 1 2 3 4 1 chunk +2 lines, -0 lines 0 comments Download
M cc/cc_tests.gyp View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
A cc/contents_scaling_layer.h View 1 2 3 1 chunk +31 lines, -0 lines 0 comments Download
A cc/contents_scaling_layer.cc View 1 2 3 1 chunk +38 lines, -0 lines 0 comments Download
A cc/contents_scaling_layer_unittest.cc View 1 2 3 4 5 1 chunk +76 lines, -0 lines 0 comments Download
M cc/damage_tracker.cc View 1 1 chunk +2 lines, -6 lines 0 comments Download
M cc/image_layer.h View 1 chunk +2 lines, -1 line 0 comments Download
M cc/image_layer.cc View 1 2 3 4 5 6 1 chunk +11 lines, -3 lines 0 comments Download
M cc/layer.h View 1 2 3 5 chunks +6 lines, -6 lines 0 comments Download
M cc/layer.cc View 1 2 3 5 chunks +15 lines, -17 lines 0 comments Download
M cc/layer_impl.h View 1 3 chunks +15 lines, -3 lines 0 comments Download
M cc/layer_impl.cc View 1 3 chunks +20 lines, -6 lines 0 comments Download
M cc/layer_impl_unittest.cc View 1 2 chunks +2 lines, -0 lines 0 comments Download
M cc/layer_tree_host_common.cc View 1 4 4 chunks +8 lines, -8 lines 0 comments Download
M cc/layer_tree_host_common_unittest.cc View 1 2 3 4 5 6 10 chunks +177 lines, -26 lines 0 comments Download
M cc/layer_tree_host_impl.cc View 1 2 3 4 1 chunk +2 lines, -6 lines 0 comments Download
M cc/layer_tree_host_impl_unittest.cc View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M cc/layer_tree_host_unittest.cc View 1 2 3 4 5 6 1 chunk +3 lines, -1 line 0 comments Download
M cc/layer_unittest.cc View 1 2 3 4 5 6 1 chunk +0 lines, -50 lines 0 comments Download
M cc/render_surface_impl.cc View 1 2 3 4 1 chunk +2 lines, -2 lines 0 comments Download
M cc/scrollbar_layer.h View 1 2 3 4 3 chunks +4 lines, -5 lines 0 comments Download
M cc/scrollbar_layer.cc View 1 2 3 4 5 6 7 chunks +14 lines, -19 lines 0 comments Download
M cc/scrollbar_layer_impl.h View 1 2 3 4 1 chunk +2 lines, -0 lines 0 comments Download
M cc/scrollbar_layer_impl.cc View 1 2 3 4 3 chunks +11 lines, -2 lines 0 comments Download
M cc/test/tiled_layer_test_common.h View 1 2 3 4 5 6 1 chunk +3 lines, -0 lines 0 comments Download
M cc/test/tiled_layer_test_common.cc View 1 2 3 4 5 6 1 chunk +15 lines, -0 lines 0 comments Download
M cc/tiled_layer.h View 2 chunks +2 lines, -5 lines 0 comments Download
M cc/tiled_layer.cc View 1 2 3 4 5 6 6 chunks +7 lines, -22 lines 0 comments Download
M cc/tiled_layer_unittest.cc View 1 2 3 4 5 6 1 chunk +2 lines, -1 line 0 comments Download

Messages

Total messages: 25 (0 generated)
wangxianzhu
8 years, 1 month ago (2012-10-27 05:36:12 UTC) #1
wangxianzhu
This is the first part (of 2) of fixing fuzzy content issue in composited layers ...
8 years, 1 month ago (2012-10-27 05:39:47 UTC) #2
danakj
On 2012/10/27 05:39:47, Xianzhu Wang wrote: > This is the first part (of 2) of ...
8 years, 1 month ago (2012-10-27 15:29:54 UTC) #3
danakj
s/between counts/between bounds()/
8 years, 1 month ago (2012-10-27 15:30:24 UTC) #4
wangxianzhu
On 2012/10/27 15:29:54, danakj wrote: > Need to think about this, but in trying to ...
8 years, 1 month ago (2012-10-29 16:28:18 UTC) #5
danakj
On Mon, Oct 29, 2012 at 12:28 PM, <wangxianzhu@google.com> wrote: > On 2012/10/27 15:29:54, danakj ...
8 years, 1 month ago (2012-10-29 17:50:17 UTC) #6
enne (OOO)
I like this patch quite a bit. http://codereview.chromium.org/11276060/diff/1/cc/layer_impl.cc File cc/layer_impl.cc (right): http://codereview.chromium.org/11276060/diff/1/cc/layer_impl.cc#newcode417 cc/layer_impl.cc:417: IntSize LayerImpl::contentBounds() ...
8 years, 1 month ago (2012-10-29 19:19:41 UTC) #7
danakj
http://codereview.chromium.org/11276060/diff/1/cc/damage_tracker.cc File cc/damage_tracker.cc (right): http://codereview.chromium.org/11276060/diff/1/cc/damage_tracker.cc#newcode271 cc/damage_tracker.cc:271: updateContentRect.scale(layer->contentsScaleX(), layer->contentsScaleY()); Can we use Layer::layerRectToContentRect() here? The current ...
8 years, 1 month ago (2012-10-29 20:06:51 UTC) #8
wangxianzhu
https://codereview.chromium.org/11276060/diff/1/cc/damage_tracker.cc File cc/damage_tracker.cc (right): https://codereview.chromium.org/11276060/diff/1/cc/damage_tracker.cc#newcode271 cc/damage_tracker.cc:271: updateContentRect.scale(layer->contentsScaleX(), layer->contentsScaleY()); On 2012/10/29 20:06:51, danakj wrote: > Can ...
8 years, 1 month ago (2012-10-30 02:14:21 UTC) #9
danakj
https://codereview.chromium.org/11276060/diff/1/cc/damage_tracker.cc File cc/damage_tracker.cc (right): https://codereview.chromium.org/11276060/diff/1/cc/damage_tracker.cc#newcode271 cc/damage_tracker.cc:271: updateContentRect.scale(layer->contentsScaleX(), layer->contentsScaleY()); On 2012/10/30 02:14:21, Xianzhu Wang wrote: > ...
8 years, 1 month ago (2012-10-30 15:56:19 UTC) #10
enne (OOO)
Thanks for all the changes. lgtm, but please take a look at the linux_layout_rel layout ...
8 years, 1 month ago (2012-10-30 18:00:22 UTC) #11
Xianzhu
The change causes many layout test crashes. In ScrollbarLayer::update() contentRect = layerRectToContentRect(FloatRect(scrollbarOrigin, bounds()) returns an ...
8 years, 1 month ago (2012-10-30 22:56:21 UTC) #12
danakj
On 2012/10/30 22:56:21, Xianzhu wrote: > The change causes many layout test crashes. In ScrollbarLayer::update() ...
8 years, 1 month ago (2012-10-30 23:03:39 UTC) #13
danakj
On 2012/10/30 23:03:39, danakj wrote: > On 2012/10/30 22:56:21, Xianzhu wrote: > > The change ...
8 years, 1 month ago (2012-10-30 23:05:43 UTC) #14
enne (OOO)
Scrollbar painting in WebKit is strange; that's just how it works. I think that convention ...
8 years, 1 month ago (2012-10-30 23:19:03 UTC) #15
Xianzhu
On 2012/10/30 23:19:03, enne wrote: > Maybe just don't use layerRectToContentRect there and leave a ...
8 years, 1 month ago (2012-10-31 17:43:00 UTC) #16
Xianzhu
No test crashed. Other failures seem not related to this change.
8 years, 1 month ago (2012-10-31 18:42:47 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/wangxianzhu@chromium.org/11276060/27001
8 years, 1 month ago (2012-10-31 18:43:16 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/wangxianzhu@chromium.org/11276060/32003
8 years, 1 month ago (2012-10-31 19:06:54 UTC) #19
enne (OOO)
lgtm. Thanks for fixing those failures. :)
8 years, 1 month ago (2012-10-31 20:53:14 UTC) #20
commit-bot: I haz the power
Retried try job too often for step(s) nacl_integration
8 years, 1 month ago (2012-10-31 21:11:19 UTC) #21
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/wangxianzhu@chromium.org/11276060/32003
8 years, 1 month ago (2012-10-31 21:24:45 UTC) #22
commit-bot: I haz the power
Failed to apply patch for cc/scrollbar_layer.cc: While running patch -p1 --forward --force --no-backup-if-mismatch; patching file ...
8 years, 1 month ago (2012-10-31 21:24:55 UTC) #23
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/wangxianzhu@chromium.org/11276060/32005
8 years, 1 month ago (2012-10-31 21:36:41 UTC) #24
commit-bot: I haz the power
8 years, 1 month ago (2012-11-01 00:12:49 UTC) #25
Change committed as 165269

Powered by Google App Engine
This is Rietveld 408576698