From 4e451c1e92b36e57f3dd5cfe55517074da7e8e20 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sun, 17 Mar 2019 04:23:54 +0100 Subject: [PATCH] Add client-side double buffering of window backing stores. This prevents flicker and looks rather good. The main downside is that resizing gets even more sluggish. That's the price we pay for now. --- Applications/Terminal/main.cpp | 1 + LibGUI/GEventLoop.h | 2 +- LibGUI/GWindow.cpp | 87 ++++++++++++++++++++--------- LibGUI/GWindow.h | 12 +++- SharedGraphics/GraphicsBitmap.cpp | 2 +- SharedGraphics/GraphicsBitmap.h | 2 +- SharedGraphics/Painter.cpp | 2 +- WindowServer/WSAPITypes.h | 2 + WindowServer/WSClientConnection.cpp | 13 ++++- WindowServer/WSMessage.h | 5 +- WindowServer/WSMessageLoop.cpp | 2 +- WindowServer/WSWindowManager.cpp | 4 +- 12 files changed, 95 insertions(+), 39 deletions(-) diff --git a/Applications/Terminal/main.cpp b/Applications/Terminal/main.cpp index 3049f59697..808b14e9b3 100644 --- a/Applications/Terminal/main.cpp +++ b/Applications/Terminal/main.cpp @@ -88,6 +88,7 @@ int main(int argc, char** argv) make_shell(ptm_fd); auto* window = new GWindow; + window->set_double_buffering_enabled(false); window->set_should_exit_app_on_close(true); Terminal terminal(ptm_fd); diff --git a/LibGUI/GEventLoop.h b/LibGUI/GEventLoop.h index 8133ef471a..325115e70a 100644 --- a/LibGUI/GEventLoop.h +++ b/LibGUI/GEventLoop.h @@ -39,7 +39,7 @@ public: WSAPI_ServerMessage sync_request(const WSAPI_ClientMessage& request, WSAPI_ServerMessage::Type response_type); - pid_t server_pid() const { return s_server_pid; } + static pid_t server_pid() { return s_server_pid; } private: void wait_for_event(); diff --git a/LibGUI/GWindow.cpp b/LibGUI/GWindow.cpp index 5e246ac342..8486489f16 100644 --- a/LibGUI/GWindow.cpp +++ b/LibGUI/GWindow.cpp @@ -3,6 +3,7 @@ #include "GEventLoop.h" #include "GWidget.h" #include +#include #include #include #include @@ -166,33 +167,19 @@ void GWindow::event(GEvent& event) return; auto& paint_event = static_cast(event); auto rect = paint_event.rect(); - bool created_new_backing_store = !m_backing; - if (!m_backing) { - ASSERT(GEventLoop::main().server_pid()); - ASSERT(!paint_event.window_size().is_empty()); - Size new_backing_store_size = paint_event.window_size(); - size_t size_in_bytes = new_backing_store_size.area() * sizeof(RGBA32); - auto shared_buffer = SharedBuffer::create(GEventLoop::main().server_pid(), size_in_bytes); - ASSERT(shared_buffer); - m_backing = GraphicsBitmap::create_with_shared_buffer( - m_has_alpha_channel ? GraphicsBitmap::Format::RGBA32 : GraphicsBitmap::Format::RGB32, - *shared_buffer, - new_backing_store_size); - } + bool created_new_backing_store = !m_back_bitmap; + if (!m_back_bitmap) + m_back_bitmap = create_backing_bitmap(paint_event.window_size()); if (rect.is_empty() || created_new_backing_store) rect = m_main_widget->rect(); + m_main_widget->event(*make(rect)); - if (created_new_backing_store) { - WSAPI_ClientMessage message; - message.type = WSAPI_ClientMessage::Type::SetWindowBackingStore; - message.window_id = m_window_id; - message.backing.bpp = 32; - message.backing.pitch = m_backing->pitch(); - message.backing.shared_buffer_id = m_backing->shared_buffer_id(); - message.backing.has_alpha_channel = m_backing->has_alpha_channel(); - message.backing.size = m_backing->size(); - GEventLoop::main().post_message_to_server(message); - } + + if (m_double_buffering_enabled) + flip(rect); + else if (created_new_backing_store) + set_current_backing_bitmap(*m_back_bitmap, true); + if (m_window_id) { WSAPI_ClientMessage message; message.type = WSAPI_ClientMessage::Type::DidFinishPainting; @@ -232,8 +219,8 @@ void GWindow::event(GEvent& event) if (event.type() == GEvent::Resize) { auto new_size = static_cast(event).size(); - if (m_backing && m_backing->size() != new_size) - m_backing = nullptr; + if (m_back_bitmap && m_back_bitmap->size() != new_size) + m_back_bitmap = nullptr; m_pending_paint_event_rects.clear(); m_rect_when_windowless = { { }, new_size }; m_main_widget->set_relative_rect({ { }, new_size }); @@ -328,6 +315,12 @@ void GWindow::set_has_alpha_channel(bool value) m_has_alpha_channel = value; } +void GWindow::set_double_buffering_enabled(bool value) +{ + ASSERT(!m_window_id); + m_double_buffering_enabled = value; +} + void GWindow::set_opacity(float opacity) { m_opacity_when_windowless = opacity; @@ -354,3 +347,45 @@ void GWindow::set_hovered_widget(GWidget* widget) if (m_hovered_widget) GEventLoop::main().post_event(*m_hovered_widget, make(GEvent::Enter)); } + +void GWindow::set_current_backing_bitmap(GraphicsBitmap& bitmap, bool flush_immediately) +{ + WSAPI_ClientMessage message; + message.type = WSAPI_ClientMessage::Type::SetWindowBackingStore; + message.window_id = m_window_id; + message.backing.bpp = 32; + message.backing.pitch = bitmap.pitch(); + message.backing.shared_buffer_id = bitmap.shared_buffer_id(); + message.backing.has_alpha_channel = bitmap.has_alpha_channel(); + message.backing.size = bitmap.size(); + message.backing.flush_immediately = flush_immediately; + GEventLoop::main().sync_request(message, WSAPI_ServerMessage::Type::DidSetWindowBackingStore); +} + +void GWindow::flip(const Rect& dirty_rect) +{ + swap(m_front_bitmap, m_back_bitmap); + + set_current_backing_bitmap(*m_front_bitmap); + + if (!m_back_bitmap || m_back_bitmap->size() != m_front_bitmap->size()) { + m_back_bitmap = create_backing_bitmap(m_front_bitmap->size()); + memcpy(m_back_bitmap->scanline(0), m_front_bitmap->scanline(0), m_front_bitmap->size().area() * sizeof(RGBA32)); + return; + } + + // Copy whatever was painted from the front to the back. + Painter painter(*m_back_bitmap); + painter.blit(dirty_rect.location(), *m_front_bitmap, dirty_rect); +} + +Retained GWindow::create_backing_bitmap(const Size& size) +{ + ASSERT(GEventLoop::server_pid()); + ASSERT(!size.is_empty()); + size_t size_in_bytes = size.area() * sizeof(RGBA32); + auto shared_buffer = SharedBuffer::create(GEventLoop::server_pid(), size_in_bytes); + ASSERT(shared_buffer); + auto format = m_has_alpha_channel ? GraphicsBitmap::Format::RGBA32 : GraphicsBitmap::Format::RGB32; + return GraphicsBitmap::create_with_shared_buffer(format, *shared_buffer, size); +} diff --git a/LibGUI/GWindow.h b/LibGUI/GWindow.h index dfe69cb296..16abab6cbf 100644 --- a/LibGUI/GWindow.h +++ b/LibGUI/GWindow.h @@ -15,6 +15,7 @@ public: static GWindow* from_window_id(int); + void set_double_buffering_enabled(bool); void set_has_alpha_channel(bool); void set_opacity(float); @@ -68,7 +69,8 @@ public: const GWidget* hovered_widget() const { return m_hovered_widget.ptr(); } void set_hovered_widget(GWidget*); - GraphicsBitmap* backing() { return m_backing.ptr(); } + GraphicsBitmap* front_bitmap() { return m_front_bitmap.ptr(); } + GraphicsBitmap* back_bitmap() { return m_back_bitmap.ptr(); } Size size_increment() const { return m_size_increment; } void set_size_increment(const Size& increment) { m_size_increment = increment; } @@ -78,7 +80,12 @@ public: private: virtual const char* class_name() const override { return "GWindow"; } - RetainPtr m_backing; + Retained create_backing_bitmap(const Size&); + void set_current_backing_bitmap(GraphicsBitmap&, bool flush_immediately = false); + void flip(const Rect& dirty_rect); + + RetainPtr m_front_bitmap; + RetainPtr m_back_bitmap; int m_window_id { 0 }; float m_opacity_when_windowless { 1.0f }; GWidget* m_main_widget { nullptr }; @@ -93,5 +100,6 @@ private: bool m_is_active { false }; bool m_should_exit_app_on_close { false }; bool m_has_alpha_channel { false }; + bool m_double_buffering_enabled { true }; }; diff --git a/SharedGraphics/GraphicsBitmap.cpp b/SharedGraphics/GraphicsBitmap.cpp index 1df228062d..eb41c041c5 100644 --- a/SharedGraphics/GraphicsBitmap.cpp +++ b/SharedGraphics/GraphicsBitmap.cpp @@ -58,7 +58,7 @@ GraphicsBitmap::GraphicsBitmap(Format format, const Size& size, RGBA32* data) { } -RetainPtr GraphicsBitmap::create_with_shared_buffer(Format format, Retained&& shared_buffer, const Size& size) +Retained GraphicsBitmap::create_with_shared_buffer(Format format, Retained&& shared_buffer, const Size& size) { return adopt(*new GraphicsBitmap(format, move(shared_buffer), size)); } diff --git a/SharedGraphics/GraphicsBitmap.h b/SharedGraphics/GraphicsBitmap.h index 156eb0f2d0..9819b33109 100644 --- a/SharedGraphics/GraphicsBitmap.h +++ b/SharedGraphics/GraphicsBitmap.h @@ -15,7 +15,7 @@ public: static Retained create(Format, const Size&); static Retained create_wrapper(Format, const Size&, RGBA32*); static RetainPtr load_from_file(Format, const String& path, const Size&); - static RetainPtr create_with_shared_buffer(Format, Retained&&, const Size&); + static Retained create_with_shared_buffer(Format, Retained&&, const Size&); ~GraphicsBitmap(); RGBA32* scanline(int y); diff --git a/SharedGraphics/Painter.cpp b/SharedGraphics/Painter.cpp index 3cbe65f3b8..35479090ec 100644 --- a/SharedGraphics/Painter.cpp +++ b/SharedGraphics/Painter.cpp @@ -28,7 +28,7 @@ Painter::Painter(GraphicsBitmap& bitmap) #ifdef LIBGUI Painter::Painter(GWidget& widget) : m_window(widget.window()) - , m_target(*m_window->backing()) + , m_target(*m_window->back_bitmap()) { m_state_stack.append(State()); state().font = &widget.font(); diff --git a/WindowServer/WSAPITypes.h b/WindowServer/WSAPITypes.h index 15291790fe..bcf075cca9 100644 --- a/WindowServer/WSAPITypes.h +++ b/WindowServer/WSAPITypes.h @@ -87,6 +87,7 @@ struct WSAPI_ServerMessage { Greeting, DidGetClipboardContents, DidSetClipboardContents, + DidSetWindowBackingStore, }; Type type { Invalid }; int window_id { -1 }; @@ -191,6 +192,7 @@ struct WSAPI_ClientMessage { size_t pitch; int shared_buffer_id; bool has_alpha_channel; + bool flush_immediately; } backing; struct { int shared_buffer_id; diff --git a/WindowServer/WSClientConnection.cpp b/WindowServer/WSClientConnection.cpp index 08a07bf685..0ad739b82d 100644 --- a/WindowServer/WSClientConnection.cpp +++ b/WindowServer/WSClientConnection.cpp @@ -357,6 +357,7 @@ void WSClientConnection::handle_request(WSAPICreateWindowRequest& request) window->set_opacity(request.opacity()); window->set_size_increment(request.size_increment()); window->set_base_size(request.base_size()); + window->invalidate(); m_windows.set(window_id, move(window)); WSAPI_ServerMessage response; response.type = WSAPI_ServerMessage::Type::DidCreateWindow; @@ -451,10 +452,16 @@ void WSClientConnection::handle_request(WSAPISetWindowBackingStoreRequest& reque request.has_alpha_channel() ? GraphicsBitmap::Format::RGBA32 : GraphicsBitmap::Format::RGB32, *shared_buffer, request.size()); - if (!backing_store) - return; window.set_backing_store(move(backing_store)); - window.invalidate(); + + if (request.flush_immediately()) + window.invalidate(); + + WSAPI_ServerMessage response; + response.type = WSAPI_ServerMessage::Type::DidSetWindowBackingStore; + response.window_id = window_id; + response.backing.shared_buffer_id = request.shared_buffer_id(); + post_message(response); } void WSClientConnection::handle_request(WSAPISetGlobalCursorTrackingRequest& request) diff --git a/WindowServer/WSMessage.h b/WindowServer/WSMessage.h index eaebfb24d1..0ef6838a48 100644 --- a/WindowServer/WSMessage.h +++ b/WindowServer/WSMessage.h @@ -320,7 +320,7 @@ private: class WSAPISetWindowBackingStoreRequest final : public WSAPIClientRequest { public: - explicit WSAPISetWindowBackingStoreRequest(int client_id, int window_id, int shared_buffer_id, const Size& size, size_t bpp, size_t pitch, bool has_alpha_channel) + explicit WSAPISetWindowBackingStoreRequest(int client_id, int window_id, int shared_buffer_id, const Size& size, size_t bpp, size_t pitch, bool has_alpha_channel, bool flush_immediately) : WSAPIClientRequest(WSMessage::APISetWindowBackingStoreRequest, client_id) , m_client_id(client_id) , m_window_id(window_id) @@ -329,6 +329,7 @@ public: , m_bpp(bpp) , m_pitch(pitch) , m_has_alpha_channel(has_alpha_channel) + , m_flush_immediately(flush_immediately) { } @@ -339,6 +340,7 @@ public: size_t bpp() const { return m_bpp; } size_t pitch() const { return m_pitch; } bool has_alpha_channel() const { return m_has_alpha_channel; } + bool flush_immediately() const { return m_flush_immediately; } private: int m_client_id { 0 }; @@ -348,6 +350,7 @@ private: size_t m_bpp; size_t m_pitch; bool m_has_alpha_channel; + bool m_flush_immediately; }; class WSAPISetWindowRectRequest final : public WSAPIClientRequest { diff --git a/WindowServer/WSMessageLoop.cpp b/WindowServer/WSMessageLoop.cpp index 06731e0375..f7283d4732 100644 --- a/WindowServer/WSMessageLoop.cpp +++ b/WindowServer/WSMessageLoop.cpp @@ -321,7 +321,7 @@ void WSMessageLoop::on_receive_from_client(int client_id, const WSAPI_ClientMess post_message(client, make(client_id, message.window_id)); break; case WSAPI_ClientMessage::Type::SetWindowBackingStore: - post_message(client, make(client_id, message.window_id, message.backing.shared_buffer_id, message.backing.size, message.backing.bpp, message.backing.pitch, message.backing.has_alpha_channel)); + post_message(client, make(client_id, message.window_id, message.backing.shared_buffer_id, message.backing.size, message.backing.bpp, message.backing.pitch, message.backing.has_alpha_channel, message.backing.flush_immediately)); break; case WSAPI_ClientMessage::Type::SetGlobalCursorTracking: post_message(client, make(client_id, message.window_id, message.value)); diff --git a/WindowServer/WSWindowManager.cpp b/WindowServer/WSWindowManager.cpp index c2ac81e020..db9f53cabd 100644 --- a/WindowServer/WSWindowManager.cpp +++ b/WindowServer/WSWindowManager.cpp @@ -916,8 +916,6 @@ void WSWindowManager::compose() for_each_visible_window_from_back_to_front([&] (WSWindow& window) { RetainPtr backing_store = window.backing_store(); - if (!backing_store) - return IterationDecision::Continue; if (!any_dirty_rect_intersects_window(window)) return IterationDecision::Continue; PainterStateSaver saver(*m_back_painter); @@ -928,6 +926,8 @@ void WSWindowManager::compose() PainterStateSaver saver(*m_back_painter); m_back_painter->set_clip_rect(dirty_rect); paint_window_frame(window); + if (!backing_store) + continue; Rect dirty_rect_in_window_coordinates = Rect::intersection(dirty_rect, window.rect()); if (dirty_rect_in_window_coordinates.is_empty()) continue;