Bug 108266 - TransformState::move should not round offset to int
Summary: TransformState::move should not round offset to int
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Emil A Eklund
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-01-29 17:13 PST by Emil A Eklund
Modified: 2013-02-12 09:42 PST (History)
8 users (show)

See Also:


Attachments
Patch (24.32 KB, patch)
2013-01-29 17:16 PST, Emil A Eklund
no flags Details | Formatted Diff | Diff
Patch (13.55 KB, patch)
2013-02-11 13:42 PST, Emil A Eklund
simon.fraser: review+
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Patch for landing (14.72 KB, patch)
2013-02-11 18:36 PST, Emil A Eklund
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.