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
Kevin Ellis
Comment 1 2012-07-25 14:11:25 PDT
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.