Summary: | Simplify RenderOverflow by using Rects | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Emil A Eklund <eae> | ||||
Component: | Layout and Rendering | Assignee: | Emil A Eklund <eae> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | darin, eric, simon.fraser | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Emil A Eklund
2012-05-18 13:40:44 PDT
Created attachment 142824 [details]
Patch
Comment on attachment 142824 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=142824&action=review Wow. total insanity. I suspect there is another round of cleanup to be done here as well with all the "logical" accessors. > Source/WebCore/page/FrameView.cpp:3283 > + IntRect dirtyRect(0, static_cast<int>(floorf(oldTop)), root->layoutOverflowRect().maxX(), static_cast<int>(ceilf(oldBottom - oldTop))); this looks a lot like enclosingIntRect(FloatRect(0, oldTop, maxX, oldBottom - oldTop))? > Source/WebCore/rendering/InlineFlowBox.h:210 > + LayoutUnit logicalLeftLayoutOverflow() const { return m_overflow ? (isHorizontal() ? m_overflow->layoutOverflowRect().x() : m_overflow->layoutOverflowRect().y()) : static_cast<LayoutUnit>(logicalLeft()); } Might make sense to fomat this as multiple lines. > Source/WebCore/rendering/InlineFlowBox.h:215 > - return isHorizontal() ? m_overflow->minYLayoutOverflow() : m_overflow->minXLayoutOverflow(); > + return isHorizontal() ? m_overflow->layoutOverflowRect().y() : m_overflow->layoutOverflowRect().x(); We need to find a cleaner way to do this flipping-on-demand that we do everywhere. just "return logicalOverflowRect().x()"? > Source/WebCore/rendering/RenderBox.cpp:3738 > - m_overflow->resetLayoutOverflow(borderBoxRect()); > + m_overflow->setLayoutOverflow(borderBoxRect()); I'm not sure I follow? Did you rename this? (In reply to comment #2) > (From update of attachment 142824 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=142824&action=review > > Wow. total insanity. I suspect there is another round of cleanup to be done here as well with all the "logical" accessors. Yeah, I plan to tackle that next. > > Source/WebCore/page/FrameView.cpp:3283 > > + IntRect dirtyRect(0, static_cast<int>(floorf(oldTop)), root->layoutOverflowRect().maxX(), static_cast<int>(ceilf(oldBottom - oldTop))); > > this looks a lot like enclosingIntRect(FloatRect(0, oldTop, maxX, oldBottom - oldTop))? It does, doesn't it? I'll see if I can change it to use enclosingIntRect. Thanks! > > > Source/WebCore/rendering/InlineFlowBox.h:210 > > + LayoutUnit logicalLeftLayoutOverflow() const { return m_overflow ? (isHorizontal() ? m_overflow->layoutOverflowRect().x() : m_overflow->layoutOverflowRect().y()) : static_cast<LayoutUnit>(logicalLeft()); } > > Might make sense to fomat this as multiple lines. Good idea. > > Source/WebCore/rendering/InlineFlowBox.h:215 > > - return isHorizontal() ? m_overflow->minYLayoutOverflow() : m_overflow->minXLayoutOverflow(); > > + return isHorizontal() ? m_overflow->layoutOverflowRect().y() : m_overflow->layoutOverflowRect().x(); > > We need to find a cleaner way to do this flipping-on-demand that we do everywhere. just "return logicalOverflowRect().x()"? Yeah, I have a couple of ideas around that. Expect more cleanup patches next week :) > > > Source/WebCore/rendering/RenderBox.cpp:3738 > > - m_overflow->resetLayoutOverflow(borderBoxRect()); > > + m_overflow->setLayoutOverflow(borderBoxRect()); > > I'm not sure I follow? Did you rename this? No, I removed the resetLayoutOverflow method as it did exactly the same thing as the set method. Committed r117697: <http://trac.webkit.org/changeset/117697> |