WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
92293
Improve touch adjustment for targetting small controls.
https://bugs.webkit.org/show_bug.cgi?id=92293
Summary
Improve touch adjustment for targetting small controls.
Kevin Ellis
Reported
2012-07-25 13:56:26 PDT
Improve touch adjustment for targetting small controls.
Attachments
Patch
(16.47 KB, patch)
2012-07-25 14:11 PDT
,
Kevin Ellis
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Kevin Ellis
Comment 1
2012-07-25 14:11:25 PDT
Created
attachment 154441
[details]
Patch
Kevin Ellis
Comment 2
2012-07-25 14:17:15 PDT
Separating the scoring adjustment out of
bug 91894
(search cancel button is hard to activate with a tap gesture even if touch adjustment is enabled).
Allan Sandfeld Jensen
Comment 3
2012-07-27 02:11:34 PDT
Looks good, with the test-case any future improvements to the distance method can check if they truly are better.
Allan Sandfeld Jensen
Comment 4
2012-07-27 02:17:19 PDT
Btw, nice improvements to the touch-adjustment test-functions. I think we should soon try to collect some of the best versions of the helper functions for touch-adjustment test-cases into a small helper file and use them more consistently; but that is a task for another bug another time.
Antonio Gomes
Comment 5
2012-07-27 06:33:43 PDT
Comment on
attachment 154441
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=154441&action=review
> Source/WebCore/page/TouchAdjustment.cpp:253 > +float hybridDistanceFunction(const IntPoint& touchHotspot, const IntRect& touchArea, const SubtargetGeometry& subtarget)
I think we do not need the term "distance" here anymore, but I am out of good suggestions now.
Kevin Ellis
Comment 6
2012-07-27 06:49:34 PDT
Comment on
attachment 154441
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=154441&action=review
>> Source/WebCore/page/TouchAdjustment.cpp:253 >> +float hybridDistanceFunction(const IntPoint& touchHotspot, const IntRect& touchArea, const SubtargetGeometry& subtarget) > > I think we do not need the term "distance" here anymore, but I am out of good suggestions now.
It is still a distance score with 0.0 being a perfect hit and 1.0 being a glancing touch; however, I can see your point. Calling it a "cost" function seems a bit too generic, but might be appropriate. Not so keen on using a term like goodness of fit (but throwing it out there anyways), as it suggests strong statistical implications that are not strictly valid. Does hybridCostFunction, sound better than hybridDistanceFunction? We can certainly revisit the naming.
Allan Sandfeld Jensen
Comment 7
2012-07-27 07:03:30 PDT
(In reply to
comment #6
)
> (From update of
attachment 154441
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=154441&action=review
> > >> Source/WebCore/page/TouchAdjustment.cpp:253 > >> +float hybridDistanceFunction(const IntPoint& touchHotspot, const IntRect& touchArea, const SubtargetGeometry& subtarget) > > > > I think we do not need the term "distance" here anymore, but I am out of good suggestions now. > > It is still a distance score with 0.0 being a perfect hit and 1.0 being a glancing touch; however, I can see your point. Calling it a "cost" function seems a bit too generic, but might be appropriate. Not so keen on using a term like goodness of fit (but throwing it out there anyways), as it suggests strong statistical implications that are not strictly valid. > > Does hybridCostFunction, sound better than hybridDistanceFunction? We can certainly revisit the naming.
The function used to bestZoomableNode is not a distance either, it is infact also some weighted hybrid. I orginally used the mathematical term "metric" (a function with distance like properties) for these functions, but using 'distance' even if slightly wrong seemed to be more accesible to other developers.
WebKit Review Bot
Comment 8
2012-07-27 07:16:35 PDT
Comment on
attachment 154441
[details]
Patch Rejecting
attachment 154441
[details]
from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: t-queue/Source/WebKit/chromium/third_party/libvpx --revision 147803 --non-interactive --force --accept theirs-conflict --ignore-externals' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' 39>At revision 147803. ________ running '/usr/bin/python tools/clang/scripts/update.py --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' ________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' Updating webkit projects from gyp files... Full output:
http://queues.webkit.org/results/13362687
WebKit Review Bot
Comment 9
2012-07-27 10:59:52 PDT
Comment on
attachment 154441
[details]
Patch Clearing flags on attachment: 154441 Committed
r123889
: <
http://trac.webkit.org/changeset/123889
>
WebKit Review Bot
Comment 10
2012-07-27 10:59:56 PDT
All reviewed patches have been landed. Closing bug.
Allan Sandfeld Jensen
Comment 11
2012-08-01 06:28:47 PDT
Comment on
attachment 154441
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=154441&action=review
Please get rid of the XHTML'isms.
> LayoutTests/touchadjustment/small-target-test.html:65 > + <input id="task" type="text" value="Task 1"/>
This is wrong but probably harmless. Flat tags does not work in HTML.
> LayoutTests/touchadjustment/small-target-test.html:70 > + <div id = "remove-button" onclick="onClick();"/>
This is completely wrong. Flat tags do not work in HTML, and fixing this error breaks the test-case.
> LayoutTests/touchadjustment/small-target-test.html:76 > + <div id = "drop-down-selector" onclick="onClick();"/>
Ditto.
Allan Sandfeld Jensen
Comment 12
2012-08-01 06:41:32 PDT
(In reply to
comment #11
)
> (From update of
attachment 154441
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=154441&action=review
> > Please get rid of the XHTML'isms. >
Well, actually I've got them. I will fix them in
bug #92869
. I noticed it will I was cleaning up for #92869 anyway.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug