Bug 133180 - Touch events should allow for device-pixel accuracy
Summary: Touch events should allow for device-pixel accuracy
Status: ASSIGNED
Alias: None
Product: WebKit
Classification: Unclassified
Component: UI Events (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Antoine Quint
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-05-22 08:08 PDT by Rick Byers
Modified: 2017-08-19 16:01 PDT (History)
12 users (show)

See Also:


Attachments
Patch (23.82 KB, patch)
2016-06-14 13:25 PDT, Antoine Quint
no flags Details | Formatted Diff | Diff
Patch (24.00 KB, patch)
2016-06-14 13:46 PDT, Antoine Quint
beidson: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Rick Byers 2014-05-22 08:08:24 PDT
Various APIs are attempting to move to floating-point co-ordinates (eg. see bug 132895).  Perhaps TouchEvent co-ordinates should also be float/double instead of long?  http://trac.webkit.org/browser/trunk/Source/WebCore/dom/Touch.idl

This may be valuable on high-dpi devices.  Eg. if scrolling is happening in physical pixel units, it should be possible to build JS applications that move things around just as smoothly (as is possible in native iOS apps).  Eg. imagine an app that does a custom pull effect when pulling down when the document is at scroll offset 0.  Today when moving slowly this is noticeably jumpier on retina devices than the buttery smooth browser scrolling you get moving in the other direction.

I'm in the process of switching blink to doubles:
https://code.google.com/p/chromium/issues/detail?id=323935

The TouchEvents community group is also discussing this:
http://lists.w3.org/Archives/Public/public-touchevents/2014May/0006.html
Comment 1 Benjamin Poulain 2014-05-23 02:30:57 PDT
I agree, we should try this too.

Do you have a test page where it is jumpy for reference?
Comment 2 Rick Byers 2014-05-23 13:58:50 PDT
Thanks!

It can be pretty subtle in practice (especially if the finger is moving quickly), but here's an easy to visualize example: http://www.rbyers.net/paint.html#points - a drawing program that draws on physical pixel point per touchmove event.  See https://code.google.com/p/chromium/issues/detail?id=323935#c34 for screenshots comparing integer to floating-point co-ordinates on a nexus 5 (which has DSF=3).
Comment 3 Rick Byers 2014-05-23 14:14:59 PDT
The difference is also pretty noticeable when dragging something around slowly. eg: http://jsbin.com/idojig/2/quiet
Comment 4 Rick Byers 2014-12-09 08:30:28 PST
Assigning to Ben per #c1
Comment 5 Bar Ziony 2016-02-04 16:32:04 PST
Just wanted to ask if this was implemented already, and if not, if it in the works?

Thanks!
Comment 6 Radar WebKit Bug Importer 2016-02-10 15:17:28 PST
<rdar://problem/24597318>
Comment 7 Dean Jackson 2016-02-10 15:29:07 PST
http://www.rbyers.net/paint.html#points is a compelling demo
Comment 8 Rick Byers 2016-02-10 19:10:21 PST
Thanks Dean.  The drawing example is easiest to visualize but is fairly niche.  The really compelling scenarios IMHO are the dragging / custom scroll cases that are so common.  Most such existing pages got noticeably smoother (when dragging slowly) when we shipped this in chromium.
Comment 9 Bar Ziony 2016-04-20 03:37:29 PDT
Hi Dean,

Is this planned for an upcoming release perhaps? Just wanted to get some updated info about this feature.

Thanks.
Comment 10 Antoine Quint 2016-06-14 12:57:08 PDT
<rdar://problem/9564176>
Comment 11 Antoine Quint 2016-06-14 13:25:13 PDT
Created attachment 281273 [details]
Patch
Comment 12 Antoine Quint 2016-06-14 13:46:40 PDT
Created attachment 281277 [details]
Patch
Comment 13 WebKit Commit Bot 2016-06-14 13:48:49 PDT
Attachment 281277 [details] did not pass style-queue:


ERROR: Source/WebKit2/Shared/ios/NativeWebTouchEventIOS.mm:99:  Comma should be at the beginning of the line in a member initialization list.  [whitespace/init] [4]
ERROR: Source/WebCore/ChangeLog:12:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 2 in 17 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 14 Benjamin Poulain 2016-06-14 14:03:44 PDT
Comment on attachment 281277 [details]
Patch

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

> Source/WebCore/platform/Widget.cpp:144
> +    if (!windowFraction.isEmpty()) {
> +        const int kFactor = 1000;
> +        IntPoint parentLineEnd = this->convertFromContainingWindow(flooredPoint + roundedIntSize(windowFraction.scaledBy(kFactor)));
> +        FloatSize parentFraction = (parentLineEnd - parentPoint).scaledBy(1.0f / kFactor);
> +        parentPoint.move(parentFraction);
> +    }

Any way to change convertFromContainingWindow() to handle floating point coordinates instead?

Linear interpolation of the decimal part is only gonna work for a small subset of transforms.

> Source/WebKit2/UIProcess/WebPageProxy.cpp:1944
> +        TrackingType touchPointTrackingType = m_scrollingCoordinatorProxy->eventTrackingTypeForPoint(roundedIntPoint(touchPoint.location()));

This is unfortunate.
I don't know how complicated it would be to add Region::contains() for FloatPoint.
Comment 15 Antoine Quint 2016-06-14 14:47:27 PDT
(In reply to comment #14)
> Comment on attachment 281277 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=281277&action=review
> 
> > Source/WebCore/platform/Widget.cpp:144
> > +    if (!windowFraction.isEmpty()) {
> > +        const int kFactor = 1000;
> > +        IntPoint parentLineEnd = this->convertFromContainingWindow(flooredPoint + roundedIntSize(windowFraction.scaledBy(kFactor)));
> > +        FloatSize parentFraction = (parentLineEnd - parentPoint).scaledBy(1.0f / kFactor);
> > +        parentPoint.move(parentFraction);
> > +    }
> 
> Any way to change convertFromContainingWindow() to handle floating point
> coordinates instead?

We would have to change quite a few methods, but I don't see why we couldn't. I'm not certain this is warranted though.

> > Source/WebKit2/UIProcess/WebPageProxy.cpp:1944
> > +        TrackingType touchPointTrackingType = m_scrollingCoordinatorProxy->eventTrackingTypeForPoint(roundedIntPoint(touchPoint.location()));
> 
> This is unfortunate.
> I don't know how complicated it would be to add Region::contains() for
> FloatPoint.

I think we could change the signature of Region::contains(const IntPoint& point) to take in a FloatPoint and operate on float values instead. I'll look into this.
Comment 16 Rick Byers 2016-06-14 15:11:42 PDT
(In reply to comment #15)
> (In reply to comment #14)
> > Comment on attachment 281277 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=281277&action=review
> > 
> > > Source/WebCore/platform/Widget.cpp:144
> > > +    if (!windowFraction.isEmpty()) {
> > > +        const int kFactor = 1000;
> > > +        IntPoint parentLineEnd = this->convertFromContainingWindow(flooredPoint + roundedIntSize(windowFraction.scaledBy(kFactor)));
> > > +        FloatSize parentFraction = (parentLineEnd - parentPoint).scaledBy(1.0f / kFactor);
> > > +        parentPoint.move(parentFraction);
> > > +    }
> > 
> > Any way to change convertFromContainingWindow() to handle floating point
> > coordinates instead?
> 
> We would have to change quite a few methods, but I don't see why we
> couldn't. I'm not certain this is warranted though.

FWIW I felt really dirty doing this linear interpolation hack in blink (and left myself a FIXME to clean it up) but honestly after 2 years I haven't heard of a single example of anyone noticing it being less than perfect in the edge cases (rotated iframes mainly I guess).  I started to go down the path of fully supporting float but it exploded into a ton of work so I put it off.  I assumed we'd have other reasons to want fractional co-ordinate conversion between widgets but it's never come up since.  Maybe that was the wrong decision though - certainly feels wrong.
Comment 17 Ge Yang 2016-09-11 01:59:38 PDT
Hey Guys, 

I'm trying to use the apple pencil on an iPad pro with canvas to replace native apps, this is when I realized that the Touch.clientX and clientY are quantized to integers. 

Floating point would be very important for this application! and I think using apple pencil to draw on iPad pros is going to be a common thing to do in the future. It would be a shame if we can't do this properly in the browser.

Thanks for all of the work!

Ge
Comment 18 Brady Eidson 2017-08-19 16:01:42 PDT
Comment on attachment 281277 [details]
Patch

r-, as this has been pending review for over a year now. It is near-impossible that this patch still applies to trunk and unlikely to still be relevant in its current form.