Change RenderReplaced and intrinsicSize to LayoutUnit to avoid rounding problems when zooming/scaling.
Created attachment 168286 [details] Patch
Comment on attachment 168286 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=168286&action=review > Source/WebCore/loader/cache/CachedImage.h:31 > +#include "SVGImageCache.h" > + I'm guessing this isn't necessary. > Source/WebCore/rendering/InlineFlowBox.cpp:631 > + LayoutUnit baselinePosition = curr->baselinePosition(baselineType); > + if (curr->renderer()->isReplaced()) > + baselinePosition = baselinePosition.floor(); It seems like the changes here and in RenderBox could be landed, along with a lot of rebaselines, first. This could help shrink the size of this patch to a more reasonable size. > Source/WebCore/rendering/RenderBox.cpp:3321 > - computedValues.m_position = logicalLeftPos.round(); > + computedValues.m_position = logicalLeftPos; I'd love to see the rationale for this in the changelog.
Good idea, I've broken out the InlineFlowBox and RenderBox changes into a separate change, see bug 99108.
Created attachment 168454 [details] Patch
Comment on attachment 168454 [details] Patch Attachment 168454 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14266407 New failing tests: fast/reflections/reflection-with-zoom.html
Created attachment 168740 [details] Patch
Attachment 168740 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1 LayoutTests/platform/chromium-linux/compositing/overflow/scroll-ancestor-update-expected.png:0: Have to enable auto props in the subversion config file (/Users/eseidel/.subversion/config "enable-auto-props = yes"). Have to set the svn:mime-type in the subversion config file (/Users/eseidel/.subversion/config "*.png = svn:mime-type=image/png"). [image/png] [5] Total errors found: 1 in 36 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 168740 [details] Patch Attachment 168740 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14389009 New failing tests: svg/zoom/page/zoom-svg-through-object-with-percentage-size.xhtml svg/zoom/page/zoom-svg-through-object-with-absolute-size-2.xhtml fast/reflections/reflection-with-zoom.html
Comment on attachment 168740 [details] Patch Attachment 168740 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14390035 New failing tests: svg/zoom/page/zoom-svg-through-object-with-percentage-size.xhtml svg/zoom/page/zoom-svg-through-object-with-absolute-size-2.xhtml fast/reflections/reflection-with-zoom.html
Created attachment 171661 [details] Patch
Any chance this fixes bug 67712?
(In reply to comment #11) > Any chance this fixes bug 67712? Sadly it does not. To fix that bug we'll have to change our image scaling to take the pixel snapped (as opposed to rounded) size into account.
Ping?
Comment on attachment 171661 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=171661&action=review Overall this looks good, except you use LayoutUnits/Rects/Sizes and FractionalLayoutUnits/Rects/Sizes inconsistently through this patch. Let's keep the "Fractional" part out for consistency and give this another try. > Source/WebCore/html/ImageDocument.cpp:158 > + // At a zoom level of 1 the image is guaranteed to have an integer size. This sounds like an assert to me :) > Source/WebCore/loader/cache/CachedImage.cpp:268 > + ASSERT(multiplier != 1.0f || (imageSize.width().fraction() == 0.0f && imageSize.height().fraction() == 0.0f)); Actually, looks like you've got it covered! > Source/WebCore/platform/graphics/FractionalLayoutSize.h:75 > - void scale(float scale) > + void scale(float widthScale, float heightScale) Why not just add this two-argument version? > Source/WebCore/rendering/RenderReplaced.h:49 > - virtual int minimumReplacedHeight() const { return 0; } > + virtual LayoutUnit minimumReplacedHeight() const { return 0; } LayoutUnit()? > Source/WebCore/rendering/RenderVideo.cpp:84 > + float scaleFactor = style()->effectiveZoom(); > + size.scale(scaleFactor, scaleFactor); This would be easier with the single-argument version, no? > Source/WebCore/rendering/RenderVideo.cpp:98 > +FractionalLayoutSize RenderVideo::calculateIntrinsicSize() Why FrationalLayoutSize instead of LayoutSize? > Source/WebCore/rendering/style/StyleGeneratedImage.cpp:70 > - IntSize size = imageSize(renderer, 1); > + IntSize size = flooredIntSize(imageSize(renderer, 1)); I think another comment about the flooring is warranted.
Created attachment 171832 [details] Patch
(In reply to comment #14) Made the changes you suggested, PTAL.
Comment on attachment 171832 [details] Patch Perfect. Thanks!
Comment on attachment 171832 [details] Patch Clearing flags on attachment: 171832 Committed r133172: <http://trac.webkit.org/changeset/133172>
All reviewed patches have been landed. Closing bug.
*** Bug 91007 has been marked as a duplicate of this bug. ***