roadlabs / chromiumembedded

Automatically exported from code.google.com/p/chromiumembedded
0 stars 1 forks source link

Cef3: Multi-touch support with offscreen rendering. #1059

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
Functions to inject touchs events when rendering browser in offscreen mode 
would be very usefull.

Event data proposal:
typedef struct _cef_touch_event_t {
  ///
  // X coordinate relative to the left side of the view.
  ///
  int x;

  ///
  // Y coordinate relative to the top side of the view.
  ///
  int y;

  ///
  // A touch point identifier that distinguishes a particular touch input.
  // This value stays consistent in a touch contact sequence from the point a contact comes down until it comes back up.
  // An id may be reused later for subsequent contacts.
  ///
  int id;

  ///
  // Bit flags describing any pressed modifier keys. See
  // cef_event_flags_t for values.
  ///
  uint32 modifiers;
} cef_touch_event_t;

Functions signatures proposal ('CefBrowser' class members):
virtual void SendTouchClickEvent(const CefTouchEvent& event, bool touchUp)=0;
virtual void SendTouchMoveEvent(const CefTouchEvent& event)=0;

Identifieds changes to support this functionality (non exhaustive):
* Add '_cef_touch_event_t' struct in 'cef_types.h'
* Add 'CefTouchEventTraits' class in 'cef_types_wrappers.h'
* Add 'CefTouchEvent' typedef in 'cef_types_wrappers.h'
* Add 'SendTouchClickEvent' and 'SendTouchMoveEvent' functions to 
'cef_browser.h', 'browser_host_impl.h' and 'browser_host_impl.cc' 
* Add 'PlatformTranslateTouchEvent' (and eventually 
'PlatformTranslateTouchClickEvent' and 'PlatformTranslateTouchMoveEvent') in 
'browser_host_impl'
* Provide implementation for 'PlatformTranslateTouchEvent' in 
'browser_host_impl_win.cc', 'browser_host_impl_mac.mm' and 
'browser_host_impl_gtk.cc'
* Add 'SendTouchEvent' in 'render_widget_host_view_osr.cc'

Original issue reported on code.google.com by jf.ver...@mgdesign.fr on 29 Aug 2013 at 12:13

GoogleCodeExporter commented 9 years ago
Sounds reasonable. Patches welcome. Please also add unit tests in 
os_rendering_unittest.cc and support in cefclient when running in OSR mode.

Original comment by magreenb...@gmail.com on 3 Sep 2013 at 5:52

GoogleCodeExporter commented 9 years ago
I'm considering implementing this, but I am only windows literate when it comes 
to multitouch input handling.  The suggestion here for the _cef_touch_event_t 
layout doesn't seems quite right to me for a multitouch system.  The suggested 
structure layout looks more like a single "touch point" (TOUCHINPUT in Windows, 
WebTouchPoint in Webkit, see 
src/third_party/WebKit/public/web/WebInputEvent.h).  In windows and apparently 
in webkit, a "touch event" contains one or more "touch points", so both Windows 
and Webkit appear to bunch individual touchpoints into a single TouchEvent.

I don't know if the suggested structure layout and naming has some other 
justification that I don't know about, or if its just a design starting point.

Original comment by johnjose...@gmail.com on 4 Oct 2013 at 7:18

GoogleCodeExporter commented 9 years ago
The windows specific code for handling and translating multitouch input lives 
in the chromium code base in 
src/content/browser/renderer_host/render_widget_host_view_win.cc.  I'm guessing 
it makes sense to use that code as a starting point.

I don't see how the mac or gtk code handles raw multitouch.  Are there 
significant limitations on those platforms, or does the OS in those cases take 
care of "cooking" multitouch into ready-made gestures?

Original comment by johnjose...@gmail.com on 4 Oct 2013 at 7:30

GoogleCodeExporter commented 9 years ago
@johnjose: "I don't know if the suggested structure layout and naming has some 
other justification that I don't know about, or if its just a design starting 
point."
It was starting point thoughts, but indeed, your proposal is better. From my 
small experience on mac, the system can provide raw touch information, the same 
way (or almost) Windows does.
Did you start to implement something concerning this functionnality?

