We use current time for touch event's timestamp instead of timestamp when event occurs. Timestamp of touch event is used to implement flick gesture, so it should be real value for accurate gesture.
Created attachment 220291 [details] Patch
Comment on attachment 220291 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=220291&action=review > Source/WebKit2/UIProcess/API/efl/EwkView.cpp:1337 > +void EwkView::handleTouchMouseDown(void* /* data */, Evas* /* canvas */, Evas_Object* ewkView, void* eventInfo) How about handleTouchOrMouseDown ? > Source/WebKit2/UIProcess/API/efl/EwkView.cpp:1339 > + toEwkView(ewkView)->feedTouchEvents(EWK_TOUCH_START, static_cast<Evas_Event_Mouse_Down*>(eventInfo)->timestamp / 1000.0); Should we divide timestamp here? > Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:355 > +void GestureRecognizer::noGesture(WKEventType type, WKArrayRef touchPoints, double timestamp) Can we give more correct information? seconds or milliseconds?
Comment on attachment 220291 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=220291&action=review >> Source/WebKit2/UIProcess/API/efl/EwkView.cpp:1337 >> +void EwkView::handleTouchMouseDown(void* /* data */, Evas* /* canvas */, Evas_Object* ewkView, void* eventInfo) > > How about handleTouchOrMouseDown ? Handlers are for touch, so it is better to rename to handleMouseDownForTouch. I will change the name. >> Source/WebKit2/UIProcess/API/efl/EwkView.cpp:1339 >> + toEwkView(ewkView)->feedTouchEvents(EWK_TOUCH_START, static_cast<Evas_Event_Mouse_Down*>(eventInfo)->timestamp / 1000.0); > > Should we divide timestamp here? Timestamp in the webkit is usually double type and it's unit is second. So, I think it is better to divide evas event's timestamp (millisecond) with 1000.0 as early as possible. >> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:355 >> +void GestureRecognizer::noGesture(WKEventType type, WKArrayRef touchPoints, double timestamp) > > Can we give more correct information? seconds or milliseconds? Unit of most timestamp in the webkit is second, so I think we can use timestamp here.
Created attachment 220292 [details] Patch Rename touch handlers.
(In reply to comment #3) > (From update of attachment 220291 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=220291&action=review > > >> Source/WebKit2/UIProcess/API/efl/EwkView.cpp:1337 > >> +void EwkView::handleTouchMouseDown(void* /* data */, Evas* /* canvas */, Evas_Object* ewkView, void* eventInfo) > > > > How about handleTouchOrMouseDown ? > > Handlers are for touch, so it is better to rename to handleMouseDownForTouch. > > I will change the name. > OK. > >> Source/WebKit2/UIProcess/API/efl/EwkView.cpp:1339 > >> + toEwkView(ewkView)->feedTouchEvents(EWK_TOUCH_START, static_cast<Evas_Event_Mouse_Down*>(eventInfo)->timestamp / 1000.0); > > > > Should we divide timestamp here? > > Timestamp in the webkit is usually double type and it's unit is second. > So, I think it is better to divide evas event's timestamp (millisecond) with 1000.0 as early as possible. > OK. I preferred lazy calculation because we can have only one division operator but it's minor issue. I don't have objection. > >> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:355 > >> +void GestureRecognizer::noGesture(WKEventType type, WKArrayRef touchPoints, double timestamp) > > > > Can we give more correct information? seconds or milliseconds? > > Unit of most timestamp in the webkit is second, so I think we can use timestamp here. OK.
Comment on attachment 220292 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=220292&action=review > Source/WebKit2/UIProcess/API/efl/EwkView.cpp:1352 > +void EwkView::handleMultiDownForTouch(void* /* data */, Evas* /* canvas */, Evas_Object* ewkView, void* eventInfo) It looks code is a little messed up /* data */ -> /*data*/ ,or how about using UNUSED_PARAM() ? > Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:412 > +void GestureRecognizer::doubleTapGesture(WKEventType type, WKArrayRef touchPoints, double timestamp) Don't you need to use UNUSED_PARAM for timestamp ? > Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:469 > +void GestureRecognizer::pinchGesture(WKEventType type, WKArrayRef touchPoints, double timestamp) ditto. > Source/WebKit2/UIProcess/API/efl/GestureRecognizer.h:59 > + void noGesture(WKEventType, WKArrayRef, double timestamp); Can't we use set|get functions instead of passing a timestamp variable ?
Comment on attachment 220292 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=220292&action=review Thanks for review :) >> Source/WebKit2/UIProcess/API/efl/EwkView.cpp:1352 >> +void EwkView::handleMultiDownForTouch(void* /* data */, Evas* /* canvas */, Evas_Object* ewkView, void* eventInfo) > > It looks code is a little messed up > > /* data */ -> /*data*/ ,or how about using UNUSED_PARAM() ? I think it is better to remove the commented parameter name because it is not used. I will modify that. >> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.h:59 >> + void noGesture(WKEventType, WKArrayRef, double timestamp); > > Can't we use set|get functions instead of passing a timestamp variable ? If we use WKTouchEventRef as a parameter, we don't have to pass a timestamp. I will modify that.
Created attachment 221322 [details] Patch Fix for Gyuyoung's comments.
Comment on attachment 221322 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=221322&action=review Looks better than before. r=me together with Ryuan informal review. > Source/WebKit2/UIProcess/API/efl/EwkView.cpp:781 > + evas_object_event_callback_add(m_evasObject, EVAS_CALLBACK_MOUSE_DOWN, handleMouseDownForTouch, sd); This function name change isn't mentioned in ChangeLog. It would be nicer if you mention this change as well.
Created attachment 221325 [details] Patch Update changelog.
Comment on attachment 221325 [details] Patch Clearing flags on attachment: 221325 Committed r162111: <http://trac.webkit.org/changeset/162111>
All reviewed patches have been landed. Closing bug.