Commit 718a6f6f authored by Nick Diego Yamane's avatar Nick Diego Yamane Committed by Commit Bot

x11: Bring back MontionNotify events coalescing

MontionNotify XEvents coalescing logic has been accidentally lost after
the PlatformEvent migration CLs have landed (crbug.com/965991), what
leads to some performance hit/regressions. This patch adds it back as an
attempt to address regressions reported at crbug.com/1051325. Even
though the numbers the report seem quite noisy and unstable, [1] shows
that this was the major cause for such perf regression.

R=thomasanderson@chromium.org

[1] https://pinpoint-dot-chromeperf.appspot.com/job/172c2f5a620000

Bug: 1051325, 965991
Change-Id: Ib5d06867d97633c3593584222571b8d910deb304
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2062098Reviewed-by: default avatarSadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: default avatarThomas Anderson <thomasanderson@chromium.org>
Commit-Queue: Nick Yamane <nickdiego@igalia.com>
Cr-Commit-Position: refs/heads/master@{#742892}
parent 06775374
...@@ -358,68 +358,89 @@ XcursorImage* SkBitmapToXcursorImage(const SkBitmap* cursor_image, ...@@ -358,68 +358,89 @@ XcursorImage* SkBitmapToXcursorImage(const SkBitmap* cursor_image,
} }
int CoalescePendingMotionEvents(const XEvent* xev, XEvent* last_event) { int CoalescePendingMotionEvents(const XEvent* xev, XEvent* last_event) {
XIDeviceEvent* xievent = static_cast<XIDeviceEvent*>(xev->xcookie.data); DCHECK(xev->type == MotionNotify || xev->type == GenericEvent);
int num_coalesced = 0;
XDisplay* display = xev->xany.display; XDisplay* display = xev->xany.display;
int event_type = xev->xgeneric.evtype; XEvent next_event;
bool is_motion = false;
DCHECK(event_type == XI_Motion || event_type == XI_TouchUpdate); int num_coalesced = 0;
while (XPending(display)) {
XEvent next_event;
XPeekEvent(display, &next_event);
// If we can't get the cookie, abort the check.
if (!XGetEventData(next_event.xgeneric.display, &next_event.xcookie))
return num_coalesced;
// If this isn't from a valid device, throw the event away, as if (xev->type == MotionNotify) {
// that's what the message pump would do. Device events come in pairs is_motion = true;
// with one from the master and one from the slave so there will while (XPending(display)) {
// always be at least one pending. XPeekEvent(xev->xany.display, &next_event);
if (!ui::TouchFactory::GetInstance()->ShouldProcessXI2Event(&next_event)) { // Discard all but the most recent motion event that targets the same
XFreeEventData(display, &next_event.xcookie); // window with unchanged state.
XNextEvent(display, &next_event); if (next_event.type == MotionNotify &&
continue; next_event.xmotion.window == xev->xmotion.window &&
next_event.xmotion.subwindow == xev->xmotion.subwindow &&
next_event.xmotion.state == xev->xmotion.state) {
XNextEvent(xev->xany.display, last_event);
} else {
break;
}
} }
} else {
if (next_event.type == GenericEvent && int event_type = xev->xgeneric.evtype;
next_event.xgeneric.evtype == event_type && XIDeviceEvent* xievent = static_cast<XIDeviceEvent*>(xev->xcookie.data);
!ui::DeviceDataManagerX11::GetInstance()->IsCMTGestureEvent( DCHECK(event_type == XI_Motion || event_type == XI_TouchUpdate);
next_event) && is_motion = event_type == XI_Motion;
ui::DeviceDataManagerX11::GetInstance()->GetScrollClassEventDetail(
next_event) == SCROLL_TYPE_NO_SCROLL) { while (XPending(display)) {
XIDeviceEvent* next_xievent = XPeekEvent(display, &next_event);
static_cast<XIDeviceEvent*>(next_event.xcookie.data);
// Confirm that the motion event is targeted at the same window // If we can't get the cookie, abort the check.
// and that no buttons or modifiers have changed. if (!XGetEventData(next_event.xgeneric.display, &next_event.xcookie))
if (xievent->event == next_xievent->event && return num_coalesced;
xievent->child == next_xievent->child &&
xievent->detail == next_xievent->detail && // If this isn't from a valid device, throw the event away, as
xievent->buttons.mask_len == next_xievent->buttons.mask_len && // that's what the message pump would do. Device events come in pairs
(memcmp(xievent->buttons.mask, next_xievent->buttons.mask, // with one from the master and one from the slave so there will
xievent->buttons.mask_len) == 0) && // always be at least one pending.
xievent->mods.base == next_xievent->mods.base && if (!ui::TouchFactory::GetInstance()->ShouldProcessXI2Event(
xievent->mods.latched == next_xievent->mods.latched && &next_event)) {
xievent->mods.locked == next_xievent->mods.locked &&
xievent->mods.effective == next_xievent->mods.effective) {
XFreeEventData(display, &next_event.xcookie); XFreeEventData(display, &next_event.xcookie);
// Free the previous cookie. XNextEvent(display, &next_event);
if (num_coalesced > 0)
XFreeEventData(display, &last_event->xcookie);
// Get the event and its cookie data.
XNextEvent(display, last_event);
XGetEventData(display, &last_event->xcookie);
++num_coalesced;
continue; continue;
} }
if (next_event.type == GenericEvent &&
next_event.xgeneric.evtype == event_type &&
!ui::DeviceDataManagerX11::GetInstance()->IsCMTGestureEvent(
next_event) &&
ui::DeviceDataManagerX11::GetInstance()->GetScrollClassEventDetail(
next_event) == SCROLL_TYPE_NO_SCROLL) {
XIDeviceEvent* next_xievent =
static_cast<XIDeviceEvent*>(next_event.xcookie.data);
// Confirm that the motion event is targeted at the same window
// and that no buttons or modifiers have changed.
if (xievent->event == next_xievent->event &&
xievent->child == next_xievent->child &&
xievent->detail == next_xievent->detail &&
xievent->buttons.mask_len == next_xievent->buttons.mask_len &&
(memcmp(xievent->buttons.mask, next_xievent->buttons.mask,
xievent->buttons.mask_len) == 0) &&
xievent->mods.base == next_xievent->mods.base &&
xievent->mods.latched == next_xievent->mods.latched &&
xievent->mods.locked == next_xievent->mods.locked &&
xievent->mods.effective == next_xievent->mods.effective) {
XFreeEventData(display, &next_event.xcookie);
// Free the previous cookie.
if (num_coalesced > 0)
XFreeEventData(display, &last_event->xcookie);
// Get the event and its cookie data.
XNextEvent(display, last_event);
XGetEventData(display, &last_event->xcookie);
++num_coalesced;
continue;
}
}
// This isn't an event we want so free its cookie data.
XFreeEventData(display, &next_event.xcookie);
break;
} }
// This isn't an event we want so free its cookie data.
XFreeEventData(display, &next_event.xcookie);
break;
} }
if (event_type == XI_Motion && num_coalesced > 0) if (is_motion && num_coalesced > 0)
UMA_HISTOGRAM_COUNTS_10000("Event.CoalescedCount.Mouse", num_coalesced); UMA_HISTOGRAM_COUNTS_10000("Event.CoalescedCount.Mouse", num_coalesced);
return num_coalesced; return num_coalesced;
} }
......
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
#include "ui/events/event_utils.h" #include "ui/events/event_utils.h"
#include "ui/events/keycodes/keyboard_code_conversion_x.h" #include "ui/events/keycodes/keyboard_code_conversion_x.h"
#include "ui/events/pointer_details.h" #include "ui/events/pointer_details.h"
#include "ui/events/types/event_type.h"
#include "ui/events/x/events_x_utils.h" #include "ui/events/x/events_x_utils.h"
#include "ui/gfx/geometry/point.h" #include "ui/gfx/geometry/point.h"
#include "ui/gfx/x/x11.h" #include "ui/gfx/x/x11.h"
......
...@@ -91,12 +91,13 @@ ui::XWindow::Configuration ConvertInitPropertiesToXWindowConfig( ...@@ -91,12 +91,13 @@ ui::XWindow::Configuration ConvertInitPropertiesToXWindowConfig(
// Coalesce touch/mouse events if needed // Coalesce touch/mouse events if needed
bool CoalesceEventsIfNeeded(XEvent* const xev, EventType type, XEvent* out) { bool CoalesceEventsIfNeeded(XEvent* const xev, EventType type, XEvent* out) {
if (xev->type != GenericEvent || if (xev->type == MotionNotify ||
(type != ui::ET_TOUCH_MOVED && type != ui::ET_MOUSE_MOVED && (xev->type == GenericEvent &&
type != ui::ET_MOUSE_DRAGGED)) { (type == ui::ET_TOUCH_MOVED || type == ui::ET_MOUSE_MOVED ||
return false; type == ui::ET_MOUSE_DRAGGED))) {
return ui::CoalescePendingMotionEvents(xev, out) > 0;
} }
return ui::CoalescePendingMotionEvents(xev, out) > 0; return false;
} }
} // namespace } // namespace
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment