Bug 57276

Summary: Add an optimization to make line height and box placement calculations faster
Product: WebKit Reporter: Dave Hyatt <hyatt>
Component: Layout and RenderingAssignee: Dave Hyatt <hyatt>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, mitz, psolanki, simon.fraser
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
Patch
none
Patch darin: review+

Dave Hyatt
Reported 2011-03-28 15:19:57 PDT
Add an optimization to make line height and box placement calculations faster. Basically try to avoid spending as much time in computeLogicalBoxHeights and placeBoxesInBlockDirection.
Attachments
Patch (17.22 KB, patch)
2011-03-28 15:20 PDT, Dave Hyatt
no flags
Patch (20.72 KB, patch)
2011-03-29 10:01 PDT, Dave Hyatt
darin: review+
Dave Hyatt
Comment 1 2011-03-28 15:20:17 PDT
Dave Hyatt
Comment 2 2011-03-29 10:01:46 PDT
Darin Adler
Comment 3 2011-03-29 10:37:34 PDT
Comment on attachment 87353 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=87353&action=review > Source/WebCore/rendering/InlineFlowBox.cpp:105 > + InlineFlowBox* childFlowBox = static_cast<InlineFlowBox*>(child); > + if (childFlowBox->hasTextDescendants()) > + m_hasTextDescendants = true; I don’t think the local variable here is helpful. > Source/WebCore/rendering/InlineFlowBox.cpp:110 > + RenderStyle* parentStyle = renderer()->style(m_firstLine); > + RenderStyle* childStyle = child->renderer()->style(m_firstLine); Seems a shame to always compute these, since they are not always used. > Source/WebCore/rendering/InlineFlowBox.cpp:111 > + bool changeResult = false; Not sure what “change result” means in this name. Can you come up with a better name. > Source/WebCore/rendering/InlineFlowBox.cpp:521 > - if (curr->isInlineFlowBox()) > - static_cast<InlineFlowBox*>(curr)->computeLogicalBoxHeights(rootBox, maxPositionTop, maxPositionBottom, maxAscent, maxDescent, > - setMaxAscent, setMaxDescent, strictMode, textBoxDataMap, > - baselineType, verticalPositionCache); > + if (inlineFlowBox) > + inlineFlowBox->computeLogicalBoxHeights(rootBox, maxPositionTop, maxPositionBottom, maxAscent, maxDescent, > + setMaxAscent, setMaxDescent, strictMode, textBoxDataMap, > + baselineType, verticalPositionCache); It’s better to not line up the subsequent lines so you don’t have to move them when making a change like this one. Also, there should be braces in this if statement.
Note You need to log in before you can comment on or make changes to this bug.