Original comment by jf.ver...@mgdesign.fr on 11 Dec 2013 at 2:24

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
Hi,
I have a working implementation of OSR touch events (Windows only), based on 
branch 1650:
https://bitbucket.org/MGD_Jeff/branches-1650-cef3
Feel free to test it and comment.

Original comment by jf.ver...@mgdesign.fr on 20 Dec 2013 at 12:18

GoogleCodeExporter commented 9 years ago
Hi,

Here commes the patch. Things that need to be done in order to validate it:
 - test compilation on other platforms (linux/osx). I added some #def for Windows related code but not sure they are all placed well.
 - test compilation on Windows XP. No problems for LibCef I think, but some function added in CefClient sample are Win7+ only. Dynamic linking these function could make the CefClient executable XP compliant.
 - Handle (or delete?) the small code part with "TODO JEFF" in it: I do not know if touch injection outside OSR mode makes sense, and nor how to integrate this easily in Cef...

This patch applies on branch 1650.

Original comment by jf.ver...@mgdesign.fr on 8 Jan 2014 at 4:07

Attachments:

GoogleCodeExporter commented 9 years ago
jf.ver...@mgdesign.fr,

I apologize, I didn't see that you had asked me a question until just now.

To answer your question, no I haven't started to implement anything so far.  I 
had to put my CEF work down for a little while, but I will need to pick it up 
again in a week or two.  If expect I'll take your patch for a test drive if 
that's OK.

Original comment by johnjose...@gmail.com on 10 Jan 2014 at 7:16

GoogleCodeExporter commented 9 years ago
I applied the patches to my local cef build and have been testing my app with 
"off screen rendering" for a few hours.  This is strictly Windows 7 testing.  I 
have two multitouch monitors for testing, one a two touch monitor, the other a 
10 touch.  No problems so far.  

I've used two publically available web site for multitouch smoke testing:

http://blogs.msdn.com/b/ie/archive/2011/10/19/handling-multi-touch-and-mouse-inp
ut-in-all-browsers.aspx
http://www.openstreetmap.org

Original comment by johnjose...@gmail.com on 17 Jan 2014 at 11:12

GoogleCodeExporter commented 9 years ago
Of the changes, those made to these files below are straightforward and I don't 
see any issues there:

include/capi/cef_browser_capi.h
include/cef_browser.h
libcef/browser/browser_host_impl.h
libcef_dll/cpptoc/browser_host_cpptoc.cc
libcef_dll/ctocpp/browser_host_ctocpp.cc
libcef_dll/ctocpp/browser_host_ctocpp.h

In the list below, the changes here are mostly to hold the newly defined "touch 
point" structure and a "touch event" which holds a set of "touch points". The 
changes here are mostly fine, but I'm not an expert on multitouch capabilities 
inside the browser or across OSes, so I can see someone tweaking these 
slightly. The struct _cef_touch_event_t uses a hard coded value of 12 to match 
the hard coded value of webkit's WebTouchEvent::touchesLengthCap defined in 
src/third_party/WebKit/public/web/WebInputEvent.h. Also, _cef_touch_event_t 
does not have the equivalent to changedTouches or targetTouches, but that's OK 
AFAIK.

include/internal/cef_types.h
include/internal/cef_types_wrappers.h

The file libcef/browser/browser_host_impl.cc has an open question if there is a 
need for touch injection outside OSR. I don't believe you need to worry about 
the non-OSR case in Windows.  AFAIK, that already worked fine before this patch 
(the WM_TOUCH events arrive through normal means when not using OSR). I can't 
speak to other platforms.

The sample/test program code changes in 
tests/cefclient/cefclient_osr_widget_win.cpp are fairly straightforward. The 
only shortcoming I see is that the contain Windows 7 (or later) API calls, so 
they will cause cefclient.exe to fail to load in Windows OSes that predate 
RegisterTouchWindow, UnregisterTouchWindow, GetTouchInputInfo, and 
CloseTouchInputHandle (like Windows XP for example). Its possible to 
dynamically attach to these entry points in user32.dll with LoadLibrary and 
GetProcAddress, i.e.:

