Replaced elements have to flow in the new sub-pixel Render Tree, but since the rendering of these often takes place outside of WebCore (or in cases such as foreign objects, in WebCore after passing through platform code), care must be taken to determine the final rendered size and location before render time. This patch brings these classes up to the latest and greatest in the subpixellayout branch.
Created attachment 131490 [details] Patch
Comment on attachment 131490 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=131490&action=review > Source/WebCore/rendering/RenderVideo.cpp:169 > + IntRect contentRect = pixelSnappedIntRect(contentBoxRect()); Sigh. I dislike all these free functions. :( > Source/WebCore/rendering/RenderWidget.cpp:147 > +static inline IntRect roundedIntRect(const LayoutRect& rect) > +{ > + return IntRect(roundedIntPoint(rect.location()), roundedIntSize(rect.size())); > +} why can't this be a method on LayoutRect (and IntRect for hte time being?) > Source/WebCore/rendering/RenderWidget.cpp:165 > + m_widget->setFrameRect(IntRect(roundedIntRect(frame))); IntRect() isn't needed here, is it? Also again, why not a method on the rect instead of a free function? > Source/WebCore/rendering/RenderWidget.cpp:181 > + LayoutRect absoluteContentBox = LayoutRect(localToAbsoluteQuad(FloatQuad(contentBox)).boundingBox()); The constructor on the right side isnt' neded. If it is, you can just remove the = () and construct directly in place.
Comment on attachment 131490 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=131490&action=review >> Source/WebCore/rendering/RenderVideo.cpp:169 >> + IntRect contentRect = pixelSnappedIntRect(contentBoxRect()); > > Sigh. I dislike all these free functions. :( So you'd rather have a LayoutRect::pixelSnap() function? It'd definitely be shorter to type :) Since this would be a pretty large refactor at this point, I'd propose it for a separate patch if you feel strongly. >> Source/WebCore/rendering/RenderWidget.cpp:147 >> +} > > why can't this be a method on LayoutRect (and IntRect for hte time being?) I put it here to discourage usage since this is seldom what you want. I'm happy to move it. >> Source/WebCore/rendering/RenderWidget.cpp:165 >> + m_widget->setFrameRect(IntRect(roundedIntRect(frame))); > > IntRect() isn't needed here, is it? Also again, why not a method on the rect instead of a free function? The IntRect definitely isn't needed. Good catch.
Comment on attachment 131490 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=131490&action=review >>> Source/WebCore/rendering/RenderWidget.cpp:147 >>> +} >> >> why can't this be a method on LayoutRect (and IntRect for hte time being?) > > I put it here to discourage usage since this is seldom what you want. I'm happy to move it. Ah, OK, that part wasn't immediately clear to me. Might want to comment to that effect. :)
Created attachment 132788 [details] Patch
Comment on attachment 132788 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=132788&action=review ok. > Source/WebCore/rendering/RenderImage.cpp:530 > + containerSize.setWidth(roundToInt(containerSize.width() * style()->effectiveZoom())); > + containerSize.setHeight(roundToInt(containerSize.height() * style()->effectiveZoom())); Don't we have a method on size to do thos multiply already?
(In reply to comment #6) > (From update of attachment 132788 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=132788&action=review > > ok. > > > Source/WebCore/rendering/RenderImage.cpp:530 > > + containerSize.setWidth(roundToInt(containerSize.width() * style()->effectiveZoom())); > > + containerSize.setHeight(roundToInt(containerSize.height() * style()->effectiveZoom())); > > Don't we have a method on size to do thos multiply already? We have a scale method (on IntSize and FractionalLayoutSize) but they floor rather than round. If flooring is acceptable here containerSize.scale(style()->effectiveZoom()) should do the trick.
(In reply to comment #7) > (In reply to comment #6) > > (From update of attachment 132788 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=132788&action=review > > > > ok. > > > > > Source/WebCore/rendering/RenderImage.cpp:530 > > > + containerSize.setWidth(roundToInt(containerSize.width() * style()->effectiveZoom())); > > > + containerSize.setHeight(roundToInt(containerSize.height() * style()->effectiveZoom())); > > > > Don't we have a method on size to do thos multiply already? > > We have a scale method (on IntSize and FractionalLayoutSize) but they floor rather than round. > > If flooring is acceptable here containerSize.scale(style()->effectiveZoom()) should do the trick. Perhaps FractionalLayoutSize::scale shouldn't floor? A thought...
Comment on attachment 132788 [details] Patch Clearing flags on attachment: 132788 Committed r111515: <http://trac.webkit.org/changeset/111515>
All reviewed patches have been landed. Closing bug.