Bug 108266

Summary: TransformState::move should not round offset to int
Product: WebKit Reporter: Emil A Eklund <eae>
Component: PlatformAssignee: Emil A Eklund <eae>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, dglazkov, eric, leviw, ojan.autocc, rniwa, simon.fraser, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
simon.fraser: review+, webkit.review.bot: commit-queue-
Patch for landing none

Description Emil A Eklund 2013-01-29 17:13:53 PST
Currently TransformState::move rounds the offset to the nearest integer values, this results in operations using TransformState to compute a position to misreport the location, specifically Element:getBoundingClientRect and repaint rects. Sizes are handled correctly and do not have the same problem.
Comment 1 Emil A Eklund 2013-01-29 17:16:58 PST
Created attachment 185353 [details]
Patch
Comment 2 Simon Fraser (smfr) 2013-01-29 17:20:39 PST
Comment on attachment 185353 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=185353&action=review

> Source/WebCore/platform/graphics/transforms/TransformState.cpp:69
>  void TransformState::translateMappedCoordinates(const IntSize& offset)

Do we still need these Int versions?
Comment 3 Emil A Eklund 2013-01-29 17:24:27 PST
(In reply to comment #2)
> (From update of attachment 185353 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=185353&action=review
> 
> > Source/WebCore/platform/graphics/transforms/TransformState.cpp:69
> >  void TransformState::translateMappedCoordinates(const IntSize& offset)
> 
> Do we still need these Int versions?

Not really. Was trying to avoid upcasting from int to LayoutUnit for callers that use an IntSize.
Comment 4 Build Bot 2013-01-29 19:05:54 PST
Comment on attachment 185353 [details]
Patch

Attachment 185353 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/16115498

New failing tests:
accessibility/image-link.html
accessibility/table-sections.html
accessibility/table-cell-spans.html
accessibility/table-detection.html
accessibility/table-cells.html
fast/dom/Window/webkitConvertPoint.html
accessibility/internal-link-anchors2.html
accessibility/table-attributes.html
Comment 5 Build Bot 2013-01-29 23:09:42 PST
Comment on attachment 185353 [details]
Patch

Attachment 185353 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://queues.webkit.org/results/16202518

New failing tests:
accessibility/image-link.html
accessibility/table-sections.html
accessibility/table-cell-spans.html
accessibility/table-detection.html
accessibility/table-cells.html
fast/dom/Window/webkitConvertPoint.html
accessibility/internal-link-anchors2.html
accessibility/table-attributes.html
Comment 6 Emil A Eklund 2013-01-30 15:51:58 PST
Comment on attachment 185353 [details]
Patch

Turns out this breaks repainting of iframes positioned on sunpixel boundaries in some cases. Marking as obsolete until for now.
Comment 7 Emil A Eklund 2013-02-11 13:42:27 PST
Created attachment 187657 [details]
Patch
Comment 8 WebKit Review Bot 2013-02-11 14:48:22 PST
Comment on attachment 187657 [details]
Patch

Attachment 187657 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/16499347

New failing tests:
platform/chromium/fast/forms/suggestion-picker/date-suggestion-picker-appearance-with-scroll-bar.html
platform/chromium/fast/forms/suggestion-picker/month-suggestion-picker-appearance-with-scroll-bar.html
platform/chromium/fast/forms/suggestion-picker/week-suggestion-picker-appearance-with-scroll-bar.html
Comment 9 Emil A Eklund 2013-02-11 15:37:02 PST
(In reply to comment #8)
> (From update of attachment 187657 [details])
> Attachment 187657 [details] did not pass chromium-ews (chromium-xvfb):
> Output: http://queues.webkit.org/results/16499347
> 
> New failing tests:
> platform/chromium/fast/forms/suggestion-picker/date-suggestion-picker-appearance-with-scroll-bar.html
> platform/chromium/fast/forms/suggestion-picker/month-suggestion-picker-appearance-with-scroll-bar.html
> platform/chromium/fast/forms/suggestion-picker/week-suggestion-picker-appearance-with-scroll-bar.html

Odd, all the platform/chromium/fast/forms/suggestion-picker tests pass for me on chromium linux.
Comment 10 Emil A Eklund 2013-02-11 17:44:15 PST
The platform/chromium/fast/forms/suggestion-picker tests fail to due to a one pixel difference in scrolling when calling Element.focus() to scroll-to-reveal. In trying to fix the suggestionPicker.js to avoid this I found a bunch of other scrolling inconsistencies so I'm going to rewrite how we do scrolling in suggestion pickers. Tracked by bug 109528.
Comment 11 Emil A Eklund 2013-02-11 18:36:57 PST
Created attachment 187747 [details]
Patch for landing
Comment 12 WebKit Review Bot 2013-02-12 09:42:07 PST
Comment on attachment 187747 [details]
Patch for landing

Clearing flags on attachment: 187747

Committed r142638: <http://trac.webkit.org/changeset/142638>
Comment 13 WebKit Review Bot 2013-02-12 09:42:11 PST
All reviewed patches have been landed.  Closing bug.