Touch processing for OSR, different API

#104 Declined
Repository
Deleted repository
Branch
touch_pr (30d68d3cd62c)
Repository
cef
Branch
master
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

Comments (98)

  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. Rion Wilson-Yue

    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?

  11. Ben Hamrick

    Is there anyway we could get this rebased or upgraded to the newest master?

    Also, what does this need to be merged? We would really like touch support without having to build cef from a custom branch.

  12. Alexey Makarov

    Hi! @Nishant Kaushik, i try to build 3.3239.1723.g071d1c1 with touch support. I found the wrong behavior when selecting items in the drop-down combo box. When i try to select an item from the list -   the combox closes without changing the value of the field, the selection of the item in the list also does not occur. What could be the reason?

  13. Jakub Głuszkiewicz

    i am affraid this awesome path is not longer possible to merge into newer branches. I spent 2 days trying to manually merge this pr into branch 3359,

    after douzen of tries im always ending up with compilation error:

    ../../cef\libcef/browser/osr/motion_event_osr.h(41,22): error: no member named 'WebTouchEvent' in namespace 'blink'
    int id_map_[blink::WebTouchEvent::kTouchesLengthCap];

    it looks like touch api was refactored from blink to webkit? i whish to fix it by my own but dont feel like i know cef/chrommium enough to successfully do it.

    I withdrawed to and trying to build with 3239 in post above someone succeffuly builded it with this branch so im trying to do same,

    can we try to maintain this path up to date? its so important pr, and i dont know why we dont have it merged into master yet,

    if any donation can help let me know on priv , my company gladly will start support this awesome project

      1. Ben Hamrick

        Where you able to figure out why @Jakub could not get touch up to work with this patch? Also do you have a newer patch or could you explain how you made this patch so I can make a new one?

        1. Kaustubh Vats

          I am not sure why Jakub is facing the issue with touch_up since I have built cef with these changes and touch is working correctly. If you want to make a new patch you can do that by making these changes:

          • In motion_event_osr.h, replace #include "third_party/WebKit/public/platform/WebTouchEvent.h" with #include "third_party/blink/public/platform/web_touch_event.h".
          • In motion_event_osr.cc , replace ACTION_POINTER_DOWN with ui::MotionEvent::Action::POINTER_DOWN (do the same for ACTION_UP, ACTION_CANCEL etc.). In CefMotionEventOSR::OnTouch, add fallthrough after TET_MOVED.
          • In render_widget_host_view_osr.cc, add false as third parameter in CreateWebTouchEventFromMotionEvent.
          • In os_rendering_unittest.cc, remove const int kDefaultVerticalScrollbarWidth = 17 .
          1. Jakub Głuszkiewicz

            Hello Guys,

            i builded CEF with this PR with Kaustubh changes with branch 3396 twice. Always the same issue, TOUCH doesnt work correctly,

            U need to touch twice on html widgets to receive any response. Also scrolling html page by swipe gesture doesnt work correctly,

            i will build newer branches with this pr and Kaustubh changes and i will see if that wrong behavior exist,

            If there is, then i can record some movie which will show you wrong behavior,

            As i said i builded 3396 twice from scratch so it cant be my mistake,

            @Kaustubh Vats are you sure that for. ex swpie scrolling is working correctly on your side? Which branch are u using?

            thanks

            1. Kaustubh Vats

              I had built 3396 branch with these changes and swipe scrolling was working correctly. I would be building 3538 branch with touch support in a few days and will try to take a look at this issue.

      2. Cristian Carli

        Hi

        I have tried to download and apply your path to your 3396 cef branch but the patch fails to apply because not find all files to change as the picture belows shows

        Best regards

  14. Jakub Głuszkiewicz

    I can confirm that 3239 is working,

    I can confirm that with newest branch 3396 this pr will not work due to cleaning whole webkit in chrome and moving touch api to blink, also there is no more content/browser/render_host/render_widget_host_view_frame_subscriber.h so build at current state is impossible with 3396

    now im trying to build with still supported 3359 branch, i see webkit headers in repo there so there is chance it will work, i will findout soon and let u know

    thanks

  15. Jakub Głuszkiewicz

    Ok i spent some time to check how deep compatibility we have on this PR,

    latest build where u will be able to build with is branch 3325 (Chrome 65)

    Both currently supported branches 3359 and 3396 are not compatible.

    3359 has refactored motion_event.h parts from enum to class and migration from webkit to blink in gestures already started

    3396 has deep changes with no webkit at all,

    so im affraid at this current state if u want to use this PR latest branch compatible will be 3325

    thanks,

    1. Jakub Głuszkiewicz

      Ben,

      this PR can not be merged at current state, it will not work due to huge refactor and migration from webkit to blink in newest chrome builds…

      luckily Kaustubh Vats posted new path above which potentially works with newest cef branch, im currently evaluating his path to see if its working,

      about dropdowns, there is very simple workaround, we have touch working with dropdowns,

      the trick is to emulate touch via mouseclick events when dropdown is open,

      you just need to ignore touch event when dropdown is open and send instead of touchdown, mousedown event to cef,

      it will work cuz windows touchdown generating mousedown events at the same time.

      only limitation of this workaround is that you will not have touchscroll (swipe scroll) in dropdowns, your end user need to use scrollbars, but everything else is working,

      so solution is acceptable even in commercial apps,

        1. Jakub Głuszkiewicz

          Its not problem at all, your cefclient class should derive from CefRenderHandler,

          CefRenderHandler has method OnPopupShow to override and this method will give you everything you need,

          look at CefRenderHandler header:

          //
          // Called when the browser wants to show or hide the popup widget. The popup
          // should be shown if \\\|show\\\| is true and hidden if \\\|show\\\| is false.
          ///
          /--cef()--/
          virtual void OnPopupShow(CefRefPtr<CefBrowser> browser, bool show) {}

          and dropdown is just a popup 😉

  16. Jakub Głuszkiewicz

    I builded branch 3396 with Kaustubh Vats path posted above,

    compilation without any problems, but sadly touch doesnt work correctly any more.

    I mean, touch work in general, but touch_up events are not registered correctly, so touch is completly broken 😞

    So still this PR is compatible only with branch 3325 at most, later cef versions will not work,

    If someone would help me to fix Kaustubh Vats path or give me advice where to dig, i would start to fix touch_up issues,

    thanks

  17. Cristian Carli

    I have tryed to implement your solution to fix the dropdown bug but I haven’t been able to understand which files has to be modified.

    Can you tell me in details the name of the file that has to be modified ?

    1. Jakub Głuszkiewicz

      You dont have to modify any CEF files, just build with this path,

      In your cefclient implementation you have to detect when drop down is open, there are methods to easly detect if popup is open or not,

      if popup is open just send mouse events instead of touch events to have a working dropdowns via touch,

  18. Jakub Głuszkiewicz

    Just spent a week trying to build this path with newest branch 3626. I was able to build motion_event_osr.cc and .h but failed in render_widghet_host with 5 build errors,

    ../../cef/libcef/browser/osr/render_widget_host_view_osr.cc(187,4): error: too many arguments to function call, expected 3, have 5
    base::TimeTicks() + base::TimeDelta::FromMicroseconds(time_micros), 1);
    ^~~~~~~~~~~~~~~
    ../..\ui/latency/latency_info.h(161,3): note: 'AddLatencyNumberWithTimestamp' declared here
    void AddLatencyNumberWithTimestamp(LatencyComponentType component,
    ^
    ../../cef/libcef/browser/osr/render_widget_host_view_osr.cc(1454,3): error: static_assert failed due to requirement 'base::trace_event::BuiltinCategories::IsAllowedCategory("libcef")' "Unknown tracing category is used. Please register your category in base/trace_event/builtin_categories.h"
    TRACE_EVENT0("libcef", "CefRenderWidgetHostViewOSR:TranslateTouchEvent");
    ^
    ~~~~~~~~~~~~~~~
    ../..\base/trace_event/common/trace_event_common.h(211,3): note: expanded from macro 'TRACE_EVENT0'
    INTERNAL_TRACE_EVENT_ADD_SCOPED(category_group, name)
    ^
    ~~~~~~~~~~
    ../..\base/trace_event/trace_event.h(300,3): note: expanded from macro 'INTERNAL_TRACE_EVENT_ADD_SCOPED'
    INTERNAL_TRACE_EVENT_GET_CATEGORY_INFO(category_group); \
    ^~~~~~~~~~~~
    ../..\base/trace_event/trace_event.h(260,3): note: expanded from macro 'INTERNAL_TRACE_EVENT_GET_CATEGORY_INFO'
    static_assert( \
    ^
    ../../cef/libcef/browser/osr/render_widget_host_view_osr.cc(1478,3): error: static_assert failed due to requirement 'base::trace_event::BuiltinCategories::IsAllowedCategory("libcef")' "Unknown tracing category is used. Please register your category in base/trace_event/builtin_categories.h"
    TRACE_EVENT0("libcef", "CefRenderWidgetHostViewOSR::SendTouchEvent");
    ^
    ~~~~~~~~~~~~~
    ../..\base/trace_event/common/trace_event_common.h(211,3): note: expanded from macro 'TRACE_EVENT0'
    INTERNAL_TRACE_EVENT_ADD_SCOPED(category_group, name)
    ^~~~~~~~~~~
    ../..\base/trace_event/trace_event.h(300,3): note: expanded from macro 'INTERNAL_TRACE_EVENT_ADD_SCOPED'
    INTERNAL_TRACE_EVENT_GET_CATEGORY_INFO(category_group); \
    ^
    ~~~~~~~~~~~

    i think im going to give up, its far beyound my knowledge about cef and chrommium,

    im very sad that this path was never merged into master, and original contribitors doesnt want to update it. Ofc, i know its not a job, its not even not paid job, so i understand and i have no expectations,

    I wanted to do it by my own but sadly im not good enough.

    Pls can we try to bring alive this path again? Im really ready to help as i can.

  19. Riku Palomäki

    I guess this PR is dead, so I think I’ll start working on a new one based on this, unless someone has already done something? It seems that the fling gesture doesn’t work at all, touch on popup widgets don’t work and scrolling pdf files is also broken. I’ll try to address these issues.

    1. Jakub Głuszkiewicz

      Riku, could you write me short message on my email address jakub.gluszkiewicz@nanovotech.pl that i can reply? Im very happy that you are interested in bringing this patch alive again. We are strongly interested in this patch and we can consider donation for you to keep you motivated. Please just write to me.

      Thanks

    2. Tammo Hinrichs author

      Yeah, it’s pretty much dead; I sadly don’t have the time to keep it updated and add the extra work to get it accepted into mainline 😕

      I’ve got a version of the PR that compiles against 3526 and runs (minus the bugs you described) if you want a better starting point but that one ignored everything that went on here in the comment section, and it doesn’t include CefClient usage. Yet if you’re interested just shout.

      Otherwise I think we’d all appreciate if you went ahead. :)

      1. Riku Palomäki

        I already started working on this: https://bitbucket.org/riku_palomaki/cef/branch/touch - I will open a new PR once it’s done.

        This now compiles against the current master (3683), and I fixed touch on popup widgets and fling gesture. I do have a hack for PDF touch as well, but I didn’t commit that since it still behaves badly on some situations and requires Chromium change, but I’ll continue working on that next week.

        So far I have been working just on Linux, I don’t know if the code even compiles on other platforms, I will test them later.

        I noticed that cefclient uses GTK2 which doesn’t seem to have touch events at all, so cefclient won’t have support for this in Linux, unless we (conditionally?) update to GTK3, but that’s probably too much work.

        Are there any other known issues?

        1. Denis Declara

          Thank you for the work you have put in! I was also on the verge of starting to fix this PM myself! I can try compiling and testing your branch on Windows first thing on Monday, and let you know how it goes. 🙂

          1. Denis Declara

            Riku Palomäki I have just built your branch in Windows and have tested the cefclient.exe app with --off-screen-rendering-enabled. Touch events seem to work fine on windows. I will try to do more thorough tests in the course of the week.

        2. Tammo Hinrichs author

          Just checked all the comments above and in the code, and tried to remember what I already did… the “Cancel all pending touches” function described above would probably a nice addition ( https://bitbucket.org/chromiumembedded/cef/pull-requests/104/touch-processing-for-osr-different-api/diff#comment-47659002 ) , there’s one failed test that Marshall found on high DPI screens ( https://bitbucket.org/chromiumembedded/cef/pull-requests/104/touch-processing-for-osr-different-api/diff#comment-45724591 ) , and there’s seemingly one unsolved problem regarding pinch zoom below in the code ( https://bitbucket.org/chromiumembedded/cef/pull-requests/104/touch-processing-for-osr-different-api/diff#comment-32106544 ). Forgot what exactly this was about tho, sorry…

          Apart from that let’s hope that the Windows client implementation is enough to let Marshall get the new PR through this time… 🙂

          1. Riku Palomäki

            Ok, thank you for this list. I should have time next week to look into these issues.

            Maybe we can use raw xcb / x11 events to implement touch on cefclient on Linux instead of updating to GTK3.

  20. Jakub Głuszkiewicz

    Riku,

    great job man, i just merged your changes into master. Touch is working again yay!! Woohooo xD

    Thanks for your time and effort. More over, now flinger gesture is working correctly finally! Just two lines of code did a miracles!

    I didnt test popups yet because i have huge stability issues on master, but i strongly doubt it has anything with your changes. I messed something during merging probably, hardware acceleration is not working and i see many crashes without hw accel. But i strongly doubt it has anything with your changes.

    After such successfull test , i will start to port your changes into branch 3626 (chrome 72), cuz im considering that branch now as a stable one.

    After that we will start some serious tests on windows platform and i will report back here about results,

    thank you!

  21. Riku Palomäki

    I fixed the issue with PDF touches and added support for touch to cefclient on Linux using just raw X11 / XInput events.

    I wasn’t able to reproduce this “cancel all pending touches” issue. There is already some code to cancel touch events on certain situations, and it matches pretty closely to what Aura is doing. It would be great if someone would be able to reproduce this with cefclient, but maybe the issue has just disappeared with this version?

    For me --disable-pinch seems to work as intended, I didn't need to modify anything for that.

    I think I’ll now move on to Windows and fix that high-dpi screen issue next.

    I have also noticed some stability issues Jakub mentioned. DCHECK_LE(presentation_callbacks_.size(), 25u); in LayerTreeView::AddPresentationCallbackseems to trigger all the time, not sure if it has anything to do with touch. I’m first going to get this touch stuff working on all platforms and then analyze the stability issues more.

  22. Jakub Głuszkiewicz

    I spend last few days testing touch thanks to Riku code, so here are the first results:

    Branch 3578 (Chrome 71) cuz im considering this branch as most stable one for now

    Machine: Win10 Pro, CPU i7, tested on GPU Intel/Nvidia

    Build succeeded and touch is working flawlessly.

    Flinger gesture works correctly in OSR standard mode, but it DOESNT work in shared textures mode,

    I managed to fix it by changing in render_widget_host_osr.cc SendExternalBeginFrame() method but im not sure if its the best way to do it,

    void CefRenderWidgetHostViewOSR::SendExternalBeginFrame()
    
      {DCHECK(external_begin_frame_enabled_);
    
      base::TimeTicks frame_time = base::TimeTicks::Now();
    
      base::TimeTicks deadline = base::TimeTicks();
    
      base::TimeDelta interval = viz::BeginFrameArgs::DefaultInterval();
    
      viz::BeginFrameArgs begin_frame_args = viz::BeginFrameArgs::Create(
        BEGINFRAME_FROM_HERE, begin_frame_source_.source_id(),begin_frame_number_, frame_time, deadline, interval,viz::BeginFrameArgs::NORMAL);
    
      DCHECK(begin_frame_args.IsValid());
    
      begin_frame_number_++;
    
      if (renderer_compositor_frame_sink_) {
        GetCompositor()->IssueExternalBeginFrame(begin_frame_args);
    
      renderer_compositor_frame_sink_->OnBeginFrame(begin_frame_args);}
    
      //add this two lines for flinger gesture in shared texture mode
      if (render_widget_host_)
        render_widget_host_->ProgressFlingIfNeeded(frame_time);
    
      if (!IsPopupWidget() && popup_host_view_) {
        popup_host_view_->SendExternalBeginFrame();
      }
    }
    

    Popups are working great now!

    PDF plugin is working great!

    Only one issue i detected is that moving gesture is not recognized in webgl applications.

    For example: http://fishgl.com try it and you will see that you cant rotate tank. In Chrome 65 with this PR it has been working correctly. So im sure its possible to fix. But sadly so far have no idea.

    I will keep reporting..

    thank you

    1. Riku Palomäki

      I don’t think this fishgl.com issue is related to this PR at all. It doesn’t work with the latest official stable desktop Google Chrome on both Linux and Windows. I think the page is just broken.

      I will look into this shared texture mode issue.

    2. Riku Palomäki

      For me fling gesture works just fine in Windows with cefclient.exe --off-screen-rendering-enabled --touch-events=enabled --shared-texture-enabled --url=https://en.wikipedia.org/wiki/World_War_I. I think the issue you are having only applies to your branch 3578, I’m now only working against master to get this merged.

      Also the unit tests work just fine with hidpi-screen on Windows, so I don’t have any more issues remaining. I’ll now open a new PR.