Skip to content

Commit

Permalink
Recreate DirectManipulationHelper when every LRWHH UpdateParent
Browse files Browse the repository at this point in the history 
This is a reland of 7da0707 and
d77c502

Compositor and window event target is associated with window's parent.
We call LRWHH UpdateParent when window's parent update, includes
window's parent actually update and window initialize. Recreate
DirectManipulationHelper on every window's parent update helps keep
DirectManipulationHelper lifecycle tracking simpler. We also make
CompositorAnimationObserver owned by DirectManipulationHelper.

With this changes, we start the DirectManipulation event polling when
DirectManipulationHelper created and stop when it destroyed. The issue
should be fix since event polling start no more depends on
DM_POINTERHITTEST.

This CL also includes 3 refactoring changes:

1. Move CompositorAnimationObserver into DirectManipulationHelper.
2. Call ZoomToRect to reset viewport of DirectManipulation when
   viewport is actually transformed in RUNNING - READAY sequence.
3. Pass the viewport size to DMEventHandler and use it to reset viewport
   at gesture end.

The original changes caused a regression that browser UI composition
keeps ticking begin frames. We register DirectManipulationHelperWin
as an AnimationObserver of ui::Compositor. UI compositor will ask for
begin frames as long as it has an AnimationObserver. We should call
OnCompositorShuttingDown() when the compositor is going Idle. Therefore,
I added a IDirectManipulationInteractionEventHandler that adds the
observer when a manipulation begins, and removes it when manipulation
ends. After my fix, we start the DirectManipulation event polling when
DirectManipulation interaction begin and stop when DirectManipulation
interaction end.

Bug: 914914
Change-Id: I9f59381bdcc6e4ed0970003d87b26ac750bfb42d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1689922
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Siye Liu <siliu@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#686111}
  • Loading branch information
@siliu1
siliu1 authored and Commit Bot committed Aug 12, 2019
1 parent 8f86965 commit 2d5f16f
Show file tree
Hide file tree
Showing 8 changed files with 198 additions and 286 deletions.
60 changes: 40 additions & 20 deletions content/browser/renderer_host/direct_manipulation_event_handler_win.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,19 +28,16 @@ bool FloatEquals(float f1, float f2) {
} // namespace

DirectManipulationEventHandler::DirectManipulationEventHandler(
DirectManipulationHelper* helper)
: helper_(helper) {}

void DirectManipulationEventHandler::SetWindowEventTarget(
ui::WindowEventTarget* event_target) {
if (!event_target && LoggingEnabled()) {
DebugLogging("Event target is null.", S_OK);
if (event_target_)
DebugLogging("Previous event target is not null", S_OK);
else
DebugLogging("Previous event target is null", S_OK);
}
event_target_ = event_target;
DirectManipulationHelper* helper,
ui::WindowEventTarget* event_target)
: helper_(helper), event_target_(event_target) {}

bool DirectManipulationEventHandler::SetViewportSizeInPixels(
const gfx::Size& viewport_size_in_pixels) {
if (viewport_size_in_pixels_ == viewport_size_in_pixels)
return false;
viewport_size_in_pixels_ = viewport_size_in_pixels;
return true;
}

