Open GoogleCodeExporter opened 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
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
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
@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
[deleted comment]
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
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:
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
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
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
@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
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
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:
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
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
#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
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
@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
Original comment by magreenb...@gmail.com
on 23 Oct 2014 at 5:28
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
Original issue reported on code.google.com by
jf.ver...@mgdesign.fr
on 29 Aug 2013 at 12:13