Chromium Code Reviews
Help | Chromium Project | Sign in
(84)

Issue 2699653002: Avoid over-eager clipping of child layers in multicol. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 week, 2 days ago by mstensho
Modified:
1 week ago
Reviewers:
eae
CC:
chromium-reviews, szager+layoutwatch_chromium.org, zoltan1, blink-reviews-layout_chromium.org, pdr+renderingwatchlist_chromium.org, eae+blinkwatch, leviw+renderwatch, jchaffraix+rendering, blink-reviews, mstensho
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Avoid over-eager clipping of child layers in multicol. Self-painting layers (caused by e.g. position:relative) don't contribute to visual overflow in their parent layout object; see LayoutBox::addOverflowFromChild(). We cannot use the visual overflow rectangle set on the flow thread when calculating the bounding box of a fragment established by a child layer. Therefore, only clip in the flow thread's block direction in overflowRectForFlowThreadPortion(), and leave the inline axis alone. I simplified the implementation, since it's now way easier to start with an infinite rectangle, and just limit the dimensions that need it afterwards. Also got rid of an old check for hasOverflowClip(), which must have been residue from the CSS regions implementation. This also happened to fix some inaccuracies mostly invisible to the naked eye, when it comes to transform:scale()d text with glyphs that have negative left bearing or overflow the line box vertically. It looks like we measure and lay out with the CSS computed font, and then switch to a scaled font when painting, so that it looks crisp and nice. This may result in tiny inaccuracies in the bounding box of the text, and fast/borders/border-antialiasing.html exhibited this, and had to be rebaselined. Added fast/multicol/scale-transform-text.html to better demonstrate what we're fixing. paint/invalidation/multicol-with-relpos.html also had to be rebaselined, since it turns out that it has never painted its stuff correctly until now. BUG=571978 Review-Url: https://codereview.chromium.org/2699653002 Cr-Commit-Position: refs/heads/master@{#451376} Committed: https://chromium.googlesource.com/chromium/src/+/e500cbd3ba36bf3216087481790f5aece0e778a5

Patch Set 1 #

Patch Set 2 : Rebaseline tests. #

Patch Set 3 : Add test that demonstrates what is now fixed in the rebaselined border-antialiasing.html #

Unified diffs Side-by-side diffs Delta from patch set Stats (+44 lines, -29 lines) Patch
A third_party/WebKit/LayoutTests/fast/multicol/overflowing-relpos.html View 1 chunk +5 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/fast/multicol/overflowing-relpos-expected.html View 1 chunk +3 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/fast/multicol/scale-transform-text.html View 1 2 1 chunk +12 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/fast/multicol/scale-transform-text-expected.html View 1 2 1 chunk +12 lines, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/paint/invalidation/multicol-with-relpos-expected.txt View 1 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/platform/linux/fast/borders/border-antialiasing-expected.png View 1 Binary file 0 comments Download
D third_party/WebKit/LayoutTests/platform/mac-mac10.10/fast/borders/border-antialiasing-expected.png View 1 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac-mac10.10/fast/forms/select/menulist-appearance-rtl-expected.png View 1 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac-mac10.10/fast/multicol/multicol-with-child-renderLayer-for-input-expected.png View 1 Binary file 0 comments Download
D third_party/WebKit/LayoutTests/platform/mac-mac10.9/fast/borders/border-antialiasing-expected.png View 1 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac-mac10.9/fast/forms/select/menulist-appearance-rtl-expected.png View 1 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac-mac10.9/fast/multicol/multicol-with-child-renderLayer-for-input-expected.png View 1 Binary file 0 comments Download
D third_party/WebKit/LayoutTests/platform/mac-retina/fast/borders/border-antialiasing-expected.png View 1 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac/fast/borders/border-antialiasing-expected.png View 1 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac/fast/forms/select/menulist-appearance-rtl-expected.png View 1 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac/fast/multicol/multicol-with-child-renderLayer-for-input-expected.png View 1 Binary file 0 comments Download
D third_party/WebKit/LayoutTests/platform/win7/fast/borders/border-antialiasing-expected.png View 1 Binary file 0 comments Download
M third_party/WebKit/Source/core/layout/LayoutMultiColumnSet.cpp View 1 chunk +11 lines, -28 lines 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 17 (13 generated)
mstensho
1 week ago (2017-02-17 12:54:36 UTC) #11
eae
...and a lot easier to understand at that. Thank you. LGTM
1 week ago (2017-02-17 19:24:18 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2699653002/40001
1 week ago (2017-02-17 20:26:38 UTC) #14
commit-bot: I haz the power
1 week ago (2017-02-17 21:00:59 UTC) #17
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/e500cbd3ba36bf3216087481790f...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f8e48bd