void DirectManipulationEventHandler::SetDeviceScaleFactor(
Expand Down Expand Up @@ -175,19 +172,28 @@ HRESULT DirectManipulationEventHandler::OnViewportStatusChanged(
if (current != DIRECTMANIPULATION_READY)
return S_OK;

// Reset the viewport when we're idle, so the content transforms always start
// at identity.
// Every animation will receive 2 ready message, we should stop request
// compositor animation at the second ready.
first_ready_ = !first_ready_;
HRESULT hr = helper_->Reset(first_ready_);
// Normally gesture sequence will receive 2 READY message, the first one is
// gesture end, the second one is from viewport reset. We don't have content
// transform in the second RUNNING -> READY. We should not reset on an empty
// RUNNING -> READY sequence.
if (last_scale_ != 1.0f || last_x_offset_ != 0 || last_y_offset_ != 0) {
HRESULT hr = viewport->ZoomToRect(
static_cast<float>(0), static_cast<float>(0),
static_cast<float>(viewport_size_in_pixels_.width()),
static_cast<float>(viewport_size_in_pixels_.height()), FALSE);
if (!SUCCEEDED(hr)) {
DebugLogging("Viewport zoom to rect failed.", hr);
return hr;
}
}

last_scale_ = 1.0f;
last_x_offset_ = 0.0f;
last_y_offset_ = 0.0f;

TransitionToState(GestureState::kNone);

return hr;
return S_OK;
}

HRESULT DirectManipulationEventHandler::OnViewportUpdated(
Expand Down Expand Up @@ -294,4 +300,18 @@ HRESULT DirectManipulationEventHandler::OnContentUpdated(
return hr;
}

HRESULT DirectManipulationEventHandler::OnInteraction(
IDirectManipulationViewport2* viewport,
DIRECTMANIPULATION_INTERACTION_TYPE interaction) {
if (interaction == DIRECTMANIPULATION_INTERACTION_BEGIN) {
DebugLogging("OnInteraction BEGIN.", S_OK);
helper_->AddAnimationObserver();
} else if (interaction == DIRECTMANIPULATION_INTERACTION_END) {
DebugLogging("OnInteraction END.", S_OK);
helper_->RemoveAnimationObserver();
}

return S_OK;
}

} // namespace content
21 changes: 15 additions & 6 deletions content/browser/renderer_host/direct_manipulation_event_handler_win.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include <wrl.h>

#include "base/macros.h"
#include "ui/gfx/geometry/size.h"

namespace ui {

Expand All @@ -21,6 +22,7 @@ class WindowEventTarget;
namespace content {

class DirectManipulationHelper;
class DirectManipulationBrowserTest;
class DirectManipulationUnitTest;

// DirectManipulationEventHandler receives status update and gesture events from
Expand All @@ -33,17 +35,19 @@ class DirectManipulationEventHandler
Microsoft::WRL::RuntimeClassFlags<
Microsoft::WRL::RuntimeClassType::ClassicCom>,
Microsoft::WRL::FtmBase,
IDirectManipulationViewportEventHandler>> {
IDirectManipulationViewportEventHandler,
IDirectManipulationInteractionEventHandler>> {
public:
explicit DirectManipulationEventHandler(DirectManipulationHelper* helper);
DirectManipulationEventHandler(DirectManipulationHelper* helper,
ui::WindowEventTarget* event_target);

// WindowEventTarget updates for every DM_POINTERHITTEST in case window
// hierarchy changed.
void SetWindowEventTarget(ui::WindowEventTarget* event_target);
// Return true if viewport_size_in_pixels_ changed.
bool SetViewportSizeInPixels(const gfx::Size& viewport_size_in_pixels);

void SetDeviceScaleFactor(float device_scale_factor);

private:
friend class DirectManipulationBrowserTest;
friend DirectManipulationUnitTest;

// DirectManipulationEventHandler();
Expand All @@ -65,18 +69,23 @@ class DirectManipulationEventHandler
OnContentUpdated(_In_ IDirectManipulationViewport* viewport,
_In_ IDirectManipulationContent* content) override;

HRESULT STDMETHODCALLTYPE
OnInteraction(_In_ IDirectManipulationViewport2* viewport,
_In_ DIRECTMANIPULATION_INTERACTION_TYPE interaction) override;

DirectManipulationHelper* helper_ = nullptr;
ui::WindowEventTarget* event_target_ = nullptr;
float device_scale_factor_ = 1.0f;
float last_scale_ = 1.0f;
int last_x_offset_ = 0;
int last_y_offset_ = 0;
bool first_ready_ = false;
bool should_send_scroll_begin_ = false;

// Current recognized gesture from Direct Manipulation.
GestureState gesture_state_ = GestureState::kNone;

gfx::Size viewport_size_in_pixels_;

DISALLOW_COPY_AND_ASSIGN(DirectManipulationEventHandler);
};

Expand Down
145 changes: 72 additions & 73 deletions content/browser/renderer_host/direct_manipulation_helper_win.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
#include "base/win/windows_version.h"
#include "ui/base/ui_base_features.h"
#include "ui/base/win/window_event_target.h"
#include "ui/compositor/compositor.h"
#include "ui/compositor/compositor_animation_observer.h"
#include "ui/display/win/screen_win.h"
#include "ui/gfx/geometry/rect.h"

Expand All @@ -39,8 +41,9 @@ void DebugLogging(const std::string& s, HRESULT hr) {
// static
std::unique_ptr<DirectManipulationHelper>
DirectManipulationHelper::CreateInstance(HWND window,
ui::Compositor* compositor,
ui::WindowEventTarget* event_target) {
if (!::IsWindow(window))
if (!::IsWindow(window) || !compositor || !event_target)
return nullptr;

if (!base::FeatureList::IsEnabled(features::kPrecisionTouchpad))
Expand All @@ -51,8 +54,7 @@ DirectManipulationHelper::CreateInstance(HWND window,
return nullptr;

std::unique_ptr<DirectManipulationHelper> instance =
base::WrapUnique(new DirectManipulationHelper());
instance->window_ = window;
base::WrapUnique(new DirectManipulationHelper(window, compositor));

if (instance->Initialize(event_target))
return instance;
Expand All @@ -73,23 +75,37 @@ DirectManipulationHelper::CreateInstanceForTesting(
return nullptr;

std::unique_ptr<DirectManipulationHelper> instance =
base::WrapUnique(new DirectManipulationHelper());
base::WrapUnique(new DirectManipulationHelper(0, nullptr));

instance->event_handler_ =
Microsoft::WRL::Make<DirectManipulationEventHandler>(instance.get());
instance->event_handler_->SetWindowEventTarget(event_target);
Microsoft::WRL::Make<DirectManipulationEventHandler>(instance.get(),
event_target);

instance->viewport_ = viewport;

return instance;
}

DirectManipulationHelper::~DirectManipulationHelper() {
if (viewport_)
viewport_->Abandon();
Destroy();
}

DirectManipulationHelper::DirectManipulationHelper() {}
DirectManipulationHelper::DirectManipulationHelper(HWND window,
ui::Compositor* compositor)
: window_(window), compositor_(compositor) {}

void DirectManipulationHelper::OnAnimationStep(base::TimeTicks timestamp) {
// Simulate 1 frame in update_manager_.
HRESULT hr = update_manager_->Update(nullptr);
if (!SUCCEEDED(hr))
DebugLogging("UpdateManager update failed.", hr);
}

void DirectManipulationHelper::OnCompositingShuttingDown(
ui::Compositor* compositor) {
DCHECK_EQ(compositor, compositor_);
Destroy();
}

bool DirectManipulationHelper::Initialize(ui::WindowEventTarget* event_target) {
// IDirectManipulationUpdateManager is the first COM object created by the
Expand Down Expand Up @@ -142,8 +158,8 @@ bool DirectManipulationHelper::Initialize(ui::WindowEventTarget* event_target) {
return false;
}

event_handler_ = Microsoft::WRL::Make<DirectManipulationEventHandler>(this);
event_handler_->SetWindowEventTarget(event_target);
event_handler_ =
Microsoft::WRL::Make<DirectManipulationEventHandler>(this, event_target);

// We got Direct Manipulation transform from
// IDirectManipulationViewportEventHandler.
Expand All @@ -155,8 +171,9 @@ bool DirectManipulationHelper::Initialize(ui::WindowEventTarget* event_target) {
}

// Set default rect for viewport before activate.
viewport_size_in_pixels_ = {1000, 1000};
RECT rect = gfx::Rect(viewport_size_in_pixels_).ToRECT();
gfx::Size viewport_size_in_pixels = {1000, 1000};
event_handler_->SetViewportSizeInPixels(viewport_size_in_pixels);
RECT rect = gfx::Rect(viewport_size_in_pixels).ToRECT();
hr = viewport_->SetViewportRect(&rect);
if (!SUCCEEDED(hr)) {
DebugLogging("Viewport set rect failed.", hr);
Expand Down Expand Up @@ -185,33 +202,9 @@ bool DirectManipulationHelper::Initialize(ui::WindowEventTarget* event_target) {
return true;
}

void DirectManipulationHelper::Activate() {
HRESULT hr = viewport_->Stop();
if (!SUCCEEDED(hr)) {
DebugLogging("Viewport stop failed.", hr);
return;
}

hr = manager_->Activate(window_);
if (!SUCCEEDED(hr))
DebugLogging("DirectManipulationManager activate failed.", hr);
}

void DirectManipulationHelper::Deactivate() {
HRESULT hr = viewport_->Stop();
if (!SUCCEEDED(hr)) {
DebugLogging("Viewport stop failed.", hr);
return;
}

hr = manager_->Deactivate(window_);
if (!SUCCEEDED(hr))
DebugLogging("DirectManipulationManager deactivate failed.", hr);
}

void DirectManipulationHelper::SetSizeInPixels(
const gfx::Size& size_in_pixels) {
if (viewport_size_in_pixels_ == size_in_pixels)
if (!event_handler_->SetViewportSizeInPixels(size_in_pixels))
return;

HRESULT hr = viewport_->Stop();
Expand All @@ -220,16 +213,13 @@ void DirectManipulationHelper::SetSizeInPixels(
return;
}

viewport_size_in_pixels_ = size_in_pixels;
RECT rect = gfx::Rect(viewport_size_in_pixels_).ToRECT();
RECT rect = gfx::Rect(size_in_pixels).ToRECT();
hr = viewport_->SetViewportRect(&rect);
if (!SUCCEEDED(hr))
DebugLogging("Viewport set rect failed.", hr);
}

bool DirectManipulationHelper::OnPointerHitTest(
WPARAM w_param,
ui::WindowEventTarget* event_target) {
void DirectManipulationHelper::OnPointerHitTest(WPARAM w_param) {
// Update the device scale factor.
event_handler_->SetDeviceScaleFactor(
display::win::ScreenWin::GetScaleFactorForHWND(window_));
Expand All @@ -240,53 +230,62 @@ bool DirectManipulationHelper::OnPointerHitTest(
// For WM_POINTER, the pointer type will show the event from mouse.
// For WM_POINTERACTIVATE, the pointer id will be different with the following
// message.
event_handler_->SetWindowEventTarget(event_target);

using GetPointerTypeFn = BOOL(WINAPI*)(UINT32, POINTER_INPUT_TYPE*);
UINT32 pointer_id = GET_POINTERID_WPARAM(w_param);
POINTER_INPUT_TYPE pointer_type;
static const auto get_pointer_type = reinterpret_cast<GetPointerTypeFn>(
base::win::GetUser32FunctionPointer("GetPointerType"));
if (get_pointer_type && get_pointer_type(pointer_id, &pointer_type) &&
pointer_type == PT_TOUCHPAD && event_target) {
pointer_type == PT_TOUCHPAD) {
HRESULT hr = viewport_->SetContact(pointer_id);
if (!SUCCEEDED(hr)) {
if (!SUCCEEDED(hr))
DebugLogging("Viewport set contact failed.", hr);
return false;
}

// Request begin frame for fake viewport.
need_poll_events_ = true;
}
return need_poll_events_;
}

HRESULT DirectManipulationHelper::Reset(bool need_poll_events) {
// By zooming the primary content to a rect that match the viewport rect, we
// reset the content's transform to identity.
HRESULT hr = viewport_->ZoomToRect(
static_cast<float>(0), static_cast<float>(0),
static_cast<float>(viewport_size_in_pixels_.width()),
static_cast<float>(viewport_size_in_pixels_.height()), FALSE);
if (!SUCCEEDED(hr)) {
DebugLogging("Viewport zoom to rect failed.", hr);
return hr;
}

need_poll_events_ = need_poll_events;
return S_OK;
void DirectManipulationHelper::AddAnimationObserver() {
DCHECK(compositor_);
compositor_->AddAnimationObserver(this);
has_animation_observer_ = true;
}

bool DirectManipulationHelper::PollForNextEvent() {
// Simulate 1 frame in update_manager_.
HRESULT hr = update_manager_->Update(nullptr);
if (!SUCCEEDED(hr))
DebugLogging("UpdateManager update failed.", hr);
return need_poll_events_;
void DirectManipulationHelper::RemoveAnimationObserver() {
DCHECK(compositor_);
compositor_->RemoveAnimationObserver(this);
has_animation_observer_ = false;
}

void DirectManipulationHelper::SetDeviceScaleFactorForTesting(float factor) {
event_handler_->SetDeviceScaleFactor(factor);
}

void DirectManipulationHelper::Destroy() {
if (!compositor_)
return;
if (has_animation_observer_)
RemoveAnimationObserver();
compositor_ = nullptr;

HRESULT hr;
if (viewport_) {
hr = viewport_->Stop();
if (!SUCCEEDED(hr))
DebugLogging("Viewport stop failed.", hr);

hr = viewport_->RemoveEventHandler(view_port_handler_cookie_);
if (!SUCCEEDED(hr))
DebugLogging("Viewport remove event handler failed.", hr);

hr = viewport_->Abandon();
if (!SUCCEEDED(hr))
DebugLogging("Viewport abandon failed.", hr);
}

if (manager_) {
hr = manager_->Deactivate(window_);
if (!SUCCEEDED(hr))
DebugLogging("DirectManipulationManager deactivate failed.", hr);
}
}

} // namespace content

0 comments on commit 2d5f16f

Please sign in to comment.