Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2690053002:
Calculate positioningArea and not just size, for tiling background. (Closed)
|
Created:
3 years, 10 months ago by Karl Øygard Modified:
3 years, 9 months ago Reviewers:
Stephen Chennney CC:
chromium-reviews, blink-reviews, dshwang, blink-reviews-paint_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionCalculate positioningArea and not just size, for tiling background.
Computing the correct background tiling size when sub-pixel border or
padding is used, requires calculating the full positioning area and not
just size. The actual fix for 686435 is in a followup cl.
BUG=686435
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Review-Url: https://codereview.chromium.org/2690053002
Cr-Commit-Position: refs/heads/master@{#454843}
Committed: https://chromium.googlesource.com/chromium/src/+/f5667c6812249214f1c8ee322b21c6f337b766cc
Patch Set 1 #
Total comments: 2
Patch Set 2 : Removed unnecessary change. #Messages
Total messages: 20 (14 generated)
Description was changed from ========== Calculate positioningArea and not just size, for tiling background. Computing the correct background tiling size when sub-pixel border or padding is used, requires calculating the full positioning area and not just size. The actual fix for 686435 is in a followup cl. BUG=686435 ========== to ========== Calculate positioningArea and not just size, for tiling background. Computing the correct background tiling size when sub-pixel border or padding is used, requires calculating the full positioning area and not just size. The actual fix for 686435 is in a followup cl. BUG=686435 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 ==========
The CQ bit was checked by karlo@opera.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== Calculate positioningArea and not just size, for tiling background. Computing the correct background tiling size when sub-pixel border or padding is used, requires calculating the full positioning area and not just size. The actual fix for 686435 is in a followup cl. BUG=686435 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 ========== to ========== Calculate positioningArea and not just size, for tiling background. Computing the correct background tiling size when sub-pixel border or padding is used, requires calculating the full positioning area and not just size. The actual fix for 686435 is in a followup cl. BUG=686435 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 ==========
karlo@opera.com changed reviewers: + schenney@chromium.org
Dependency for the previously approved cl, ptal
Just one concern about snapping. https://codereview.chromium.org/2690053002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/paint/BackgroundImageGeometry.cpp (right): https://codereview.chromium.org/2690053002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/paint/BackgroundImageGeometry.cpp:539: LayoutSize positioningAreaSize(positioningArea.pixelSnappedSize()); This is a subtle change that might change snapping because we're no longer trying to snap the same way that the dest rect will snap (or something like that). Is your change in behavior deliberate? If not, leave it in the old format in this case.
The CQ bit was checked by karlo@opera.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2690053002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/paint/BackgroundImageGeometry.cpp (right): https://codereview.chromium.org/2690053002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/paint/BackgroundImageGeometry.cpp:539: LayoutSize positioningAreaSize(positioningArea.pixelSnappedSize()); On 2017/03/01 14:48:29, Stephen Chennney wrote: > This is a subtle change that might change snapping because we're no longer > trying to snap the same way that the dest rect will snap (or something like > that). Is your change in behavior deliberate? If not, leave it in the old format > in this case. Good catch, this does not belong here. Thanks!
On 2017/03/03 15:00:07, Karl Øygard wrote: > https://codereview.chromium.org/2690053002/diff/1/third_party/WebKit/Source/c... > File third_party/WebKit/Source/core/paint/BackgroundImageGeometry.cpp (right): > > https://codereview.chromium.org/2690053002/diff/1/third_party/WebKit/Source/c... > third_party/WebKit/Source/core/paint/BackgroundImageGeometry.cpp:539: LayoutSize > positioningAreaSize(positioningArea.pixelSnappedSize()); > On 2017/03/01 14:48:29, Stephen Chennney wrote: > > This is a subtle change that might change snapping because we're no longer > > trying to snap the same way that the dest rect will snap (or something like > > that). Is your change in behavior deliberate? If not, leave it in the old > format > > in this case. > > Good catch, this does not belong here. Thanks! All good now. lgtm.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by karlo@opera.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1488791628992290,
"parent_rev": "88c14863d3b6b22cfcf95e8847130b2c4f00bf6f", "commit_rev":
"f5667c6812249214f1c8ee322b21c6f337b766cc"}
Message was sent while issue was closed.
Description was changed from ========== Calculate positioningArea and not just size, for tiling background. Computing the correct background tiling size when sub-pixel border or padding is used, requires calculating the full positioning area and not just size. The actual fix for 686435 is in a followup cl. BUG=686435 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 ========== to ========== Calculate positioningArea and not just size, for tiling background. Computing the correct background tiling size when sub-pixel border or padding is used, requires calculating the full positioning area and not just size. The actual fix for 686435 is in a followup cl. BUG=686435 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Review-Url: https://codereview.chromium.org/2690053002 Cr-Commit-Position: refs/heads/master@{#454843} Committed: https://chromium.googlesource.com/chromium/src/+/f5667c6812249214f1c8ee322b21... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/f5667c6812249214f1c8ee322b21... |
