In snapToSize we use the fraction of the location rather than the full location when adjusting the width. This results in the sign being lost which in turn affects rounding. Downstream chromium bug: http://code.google.com/p/chromium/issues/detail?id=150802
Created attachment 165002 [details] Patch
Comment on attachment 165002 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=165002&action=review > Source/WebCore/platform/FractionalLayoutUnit.h:810 > + FractionalLayoutUnit fraction = (location - location.floor()) * (location < 0 ? -1 : 1); I this the most elegant way to do this? It seems you're undoing floor's magic, which presumably ignores sign.
(In reply to comment #2) > (From update of attachment 165002 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=165002&action=review > > > Source/WebCore/platform/FractionalLayoutUnit.h:810 > > + FractionalLayoutUnit fraction = (location - location.floor()) * (location < 0 ? -1 : 1); > > I this the most elegant way to do this? It seems you're undoing floor's magic, which presumably ignores sign. I'm sure it is not but it is the best I've managed to come up with so far. If you have any suggestions I'm all ears.
Comment on attachment 165002 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=165002&action=review >>> Source/WebCore/platform/FractionalLayoutUnit.h:810 >>> + FractionalLayoutUnit fraction = (location - location.floor()) * (location < 0 ? -1 : 1); >> >> I this the most elegant way to do this? It seems you're undoing floor's magic, which presumably ignores sign. > > I'm sure it is not but it is the best I've managed to come up with so far. > If you have any suggestions I'm all ears. Isn't the fraction just location.rawValue() % kFixedPointDenominator?
Comment on attachment 165002 [details] Patch Attachment 165002 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13932472 New failing tests: compositing/reflections/reflection-in-composited.html fast/writing-mode/Kusa-Makura-background-canvas.html css3/filters/nested-filter.html media/video-zoom-controls.html fast/multicol/vertical-lr/float-multicol.html
Created attachment 165195 [details] Patch
Comment on attachment 165195 [details] Patch LGTM.
Comment on attachment 165195 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=165195&action=review > Source/WebCore/platform/FractionalLayoutUnit.h:811 > + fraction.setRawValue(location.rawValue() % kFixedPointDenominator); A comment about why we do it this way might help, but this is also OK.
Created attachment 165199 [details] Patch for landing
Thanks Eric. Added a comment explaining why we do it this way.
Comment on attachment 165199 [details] Patch for landing Rejecting attachment 165199 [details] from commit-queue. New failing tests: WebFilterOperationsTest.saveAndRestore media/video-zoom-controls.html Full output: http://queues.webkit.org/results/13956808
Committed r129370: <http://trac.webkit.org/changeset/129370>