Summary: | Continuously repainting large parts of Huffington Post. | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | zalan <zalan> | ||||||
Component: | Layout and Rendering | Assignee: | zalan <zalan> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | barraclough, commit-queue, esprehn+autocc, glenn, hyatt, kling, koivisto, kondapallykalyan, simon.fraser | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
zalan
2014-12-09 19:14:25 PST
Created attachment 242986 [details]
Patch
Comment on attachment 242986 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=242986&action=review > LayoutTests/platform/mac/fast/line-grid/line-align-right-edges-expected.txt:25 > - RenderText {#text} at (170,3) size 120x24 > - text run at (170,3) width 120: "X X X" > + RenderText {#text} at (161,3) size 120x24 > + text run at (161,3) width 120: "X X X" Dave, this is the line-grid issue that I mentioned. Apparently the relative value of the left position for a line inside a grid depends on the position of the parent. Normally (simple case) if a box is at (10px,10px) relative to its parent, when the parent moves the child box still has the (10px, 10px) relative position (layout time). However, in RenderBlock::adjustLogicalLeftOffsetForLine() we use the current layoutOffset when figuring out the left value for a line (grid line case only). It produces different output now that we "skip" a layout for that line (renders the same when loading it to Safari). Do you know what we are supposed to do here? Comment on attachment 242986 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=242986&action=review > Source/WebCore/rendering/RenderBlockFlow.cpp:3497 > - ASSERT(!m_lineBoxes.firstLineBox()); > + bool needsLayout = selfNeedsLayout() || relayoutChildren || !m_simpleLineLayout; > + if (needsLayout) { > + deleteLineBoxesBeforeSimpleLineLayout(); > + ASSERT(!m_lineBoxes.firstLineBox()); > > - m_simpleLineLayout = SimpleLineLayout::create(*this); > + m_simpleLineLayout = SimpleLineLayout::create(*this); > + } else { > + // Any property change that triggers line layout mode switching should also dirty the box. > + ASSERT(!m_lineBoxes.firstLineBox()); > + } You should just have one assert below the if() covering all cases. Created attachment 243067 [details]
Patch
Committed r177128: <http://trac.webkit.org/changeset/177128> |