Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 9 additions & 10 deletions shell/platform/windows/angle_surface_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,7 @@ void AngleSurfaceManager::CleanUp() {

bool AngleSurfaceManager::CreateSurface(WindowsRenderTarget* render_target,
EGLint width,
EGLint height,
bool vsync_enabled) {
EGLint height) {
if (!render_target || !initialize_succeeded_) {
return false;
}
Expand All @@ -255,13 +254,6 @@ bool AngleSurfaceManager::CreateSurface(WindowsRenderTarget* render_target,
surface_width_ = width;
surface_height_ = height;
render_surface_ = surface;

if (!MakeCurrent()) {
LogEglError("Unable to make surface current to update the swap interval");
return false;
}

SetVSyncEnabled(vsync_enabled);
return true;
}

Expand All @@ -277,11 +269,13 @@ void AngleSurfaceManager::ResizeSurface(WindowsRenderTarget* render_target,

ClearContext();
DestroySurface();
if (!CreateSurface(render_target, width, height, vsync_enabled)) {
if (!CreateSurface(render_target, width, height)) {
FML_LOG(ERROR)
<< "AngleSurfaceManager::ResizeSurface failed to create surface";
}
}

SetVSyncEnabled(vsync_enabled);
}

void AngleSurfaceManager::GetSurfaceDimensions(EGLint* width, EGLint* height) {
Expand Down Expand Up @@ -342,6 +336,11 @@ EGLSurface AngleSurfaceManager::CreateSurfaceFromHandle(
}

void AngleSurfaceManager::SetVSyncEnabled(bool enabled) {
if (!MakeCurrent()) {
LogEglError("Unable to make surface current to update the swap interval");
return;
}

// OpenGL swap intervals can be used to prevent screen tearing.
// If enabled, the raster thread blocks until the v-blank.
// This is unnecessary if DWM composition is enabled.
Expand Down
14 changes: 10 additions & 4 deletions shell/platform/windows/angle_surface_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,11 @@ class AngleSurfaceManager {
// Target represents the visual entity to bind to. Width and
// height represent dimensions surface is created at.
//
// This binds |egl_context_| to the current thread.
// After the surface is created, |SetVSyncEnabled| should be called on a
// thread that can bind the |egl_context_|.
virtual bool CreateSurface(WindowsRenderTarget* render_target,
EGLint width,
EGLint height,
bool enable_vsync);
EGLint height);

// Resizes backing surface from current size to newly requested size
// based on width and height for the specific case when width and height do
Expand Down Expand Up @@ -68,7 +68,7 @@ class AngleSurfaceManager {
virtual bool MakeCurrent();

// Unbinds the current EGL context from the current thread.
bool ClearCurrent();
virtual bool ClearCurrent();

// Clears the |egl_context_| draw and read surfaces.
bool ClearContext();
Expand All @@ -91,6 +91,12 @@ class AngleSurfaceManager {

// If enabled, makes the current surface's buffer swaps block until the
// v-blank.
//
// If disabled, allows one thread to swap multiple buffers per v-blank
// but can result in screen tearing if the system compositor is disabled.
//
// This binds |egl_context_| to the current thread and makes the render
// surface current.
virtual void SetVSyncEnabled(bool enabled);

// Gets the |ID3D11Device| chosen by ANGLE.
Expand Down
2 changes: 1 addition & 1 deletion shell/platform/windows/flutter_window.cc
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ ui::AXPlatformNodeWin* FlutterWindow::GetAlert() {
return alert_node_.get();
}

bool FlutterWindow::NeedsVSync() {
bool FlutterWindow::NeedsVSync() const {
// If the Desktop Window Manager composition is enabled,
// the system itself synchronizes with v-sync.
// See: https://learn.microsoft.com/windows/win32/dwm/composition-ovw
Expand Down
2 changes: 1 addition & 1 deletion shell/platform/windows/flutter_window.h
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ class FlutterWindow : public KeyboardManager::WindowDelegate,
virtual ui::AXPlatformNodeWin* GetAlert() override;

// |WindowBindingHandler|
virtual bool NeedsVSync() override;
virtual bool NeedsVSync() const override;

// Called to obtain a pointer to the fragment root delegate.
virtual ui::AXFragmentRootDelegateWin* GetAxFragmentRootDelegate();
Expand Down
2 changes: 1 addition & 1 deletion shell/platform/windows/flutter_windows_engine.cc
Original file line number Diff line number Diff line change
Expand Up @@ -685,7 +685,7 @@ bool FlutterWindowsEngine::MarkExternalTextureFrameAvailable(
engine_, texture_id) == kSuccess);
}

bool FlutterWindowsEngine::PostRasterThreadTask(fml::closure callback) {
bool FlutterWindowsEngine::PostRasterThreadTask(fml::closure callback) const {
struct Captures {
fml::closure callback;
};
Expand Down
8 changes: 5 additions & 3 deletions shell/platform/windows/flutter_windows_engine.h
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ class FlutterWindowsEngine {
bool Run(std::string_view entrypoint);

// Returns true if the engine is currently running.
bool running() { return engine_ != nullptr; }
virtual bool running() const { return engine_ != nullptr; }

// Stops the engine. This invalidates the pointer returned by engine().
//
Expand Down Expand Up @@ -141,7 +141,9 @@ class FlutterWindowsEngine {

// The ANGLE surface manager object. If this is nullptr, then we are
// rendering using software instead of OpenGL.
AngleSurfaceManager* surface_manager() { return surface_manager_.get(); }
AngleSurfaceManager* surface_manager() const {
return surface_manager_.get();
}

WindowProcDelegateManager* window_proc_delegate_manager() {
return window_proc_delegate_manager_.get();
Expand Down Expand Up @@ -201,7 +203,7 @@ class FlutterWindowsEngine {
bool MarkExternalTextureFrameAvailable(int64_t texture_id);

// Posts the given callback onto the raster thread.
virtual bool PostRasterThreadTask(fml::closure callback);
virtual bool PostRasterThreadTask(fml::closure callback) const;

// Invoke on the embedder's vsync callback to schedule a frame.
void OnVsync(intptr_t baton);
Expand Down
60 changes: 35 additions & 25 deletions shell/platform/windows/flutter_windows_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,38 @@ bool SurfaceWillUpdate(size_t cur_width,
(cur_height != target_height) || (cur_width != target_width);
return non_zero_target_dims && not_same_size;
}

/// Update the surface's swap interval to block until the v-blank iff
/// the system compositor is disabled.
void UpdateVsync(const FlutterWindowsEngine& engine,
const WindowBindingHandler& window) {
AngleSurfaceManager* surface_manager = engine.surface_manager();
if (!surface_manager) {
return;
}

// Updating the vsync makes the EGL context and render surface current.
// If the engine is running, the render surface should only be made current on
// the raster thread. If the engine is initializing, the raster thread doesn't
// exist yet and the render surface can be made current on the platform
// thread.
Comment on lines +49 to +53
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this operation safe/effective on either the raster or platform thread? Or in other words, should we expect it to work both before and after the engine initializes? And if so, what is the reasoning for posting it as a task only when it is possible to do so?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this operation safe/effective on either the raster or platform thread?

No. The EGL context cannot be current on multiple threads. Since the raster thread uses the EGL context without any synchronization, other threads shouldn't use the EGL context at all while the raster thread exists. In other words, this operation must not run on the platform thread while the engine is running and the raster thread exists.

However, the EGL context is unused before the engine is initialized. Thus, this operation is safe on the platform thread during startup.

should we expect it to work both before and after the engine initializes?

Yup

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is achieved by running it on the platform thread when the engine is not initialized? Do we want to make the EGL context current at all on the platform thread in the case where there is not yet any raster thread?

Copy link
Member Author

@loic-sharma loic-sharma Nov 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our end goal is to set the surface's swap interval to 0. Without this, you'd see Windows takes ~17ms - the full frame budget - to render each surface as EGL's swap buffer blocks until the v-blank. This would prevent us from rendering multiple views using a single raster thread on Windows.

To update the swap interval, you first must first make the EGL context current.

Ideally, only the raster thread would make the EGL context current. However, doing so would require us to update the swap intervals in 3 locations:

  1. When a view is created and the engine is running, we can update its surface's swap interval immediately
  2. When the engine is started, we can update surfaces that were created before the engine was started 
  3. When the system compositor changes, we need to update surfaces' swap intervals (the blocking until v-blank prevents screen tearing in this scenario)

Using the platform thread allows us to remove that 2nd location by allowing us to update the swap interval whenever a view is created.

auto needs_vsync = window.NeedsVSync();
if (engine.running()) {
engine.PostRasterThreadTask([surface_manager, needs_vsync]() {
surface_manager->SetVSyncEnabled(needs_vsync);
});
} else {
surface_manager->SetVSyncEnabled(needs_vsync);

// Release the EGL context so that the raster thread can use it.
if (!surface_manager->ClearCurrent()) {
FML_LOG(ERROR)
<< "Unable to clear current surface after updating the swap interval";
return;
}
}
}

} // namespace

FlutterWindowsView::FlutterWindowsView(
Expand Down Expand Up @@ -580,15 +612,10 @@ bool FlutterWindowsView::PresentSoftwareBitmap(const void* allocation,
void FlutterWindowsView::CreateRenderSurface() {
if (engine_ && engine_->surface_manager()) {
PhysicalWindowBounds bounds = binding_handler_->GetPhysicalWindowBounds();
bool enable_vsync = binding_handler_->NeedsVSync();
engine_->surface_manager()->CreateSurface(GetRenderTarget(), bounds.width,
bounds.height, enable_vsync);
bounds.height);

// The EGL context cannot be current on multiple threads.
// Creating the render surface runs on the platform thread and
// makes the EGL context current. Thus, the EGL context must be
// released so that the raster thread can use it for rendering.
engine_->surface_manager()->ClearCurrent();
UpdateVsync(*engine_, *binding_handler_);

resize_target_width_ = bounds.width;
resize_target_height_ = bounds.height;
Expand Down Expand Up @@ -660,24 +687,7 @@ void FlutterWindowsView::UpdateSemanticsEnabled(bool enabled) {
}

void FlutterWindowsView::OnDwmCompositionChanged() {
AngleSurfaceManager* surface_manager = engine_->surface_manager();
if (!surface_manager) {
return;
}

// Update the surface with the new composition state.
// Switch to the raster thread as the render EGL context can only be
// current on a single thread a time.
auto needs_vsync = binding_handler_->NeedsVSync();
engine_->PostRasterThreadTask([surface_manager, needs_vsync]() {
if (!surface_manager->MakeCurrent()) {
FML_LOG(ERROR)
<< "Unable to make surface current to update the swap interval";
return;
}

surface_manager->SetVSyncEnabled(needs_vsync);
});
UpdateVsync(*engine_, *binding_handler_);
}

void FlutterWindowsView::OnWindowStateEvent(HWND hwnd, WindowStateEvent event) {
Expand Down
Loading