typedef BOOL (WINAPI * PFNREGISTERTOUCHWINDOW)( HWND hWnd, ULONG ulFlags );

g_hDUser32Dll = LoadLibraryW( L"user32.dll" );

if ( g_hDUser32Dll )
{
g_FLASHLOADER_pfnRegisterTouchWindow = (PFNREGISTERTOUCHWINDOW)GetProcAddress( 
g_hDUser32Dll, "RegisterTouchWindow" );

...

That's what I do in my app.

The changes in these files below are the meat the work involved and will take a 
little more effort to get an understanding of the differences:

libcef/browser/render_widget_host_view_osr.cc
libcef/browser/render_widget_host_view_osr.h

Original comment by johnjose...@gmail.com on 17 Jan 2014 at 11:17

GoogleCodeExporter commented 9 years ago
@comment#7: Thanks for the patch. Can you update it to work with the current 
trunk? Some comments:

1. Line 55:

+typedef struct _cef_touch_point_t {
+  ///
+  //
+  ///
+  uint32 id;

Improve the documentation. What is the id and how does the user generate it?

2. Line 81:

+  ///
+  // Event timestamp, in seconds (needed for scrolling inertia for example).
+  ///
+  double timestamp;

Improve the documentation. How should this timestamp be created or formatted?

3. Line 183:

+    // TODO JEFF
+    /*if (widget)
+      widget->SendTouchEvent(event);*/

Supporting usage when windowed rendering is enabled would be consistent with 
the other event types. Does it work?

4. Line 436:

+  // Pinch gestures are disabled by default on windows desktop. See
+  // crbug.com/128477 and crbug.com/148816
+  if (gesture->type() == ui::ET_GESTURE_PINCH_BEGIN ||
+      gesture->type() == ui::ET_GESTURE_PINCH_UPDATE ||
+      gesture->type() == ui::ET_GESTURE_PINCH_END) {
+    return true;
+  }

It seems that these bugs have been resolved with the "--enable-pinch" flag.

5. Add unit tests in os_rendering_unittest.cc.

Also, lots of style errors. You can use the check_style tool to catch most of 
these.

6. Line 101:

+  ///
+  // List of touch points. WebKit supports a maximum of 12 simultaneous touch 
points, see WebInputEvent class in WebTouchEvent.h
+  ///

Wrap lines to 80 characters. Same in other source files.

7. Line 236:

 #include "webkit/common/cursors/webcursor.h"
+#include "ui/base/sequential_id_generator.h"
+#include "ui/events/event.h"
+#include "ui/events/event_utils.h"
+#include "ui/gfx/win/dpi.h"

Include order should be alphabetical.

8. Line 252:

+class CefWebTouchState {
+  public:
+    explicit CefWebTouchState(const CefRenderWidgetHostViewOSR* osrWindow);

Wrong indentation for this class. Some of the methods of this class should also 
be marked 'const'.

9. Line 290:

+#endif // defined(OS_WIN)

Two spaces before //.

10. Line 371:

+  // Copy event because we van modify it internally

Typo and missing period.

11. Line 377:

+    if(render_widget_host_->ShouldForwardTouchEvent()) {

Need a space between if and (.

12. Line 478:

+size_t CefWebTouchState::UpdateTouchPoints(CefTouchEvent& event, size_t offset)
+{

Bracket in the wrong place.

13. Line 598:

+    touch_event_.changedTouches[i].state =
+      WebKit::WebTouchPoint::StateReleased;

Wrong indent. Should be 4 spaces on the continuation line.

Original comment by magreenb...@gmail.com on 11 Feb 2014 at 7:51

GoogleCodeExporter commented 9 years ago
Thanks to both of you for your remarks. Unfortunately, I'm currently in 
"project mode" and do not have time to migrate to trunk (I try to launch 
automate to build a trunk version of Cef as a background task, but the automate 
build seem to be broken at this time...). Guess I will be able to do it soon...
Alright for the documentation, I forgot to describe those 2 vars.
Concerning the commented TODO, I don't know how to do to dispatch the message 
the same way as the mouse event are dispatched, because if I remember well, 
"widget" do not have an injection method for touch.
If the "enable-pinch" bug is corrected in trunk, then obviously this could be 
deleted (although it seem necessary in branch 1650).
And thanks for mentionning check_style tool, I'll know its existence for the 
next time I'll patch something :)

Original comment by jf.ver...@mgdesign.fr on 13 Feb 2014 at 2:23

GoogleCodeExporter commented 9 years ago
Here comes the patch updated for the branch 1750.
As Chromium sources still have the code concerning "ET_GESTURE_PINCH_x" 
(chromium\src\content\browser\renderer_host\render_widget_host_view_win.cc), I 
have choosen to keep this part.
I made a pass concerning style, so it should be better.

Original comment by jf.ver...@mgdesign.fr on 18 Feb 2014 at 1:50

Attachments:

GoogleCodeExporter commented 9 years ago
As CEF 1750 now use Aura, there's big chances that the proposed 1750 patch do 
not reflect exactly the comportement of non-offscreen mode... Unfortunaly, I 
was not aware about the switch to Aura, so I convert the existing 1650 patch 
directly. Porting the patch to 1750 will need more work than I though, as Aura 
input injection seems a little different than the old system.

* As a side note, there's 2 missing lines in 1750 patch, in 
"CefRenderWidgetHostViewOSR.cpp":
 > line 172 (inside CefRenderWidgetHostViewOSR ctor):
+  gesture_recognizer_->AddGestureEventHelper(this);
 > line 182 (inside CefRenderWidgetHostViewOSR dtor):
+  gesture_recognizer_->RemoveGestureEventHelper(this);

* Furthemore, there's another bug on 1650 patch (and so on 1750), still in 
"CefRenderWidgetHostViewOSR.cpp", "UpdateTouchPoints" function:

           if (!(point = AddTouchPoint(event.points[i + offset])))
             continue;

-          last_type = TPT_RELEASED;
-          event.points[i + offset].type = TPT_RELEASED;
+          last_type = TPT_PRESSED;
+          event.points[i + offset].type = TPT_PRESSED;
          touch_event_.type = blink::WebInputEvent::TouchStart;
        }
        break;

Original comment by jf.ver...@mgdesign.fr on 20 Feb 2014 at 3:17

GoogleCodeExporter commented 9 years ago
Is there's plans for converting "CefRenderWidgetHostViewOSR" inputs events 
injection functions to Aura style (make CefRenderWidgetHostViewOSR inherits 
from "aura::WindowDelegate")?

Original comment by jf.ver...@mgdesign.fr on 14 Mar 2014 at 3:25

GoogleCodeExporter commented 9 years ago
#comment#15: Probably not, since Aura is only used on Windows and Linux.

Original comment by magreenb...@gmail.com on 14 Mar 2014 at 3:37

GoogleCodeExporter commented 9 years ago
Ok, that's sad because there's a bug with win8 and cef non-aura (and as far as 
my tests goes, I can say more generally with chromium non-aura). Will compile 
cef 1750 branch without Aura to confirm my diagnostic...

Original comment by jf.ver...@mgdesign.fr on 14 Mar 2014 at 4:18

GoogleCodeExporter commented 9 years ago
@comment#17: 1750+ will never be non-Aura on Windows. The non-Aura code is 
being deleted from Chromium.

Original comment by magreenb...@gmail.com on 14 Mar 2014 at 5:04

GoogleCodeExporter commented 9 years ago

Original comment by magreenb...@gmail.com on 23 Oct 2014 at 5:28

GoogleCodeExporter commented 9 years ago
CEF is transitioning from Google Code to Bitbucket project hosting. If you 
would like to continue receiving notifications on this issue please add 
yourself as a Watcher at the new location: 
https://bitbucket.org/chromiumembedded/cef/issue/1059

Original comment by magreenb...@gmail.com on 14 Mar 2015 at 3:27