Simplify the RenderOverflow class and the uses of it by storing the overflow values in rects and using the rects instead of the individual values where it makes sense.
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>