Clarify hololens loop

ports/libsimpleservo/api/src/lib.rs

@@ -488,7 +488,8 @@ impl ServoGlue {
                             .host_callbacks
                             .on_allow_navigation(url.to_string());
                         let window_event = WindowEvent::AllowNavigationResponse(pipeline_id, data);
-                        let _ = self.process_event(window_event);
+                        self.events.push(window_event);
+                        let _ = self.perform_updates();
                     }
                 },
                 EmbedderMsg::HistoryChanged(entries, current) => {

support/hololens/ServoApp/BrowserPage.cpp

@@ -22,6 +22,8 @@ namespace winrt::ServoApp::implementation {
 BrowserPage::BrowserPage() {
   log("BrowserPage::BrowserPage()");
   InitializeComponent();
+  InitializeConditionVariable(&mGLCondVar);
+  InitializeCriticalSection(&mGLLock);
   Loaded(std::bind(&BrowserPage::OnPageLoaded, this, _1, _2));
   Window::Current().CoreWindow().VisibilityChanged(
       std::bind(&BrowserPage::OnVisibilityChanged, this, _1, _2));
@@ -31,15 +33,13 @@ void BrowserPage::Shutdown() {
   log("BrowserPage::Shutdown()");
 
   if (mServo != nullptr) {
-    if (!IsLoopRunning()) {
+    if (!mLooping) {
       // FIXME: this should not happen. In that case, we can't send the
       // shutdown event to Servo.
     } else {
-      HANDLE hEvent = ::CreateEventA(nullptr, FALSE, FALSE, sShutdownEvent);
       RunOnGLThread([=] { mServo->RequestShutdown(); });
-      log("Waiting for Servo to shutdown");
+      mLoopTask->wait();
-      ::WaitForSingleObject(hEvent, INFINITE);
+      mLoopTask.reset();
-      StopRenderLoop();
       mServo.reset();
     }
   }
@@ -63,10 +63,10 @@ void BrowserPage::OnVisibilityChanged(CoreWindow const &,
   // stopping the event loop, which we can't recover from yet (see comment in
   // Loop())
 
-  // if (visible && !IsLoopRunning()) {
+  // if (visible && !mLooping) {
   //  StartRenderLoop();
   //}
-  // if (!visible && IsLoopRunning()) {
+  // if (!visible && mLooping) {
   //  StopRenderLoop();
   //}
 }
@@ -92,15 +92,9 @@ void BrowserPage::RecoverFromLostDevice() {
 
 /**** GL THREAD LOOP ****/
 
-bool BrowserPage::IsLoopRunning() {
+void BrowserPage::Loop() {
-  return mLoopTask != nullptr && !mLoopTask->is_done();
-}
-
-void BrowserPage::Loop(cancellation_token cancel) {
   log("BrowserPage::Loop(). GL thread: %i", GetCurrentThreadId());
 
-  HANDLE hEvent = ::CreateEventA(nullptr, FALSE, FALSE, sWakeupEvent);
-
   mOpenGLES.MakeCurrent(mRenderSurface);
 
   EGLint panelWidth = 0;
@@ -118,51 +112,48 @@ void BrowserPage::Loop(cancellation_token cancel) {
     throw winrt::hresult_error(E_FAIL, L"Recovering loop unimplemented");
   }
 
-  // mServo->SetBatchMode(true);
+  mServo->SetBatchMode(true);
-  // FIXME: ^ this should be necessary as we call perform_update
-  // ourself. But enabling batch mode will make clicking a link
-  // not working because during the click, this thread is not
-  // waiting on the hEvent object. See the "wakeup" comment.
 
-  log("Entering loop");
+  while (true) {
-  while (!cancel.is_canceled()) {
+    EnterCriticalSection(&mGLLock);
-    // Block until wakeup is called.
+    while (mTasks.size() == 0 && !mAnimating && mLooping) {
-    // Or run full speed if animating (see OnAnimatingChanged),
+      SleepConditionVariableCS(&mGLCondVar, &mGLLock, INFINITE);
-    // it will endup blocking on Flush to limit rendering to 60FPS
+    }
-    if (!mAnimating) {
+    if (!mLooping) {
-      ::WaitForSingleObject(hEvent, INFINITE);
+      LeaveCriticalSection(&mGLLock);
+      break;
     }
-    mTasksMutex.lock();
     for (auto &&task : mTasks) {
       task();
     }
     mTasks.clear();
-    mTasksMutex.unlock();
+    LeaveCriticalSection(&mGLLock);
     mServo->PerformUpdates();
   }
-  log("Leaving loop");
   mServo->DeInit();
   cancel_current_task();
 } // namespace winrt::ServoApp::implementation
 
 void BrowserPage::StartRenderLoop() {
-  if (IsLoopRunning()) {
+  if (mLooping) {
 #if defined _DEBUG
     throw winrt::hresult_error(E_FAIL, L"GL thread is already looping");
 #else
     return;
 #endif
   }
+  mLooping = true;
   log("BrowserPage::StartRenderLoop(). UI thread: %i", GetCurrentThreadId());
-  auto token = mLoopCancel.get_token();
+  auto task = Concurrency::create_task([=] { Loop(); });
-  mLoopTask = std::make_unique<Concurrency::task<void>>(
+  mLoopTask = std::make_unique<Concurrency::task<void>>(task);
-      Concurrency::create_task([=] { Loop(token); }, token));
 }
 
 void BrowserPage::StopRenderLoop() {
-  if (IsLoopRunning()) {
+  if (mLooping) {
-    mLoopCancel.cancel();
+    EnterCriticalSection(&mGLLock);
-    WakeUp();
+    mLooping = false;
+    LeaveCriticalSection(&mGLLock);
+    WakeConditionVariable(&mGLCondVar);
     mLoopTask->wait();
     mLoopTask.reset();
   }
@@ -192,9 +183,9 @@ void BrowserPage::OnHistoryChanged(bool back, bool forward) {
 }
 
 void BrowserPage::OnShutdownComplete() {
-  log("Servo notified ShutdownComplete");
+  EnterCriticalSection(&mGLLock);
-  HANDLE hEvent = ::OpenEventA(EVENT_ALL_ACCESS, FALSE, sShutdownEvent);
+  mLooping = false;
-  ::SetEvent(hEvent);
+  LeaveCriticalSection(&mGLLock);
 }
 
 void BrowserPage::OnAlert(std::wstring message) {
@@ -225,15 +216,17 @@ void BrowserPage::Flush() {
 void BrowserPage::MakeCurrent() { mOpenGLES.MakeCurrent(mRenderSurface); }
 
 void BrowserPage::WakeUp() {
-  // FIXME: this won't work if it's triggered while the thread is not
+  RunOnGLThread([=] { });
-  // waiting. We need a better looping logic.
-  HANDLE hEvent = ::OpenEventA(EVENT_ALL_ACCESS, FALSE, sWakeupEvent);
-  ::SetEvent(hEvent);
 }
 
 bool BrowserPage::OnAllowNavigation(std::wstring) { return true; }
 
-void BrowserPage::OnAnimatingChanged(bool animating) { mAnimating = animating; }
+void BrowserPage::OnAnimatingChanged(bool animating) {
+  EnterCriticalSection(&mGLLock);
+  mAnimating = animating;
+  LeaveCriticalSection(&mGLLock);
+  WakeConditionVariable(&mGLCondVar);
+}
 
 template <typename Callable> void BrowserPage::RunOnUIThread(Callable cb) {
   swapChainPanel().Dispatcher().RunAsync(
@@ -300,10 +293,10 @@ void BrowserPage::OnSurfaceClicked(IInspectable const &,
 }
 
 void BrowserPage::RunOnGLThread(std::function<void()> task) {
-  mTasksMutex.lock();
+  EnterCriticalSection(&mGLLock);
   mTasks.push_back(task);
-  mTasksMutex.unlock();
+  LeaveCriticalSection(&mGLLock);
-  WakeUp();
+  WakeConditionVariable(&mGLCondVar);
 }
 
 } // namespace winrt::ServoApp::implementation

support/hololens/ServoApp/BrowserPage.h

@@ -11,9 +11,6 @@
 
 namespace winrt::ServoApp::implementation {
 
-static char sWakeupEvent[] = "SIGNAL_WAKEUP";
-static char sShutdownEvent[] = "SIGNAL_SHUTDOWN";
-
 struct BrowserPage : BrowserPageT<BrowserPage>,
                       public servo::ServoDelegate {
 public:
@@ -66,21 +63,22 @@ private:
 
   void StartRenderLoop();
   void StopRenderLoop();
-  void Loop(Concurrency::cancellation_token);
+  void Loop();
-  bool IsLoopRunning();
 
-  Concurrency::cancellation_token_source mLoopCancel;
   std::unique_ptr<Concurrency::task<void>> mLoopTask;
   winrt::ServoApp::ImmersiveViewSource mImmersiveViewSource;
   EGLSurface mRenderSurface{EGL_NO_SURFACE};
   std::unique_ptr<servo::Servo> mServo;
 
   std::vector<std::function<void()>> mTasks;
-  std::mutex mTasksMutex;
+
+  CRITICAL_SECTION mGLLock;
+  CONDITION_VARIABLE mGLCondVar;
+
+  bool mAnimating = false;
+  bool mLooping = false;
 
   OpenGLES mOpenGLES; // FIXME: shared pointer
-
-  bool mAnimating;
 };
 } // namespace winrt::ServoApp::implementation