Touch processing for OSR, different API

#104 Open
Repository
cef_osrtouch
Branch
touch_pr
Repository
cef
Branch
master

Bitbucket cannot automatically merge this request.

The commits that make up this pull request have been removed.

Bitbucket cannot automatically merge this request due to conflicts.

Review the conflicts on the Overview tab. You can then either decline the request or merge it manually on your local system using the following commands:

git checkout master
git remote add kebbyfr/cef_osrtouch https://bitbucket.org/kebbyfr/cef_osrtouch.git
git fetch kebbyfr/cef_osrtouch
git merge --no-ff -m 'Merged in kebbyfr/cef_osrtouch/touch_pr (pull request #104)' remotes/kebbyfr/cef_osrtouch/touch_pr
Author
  1. Tammo Hinrichs
Reviewers
Description

An alternative approach to https://bitbucket.org/chromiumembedded/cef/pull-requests/100 - please read my comments there. This PR is the implementation of what I suggested.

Original issue: https://bitbucket.org/chromiumembedded/cef/issues/1059/cef3-multi-touch-support-with-offscreen

Missing: CefClient usage (I can probably do Win32, anyone up for Linux/Mac/etc?), unit tests

  • Issues #100: Add support for off-screen rendering resolved

Comments (54)

  1. Nishant Kaushik

    (I hope I am not stepping over the lines here, but we desperately need Touch support :) So my 2 cents on closing this -

    @Marshall Greenblatt , @Tammo Hinrichs I have added UnitTests, and OSX sample usage to this pull request. It includes -

    0) Since the Pull request was out of date, I have rebased it to the latest master.

    1) Sample client usage for OSX. Since there is no touch screen on OSX devices, the sample usage simulates touch screen by mapping touch events on TrackPad to Touch Events on Screen (and may be slightly flaky hence).To ensure it does not interfere with other Trackpad events, this mapping is only enabled if touch-events=enabled commandline is passed and caps lock key is on.

    2) Sample Client Usage Win: Sample client usage for Win is based on WM_TOUCH (and not WM_POINTER) since WM_POINTER is not available on Win7, and a lot of clients will likely require Win7 support.

    3) UnitTests for SendTouchEvent API. The UnitTest ensures that SendTouchEvent are working by validating that appropriate JavaScript events are triggered.

    4) Style cleanups.

    A live patch (pull request) is here - "curl https://api.bitbucket.org/2.0/repositories/nishant_kaushik/acrocef/pullrequests/6/patch -L"

    The unit tests are working on all platforms and the sample simulating Touch by mapping Trackpad seems to work reasonably well. Please review. @Tammo Hinrichs It'll be great if you can help with adding Win sample usage to close this long standing issue.

        1. Nishant Kaushik

          Thanks Tammo, I can understand that :). I'll try to add Windows sample usage and if it works update the pull request with that and you can help review that to make sure it is correct.

        2. Nishant Kaushik

          @Tammo Hinrichs I have added Windows sample usage also. The Windows Sample client usage for is based on WM_TOUCH (and not WM_POINTER) since WM_POINTER is not available on Win7, and we (and a lot of other clients likely) still require Win7 support.

          Sample Usage appended to Pull Request itself.

          Or you can look at the commit -

          https://bitbucket.org/nishant_kaushik/acrocef/commits/0ee2c5a33696700e59efaafd0e8fcaaab7cf70f3?at=master

          I also noticed two minor issues that I have updated in PR below and have fixed. In addition this commit also includes the style fixes and cleanup. Please review the sample usages and other changes.

        3. Nishant Kaushik

          @Tammo Hinrichs [again overstepping the boundaries but :)] Please let me know if I can help further with rebasing these changes and keeping them in Sync with the master to ease with merging to Trunk. (While my other PRs were under review I had automated, rebasing, building and validating my PRs on Win/Mac.) Since we are now using this Touch implementation, I am doing the same with this PR by forking this as a local PR, with original commits rebased to master and sample usage + cleanup) https://bitbucket.org/nishant_kaushik/acrocef/pull-requests/6

          (I would need write access to https://bitbucket.org/kebbyfr/cef_osrtouch for that though)

          1. Tammo Hinrichs author

            Not overstepping the boundaries at all; you seem to have the time to complete this PR (and I don't really) so I gave you write access to the repo. Update ahead as you wish. And thank you! :)

              1. Dmitry Azaraev

                Thanks to all. As for Windows example while it miss WM_POINTER usage, i suggest just to include comment about in sample code, otherwise followers miss this point. (Argues about Windows 7 is correct, but on modern systems WM_POINTER support should be mandatory choice. )

              2. Tammo Hinrichs author

                Looks very good to me now. Thanks!

                Speaking of overstepping boundaries and now becoming the guy who demands stuff: Could you check John's comment on CefRenderWidgetHostViewOSR::OnGestureEvent() below and add the three lines of code he suggested?

  2. Alexey Makarov

    @Nishant Kaushik. I have complie CEF 3071 with changes from PR 104 and test on Win10. cefclient.exe --off-screen-rendering-enabled --touch-events-enabled You see, the problem is that though touch-scroll works just fine but if there is some texting and other objects then it become selected while I try to touch-scroll the page. Screen captupe link https://yadi.sk/i/fm-LggO-3KooNf Problem in the CefClient.

  3. Cristian Carli

    Hello everyone i follow this project for long time and I want to try it because is extremely charming but i tried to download the project and build it and I obtain a lot of error and I don't know how to solve it. I have also tried to start from a standard build and follow the instruction from the guide but i obtain an error during the extracting. So is there anyone that can can send me the build project with .sln file ? Thanks to much

  4. Benjamin Laffont

    Hello,

    have you tested it on maps.google.fr ?

    I have included it on the 55 build version with CefSharp on it and i was unable to pinch to zoom on maps. (it can be my own integration that have failed...)

    Thanks,

      1. Benjamin Laffont

        Hello,

        Sorry to insist, it will be my last post :D

        I have found the problem and make my own resolution (maybe not the best...) My touch problem was located in the render_widget_host_view_osr.cc file, in the CefRenderWidgetHostViewOSR::TranslateTouchEvent method

        Symptom : When you totally disable mouse events, you cannot pinch to zoom in maps.google.fr. You can invoke the devtool on this website and launch a monitorEvents(document) on the console. You will see (on a fully touch action) some mouse events.

        Problem : In the given method, we call CreateWebTouchEventFromMotionEvent but this method does not fill properly the Id so that when we call MarkUnchangedTouchPointsAsStationary, we mark ALL events as stationary (even the last move event sent).

        Solution : I just comment the call of MarkUnchangedTouchPointsAsStationary. This is not the best but i have now a fully functional solution with native touch/pinch to zoom.

        Let me know if it is the right solution :)

        Regards,

        1. Nishant Kaushik

          Hi Benjamin,

          (CreateWebTouchEventFromMotionEvent is actually implemented is chromium/src/ui/events/blink/blink_event_util.cc and is the same code that used by chromium and in non osr mode and that too is working as expected. So the issue likely lies elsewhere.)

          Pinch to zoom on maps.google.fr seems to be working fine in sample CefClient (please make sure you pass touch-events=enabled command line flag). Can you please confirm if this is working in sample Cef client.

      1. Alexey Makarov

        @Nishant Kaushik There is an error OnTouchEventAck has wrong arguments in cef\libcef\browser\osr\render_widget_host_view_osr.cc gesture_provider_.OnTouchEventAck(touch.event.unique_touch_event_id, event_consumed, false); when i try to complile PR104 on branch 3107 with latest changes. It depends on the branch?

  5. Alexey Makarov

    @Nishant Kaushik i found a small bug in cefclient. If cefclient starts on touch monitor without touch-events=enabled flag no events MOUSEFROMTOUCH happen and there is no navigation control etc. May be it worth analyzing this flag in the function IsMouseEventFromTouch()?

  6. Marshall Greenblatt

    @Nishant Kaushik Please try compiling this PR at current master. Fails with:

    FAILED: obj/cef/libcef_static/browser_platform_delegate_create.obj
    ninja -t msvc -e environment.x86 -- "c:\program files (x86)\microsoft visual studio 14.0\vc\bin\amd64_x86/cl.exe" /nologo /showIncludes  @obj/cef/libcef_static/browser_platform_delegate_create.obj.rsp /c ../../cef/libcef/browser/browser_platform_delegate_create.cc /Foobj/cef/libcef_static/browser_platform_delegate_create.obj /Fd"obj/cef/libcef_static_cc.pdb"
    c:\program files (x86)\microsoft visual studio 14.0\vc\include\memory(1630): error C2259: 'CefBrowserPlatformDelegateBackground': cannot instantiate abstract class
    c:\program files (x86)\microsoft visual studio 14.0\vc\include\memory(1630): note: due to following members:
    c:\program files (x86)\microsoft visual studio 14.0\vc\include\memory(1630): note: 'void CefBrowserPlatformDelegate::SendTouchEvent(const blink::WebTouchEvent &)': is abstract
    C:\code\chromium_git\chromium\src\cef\libcef/browser/browser_platform_delegate.h(152): note: see declaration of 'CefBrowserPlatformDelegate::SendTouchEvent'
    c:\program files (x86)\microsoft visual studio 14.0\vc\include\memory(1630): note: 'bool CefBrowserPlatformDelegate::TranslateTouchEvent(blink::WebTouchEvent &,const CefTouchEvent &)': is abstract
    C:\code\chromium_git\chromium\src\cef\libcef/browser/browser_platform_delegate.h(200): note: see declaration of 'CefBrowserPlatformDelegate::TranslateTouchEvent'
    C:\code\chromium_git\chromium\src\base/memory/ptr_util.h(25): note: see reference to function template instantiation 'std::unique_ptr<T,std::default_delete<_Ty>> std::make_unique<T,std::unique_ptr<CefBrowserPlatformDelegateNative,std::default_delete<CefBrowserPlatformDelegateNative>>>(std::unique_ptr<CefBrowserPlatformDelegateNative,std::default_delete<CefBrowserPlatformDelegateNative>> &&)' being compiled
            with
            [
                T=CefBrowserPlatformDelegateBackground,
                _Ty=CefBrowserPlatformDelegateBackground
            ]
    ../../cef/libcef/browser/browser_platform_delegate_create.cc(89): note: see reference to function template instantiation 'std::unique_ptr<T,std::default_delete<_Ty>> base::MakeUnique<CefBrowserPlatformDelegateBackground,std::unique_ptr<CefBrowserPlatformDelegateNative,std::default_delete<CefBrowserPlatformDelegateNative>>>(std::unique_ptr<CefBrowserPlatformDelegateNative,std::default_delete<CefBrowserPlatformDelegateNative>> &&)' being compiled
            with
            [
                T=CefBrowserPlatformDelegateBackground,
                _Ty=CefBrowserPlatformDelegateBackground
            ]
    
    1. Marshall Greenblatt

      Here's the fix:

      diff --git libcef/browser/extensions/browser_platform_delegate_background.cc libcef/browser/extensions/browser_platform_delegate_background.cc
      index fc4e5356..aa55deba 100644
      --- libcef/browser/extensions/browser_platform_delegate_background.cc
      +++ libcef/browser/extensions/browser_platform_delegate_background.cc
      @@ -58,6 +58,11 @@ void CefBrowserPlatformDelegateBackground::SendMouseWheelEvent(
         // Nothing to do here.
       }
      
      +void CefBrowserPlatformDelegateBackground::SendTouchEvent(
      +    const blink::WebTouchEvent& web_event) {
      +  // Nothing to do here.
      +}
      +
       void CefBrowserPlatformDelegateBackground::SendFocusEvent(bool setFocus) {
         // Nothing to do here.
       }
      @@ -113,6 +118,12 @@ void CefBrowserPlatformDelegateBackground::TranslateWheelEvent(
         native_delegate_->TranslateWheelEvent(result, mouse_event, deltaX, deltaY);
       }
      
      +bool CefBrowserPlatformDelegateBackground::TranslateTouchEvent(
      +    blink::WebTouchEvent& result,
      +    const CefTouchEvent& touch_event) {
      +  return native_delegate_->TranslateTouchEvent(result, touch_event);
      +}
      +
       CefEventHandle CefBrowserPlatformDelegateBackground::GetEventHandle(
           const content::NativeWebKeyboardEvent& event) const {
         return native_delegate_->GetEventHandle(event);
      diff --git libcef/browser/extensions/browser_platform_delegate_background.h libcef/browser/extensions/browser_platform_delegate_background.h
      index c21d4f67..56127144 100644
      --- libcef/browser/extensions/browser_platform_delegate_background.h
      +++ libcef/browser/extensions/browser_platform_delegate_background.h
      @@ -26,6 +26,7 @@ class CefBrowserPlatformDelegateBackground
         void SendKeyEvent(const content::NativeWebKeyboardEvent& event) override;
         void SendMouseEvent(const blink::WebMouseEvent& event) override;
         void SendMouseWheelEvent(const blink::WebMouseWheelEvent& event) override;
      +  void SendTouchEvent(const blink::WebTouchEvent& web_event) override;
         void SendFocusEvent(bool setFocus) override;
         gfx::Point GetScreenPoint(const gfx::Point& view) const override;
         void ViewText(const std::string& text) override;
      @@ -46,6 +47,8 @@ class CefBrowserPlatformDelegateBackground
                                  const CefMouseEvent& mouse_event,
                                  int deltaX,
                                  int deltaY) const override;
      +  bool TranslateTouchEvent(blink::WebTouchEvent& result,
      +                           const CefTouchEvent& touch_event) override;
         CefEventHandle GetEventHandle(
             const content::NativeWebKeyboardEvent& event) const override;
         std::unique_ptr<CefFileDialogRunner> CreateFileDialogRunner() override;
      
  7. Marshall Greenblatt

    @Nishant Kaushik When building with GN_DEFINES=is_component_build=true on Windows:

    c:/code/depot_tools/win_tools-2_7_6_bin/python/bin/python.exe ../../build/toolchain/win/tool_wrapper.py link-wrapper environment.x86 False link.exe /nologo /IMPLIB:./libcef.dll.lib /DLL /OUT:./libcef.dll /PDB:./libcef.dll.pdb @./libcef.dll.rsp
    libcef_static.lib(render_widget_host_view_osr.obj) : error LNK2019: unresolved external symbol "class blink::WebTouchEvent __cdecl ui::CreateWebTouchEventFromMotionEvent(class ui::MotionEvent const &,bool)" (?CreateWebTouchEventFromMotionEvent@ui@@YA?AVWebTouchEvent@blink@@ABVMotionEvent@1@_N@Z) referenced in function "public: bool __thiscall CefRenderWidgetHostViewOSR::TranslateTouchEvent(class blink::WebTouchEvent &,class CefStructBase<struct CefTouchEventTraits> const &)" (?TranslateTouchEvent@CefRenderWidgetHostViewOSR@@QAE_NAAVWebTouchEvent@blink@@ABV?$CefStructBase@UCefTouchEventTraits@@@@@Z)
    libcef_static.lib(render_widget_host_view_osr.obj) : error LNK2019: unresolved external symbol "class blink::WebGestureEvent __cdecl ui::CreateWebGestureEventFromGestureEventData(struct ui::GestureEventData const &)" (?CreateWebGestureEventFromGestureEventData@ui@@YA?AVWebGestureEvent@blink@@ABUGestureEventData@1@@Z) referenced in function "public: virtual void __thiscall CefRenderWidgetHostViewOSR::OnGestureEvent(struct ui::GestureEventData const &)" (?OnGestureEvent@CefRenderWidgetHostViewOSR@@UAEXABUGestureEventData@ui@@@Z)
    libcef_static.lib(render_widget_host_view_osr.obj) : error LNK2019: unresolved external symbol "bool __cdecl content::IsPinchToZoomEnabled(void)" (?IsPinchToZoomEnabled@content@@YA_NXZ) referenced in function "public: __thiscall CefRenderWidgetHostViewOSR::CefRenderWidgetHostViewOSR(unsigned int,class content::RenderWidgetHost *,class CefRenderWidgetHostViewOSR *,bool)" (??0CefRenderWidgetHostViewOSR@@QAE@IPAVRenderWidgetHost@content@@PAV0@_N@Z)
    ./libcef.dll : fatal error LNK1120: 3 unresolved externals
    ninja: build stopped: subcommand failed.
    command took 0:1:20.65 (80.65s total)
    
    1. Marshall Greenblatt

      Here's the fix:

      diff --git patch/patches/component_build_1617.patch patch/patches/component_build_1617.patch
      index 056bd3f7..5a3a7f04 100644
      --- patch/patches/component_build_1617.patch
      +++ patch/patches/component_build_1617.patch
      @@ -24,6 +24,19 @@ index 7563ce48bf5e..6c594749d57b 100644
         public:
          explicit ContentServiceManagerMainDelegate(const ContentMainParams& params);
          ~ContentServiceManagerMainDelegate() override;
      +diff --git content/common/content_switches_internal.h content/common/content_switches_internal.h
      +index 8bb2c96934d9..aaec57862569 100644
      +--- content/common/content_switches_internal.h
      ++++ content/common/content_switches_internal.h
      +@@ -15,7 +15,7 @@ class CommandLine;
      + 
      + namespace content {
      + 
      +-bool IsPinchToZoomEnabled();
      ++bool CONTENT_EXPORT IsPinchToZoomEnabled();
      + #if defined(OS_WIN)
      + // Returns whether Win32k lockdown is enabled for child processes or not.
      + bool IsWin32kLockdownEnabled();
       diff --git third_party/WebKit/Source/controller/BUILD.gn third_party/WebKit/Source/controller/BUILD.gn
       index dbcd69337de8..fc0b6aca080f 100644
       --- third_party/WebKit/Source/controller/BUILD.gn
      @@ -36,3 +49,46 @@ index dbcd69337de8..fc0b6aca080f 100644
            "//skia",
            "//third_party/WebKit/Source/core",
            "//third_party/WebKit/Source/modules",
      +diff --git ui/events/blink/BUILD.gn ui/events/blink/BUILD.gn
      +index ce5d02c4bc6b..a8b2c0018c5f 100644
      +--- ui/events/blink/BUILD.gn
      ++++ ui/events/blink/BUILD.gn
      +@@ -40,6 +40,8 @@ source_set("blink") {
      +     "//ui/latency",
      +   ]
      + 
      ++  defines = [ "CONTENT_IMPLEMENTATION" ]
      ++
      +   if (is_win) {
      +     sources += [
      +       "web_input_event_builders_win.cc",
      +diff --git ui/events/blink/blink_event_util.h ui/events/blink/blink_event_util.h
      +index 0e9b75c8d37d..b048e5ea670b 100644
      +--- ui/events/blink/blink_event_util.h
      ++++ ui/events/blink/blink_event_util.h
      +@@ -7,6 +7,7 @@
      + 
      + #include <memory>
      + 
      ++#include "content/common/content_export.h"
      + #include "third_party/WebKit/public/platform/WebGestureEvent.h"
      + #include "third_party/WebKit/public/platform/WebInputEvent.h"
      + #include "third_party/WebKit/public/platform/WebTouchEvent.h"
      +@@ -39,7 +40,7 @@ CoalesceScrollAndPinch(const blink::WebGestureEvent* second_last_event,
      +                        const blink::WebGestureEvent& last_event,
      +                        const blink::WebGestureEvent& new_event);
      + 
      +-blink::WebTouchEvent CreateWebTouchEventFromMotionEvent(
      ++blink::WebTouchEvent CONTENT_EXPORT CreateWebTouchEventFromMotionEvent(
      +     const MotionEvent& event,
      +     bool may_cause_scrolling);
      + 
      +@@ -51,7 +52,7 @@ blink::WebGestureEvent CreateWebGestureEvent(const GestureEventDetails& details,
      +                                              uint32_t unique_touch_event_id);
      + 
      + // Convenience wrapper for |CreateWebGestureEvent| using the supplied |data|.
      +-blink::WebGestureEvent CreateWebGestureEventFromGestureEventData(
      ++blink::WebGestureEvent CONTENT_EXPORT CreateWebGestureEventFromGestureEventData(
      +     const GestureEventData& data);
      + 
      + int EventFlagsToWebEventModifiers(int flags);
      
    1. Nishant Kaushik

      Thanks Marshall. I have applied the patches and fixed the build. Also tests seem to be successful in Release build on both Win and Mac. I am building the debug build and will fix or update about Test failure.

    2. Nishant Kaushik

      @Marshall Greenblatt, I verified the tests on Win10/Win 7 machines and OSX on two different machines, for both Debug and Release configurations 5 runs for each instances. I did not observe any test failure or time out. Can you please confirm if you are seeing the failure still on latest PR? (After I applied the changes you suggested and latest rebase). Also can you please have 2-3 runs of test in case of failure to narrow down if the failure is real for you?

      1. Marshall Greenblatt

        It works for me on Linux but fails on a Win10 machine with a high-dpi display (dual-boot MacBook Pro). Perhaps something is scaling the mouse position incorrectly based on display information?

  8. Alexey Makarov

    @Nishant Kaushik, the question is a bit aside from the topic. I need to determine the moment when the node definite types (element) touched (clicked) but if touch (click) on the same element, the node has not changed.
    Cef has an event NodeСhanged. In the case of a Mouse Click, NodeСhanged occurs before the event MouseUp. In MouseUp i can determine clicked Element(Node) and compare with previous node. In the case of Touch, NodeСhanged occurs after all events. In touch/click events i dont know touched Element.

    How can i recieve the Node before touchUp event, if it`s even possible?

  9. Dennis Mohr

    Hello, thanks for this PR it's really helping me putting things in my app forward.

    I have some feedback after testing this today on a Linux x64 host. I realized that sometimes the browser doesn't respond to touch events anymore (mouse works fine).

    I found that this was caused by a touch sequence starting but not ending, so for example when I use long touch to trigger a context menu right under the cursor the browserhost doesn't receive a TET_RELEASED or TET_CANCELLED.

    Right now I, as application programmer, am forced to internally track all touches (which this PR already does) and end them synthetically when I detect a focus change for example. It would be a nice gimmick to have a function to cancel any pending touches.

    Maybe it would be a good idea to automatically cancel all pending touch sequences on different occasions:

    • host->SendFocusEvent(FALSE) - we lost focus. No need for left over touch sequences, don't know if TET_RELEASED or TET_CANCELLED in this case.
    • OnLoadStarted? I don't know if that's really a good idea we left the page, what are pending touches good for? But OnLoadStarted doesn't necessarily mean page change.
    • A timer? Not very elegant but who keeps a touch going for more than 30s for example?

    I can write a patch after some considerations, what are your thoughts?

  10. sticman22

    Hi, I've been playing with this pull request on 64-bit Linux (CentOS) and noticed some odd behavior. I applied this patch to Release Branch 3239 and tested it with the site https://frames-per-second.appspot.com in CefClient. Without offscreen rendering I'm able to render and paint at 60fps (Screenshot). With off-screen-rendering it is only able to paint at 30fps (Screenshot).

    When I apply this patch performance drops further and I can only render at 30fps and it struggled to paint at 30fps. (Screenshot)

    Any suggestions as to what may cause this?