summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJelle Raaijmakers <jelle@gmta.nl>2023-01-11 21:46:14 +0100
committerAndreas Kling <kling@serenityos.org>2023-01-17 18:16:02 +0100
commit634d1e0197531e924de71f4f5cb8b7b2357ac578 (patch)
tree32869842e0794a9cc795ea129eed5f041a72c5c1
parent45e85d20b64862df119f643f24e2d500c76c58f3 (diff)
downloadserenity-634d1e0197531e924de71f4f5cb8b7b2357ac578.zip
LibGUI+WindowServer: Improve window resizing performance
The old `GUI::Window` resizing behavior created a new backing store for each resize event (i.e. every visible window size). This caused a lot of trashing and on my machine, caused up to 25% of CPU time spent in creating new backing stores. The new behavior is a bit more sensible: * If the window size is shrinking, the backing store is already large enough to contain the entire window - so we don't create a new one. * If the window size is growing, as soon as the backing store can no longer contain the window, it is inflated with a large margin (of an arbitrary chosen 64 pixels) in both directions to accommodate some leeway in resizing before an even larger backing store is required. * When the user stops resizing the window, the backing store is resized to the exact dimensions of the window. For me, this brings the CPU time for creating backing stores down to 0%.
-rw-r--r--Userland/Libraries/LibGUI/Window.cpp79
-rw-r--r--Userland/Libraries/LibGUI/Window.h8
-rw-r--r--Userland/Services/WindowServer/Compositor.cpp13
-rw-r--r--Userland/Services/WindowServer/ConnectionFromClient.cpp3
-rw-r--r--Userland/Services/WindowServer/ConnectionFromClient.h2
-rw-r--r--Userland/Services/WindowServer/Window.cpp1
-rw-r--r--Userland/Services/WindowServer/Window.h4
-rw-r--r--Userland/Services/WindowServer/WindowServer.ipc2
8 files changed, 73 insertions, 39 deletions
diff --git a/Userland/Libraries/LibGUI/Window.cpp b/Userland/Libraries/LibGUI/Window.cpp
index 3c6d7c369b..4e2cbda7d4 100644
--- a/Userland/Libraries/LibGUI/Window.cpp
+++ b/Userland/Libraries/LibGUI/Window.cpp
@@ -39,6 +39,7 @@ public:
explicit WindowBackingStore(NonnullRefPtr<Gfx::Bitmap> bitmap)
: m_bitmap(move(bitmap))
, m_serial(++s_next_backing_store_serial)
+ , m_visible_size(m_bitmap->size())
{
}
@@ -49,9 +50,13 @@ public:
i32 serial() const { return m_serial; }
+ Gfx::IntSize visible_size() const { return m_visible_size; }
+ void set_visible_size(Gfx::IntSize visible_size) { m_visible_size = visible_size; }
+
private:
NonnullRefPtr<Gfx::Bitmap> m_bitmap;
const i32 m_serial;
+ Gfx::IntSize m_visible_size;
};
static NeverDestroyed<HashTable<Window*>> all_windows;
@@ -395,6 +400,15 @@ void Window::handle_mouse_event(MouseEvent& event)
result.widget->dispatch_event(local_event, this);
}
+Gfx::IntSize Window::backing_store_size(Gfx::IntSize window_size) const
+{
+ if (!m_resizing)
+ return window_size;
+
+ int const backing_margin_during_resize = 64;
+ return { window_size.width() + backing_margin_during_resize, window_size.height() + backing_margin_during_resize };
+}
+
void Window::handle_multi_paint_event(MultiPaintEvent& event)
{
if (!is_visible())
@@ -409,16 +423,22 @@ void Window::handle_multi_paint_event(MultiPaintEvent& event)
rects.extend(move(m_pending_paint_event_rects));
}
VERIFY(!rects.is_empty());
- if (m_back_store && m_back_store->size() != event.window_size()) {
- // Eagerly discard the backing store if we learn from this paint event that it needs to be bigger.
- // Otherwise we would have to wait for a resize event to tell us. This way we don't waste the
- // effort on painting into an undersized bitmap that will be thrown away anyway.
+
+ // Throw away our backing store if its size is different, and we've stopped resizing or double buffering is disabled.
+ // This ensures that we shrink the backing store after a resize, and that we do not get flickering artifacts when
+ // directly painting into a shared active backing store.
+ if (m_back_store && (!m_resizing || !m_double_buffering_enabled) && m_back_store->size() != event.window_size())
m_back_store = nullptr;
- }
- bool created_new_backing_store = !m_back_store;
+
+ // Discard our backing store if it's unable to contain the new window size. Smaller is fine though, that prevents
+ // lots of backing store allocations during a resize.
+ if (m_back_store && !m_back_store->size().contains(event.window_size()))
+ m_back_store = nullptr;
+
+ bool created_new_backing_store = false;
if (!m_back_store) {
- m_back_store = create_backing_store(event.window_size());
- VERIFY(m_back_store);
+ m_back_store = create_backing_store(backing_store_size(event.window_size())).release_value_but_fixme_should_propagate_errors();
+ created_new_backing_store = true;
} else if (m_double_buffering_enabled) {
bool was_purged = false;
bool bitmap_has_memory = m_back_store->bitmap().set_nonvolatile(was_purged);
@@ -439,8 +459,7 @@ void Window::handle_multi_paint_event(MultiPaintEvent& event)
}
}
- auto rect = rects.first();
- if (rect.is_empty() || created_new_backing_store) {
+ if (created_new_backing_store) {
rects.clear();
rects.append({ {}, event.window_size() });
}
@@ -449,6 +468,7 @@ void Window::handle_multi_paint_event(MultiPaintEvent& event)
PaintEvent paint_event(rect);
m_main_widget->dispatch_event(paint_event, this);
}
+ m_back_store->set_visible_size(event.window_size());
if (m_double_buffering_enabled)
flip(rects);
@@ -519,8 +539,10 @@ void Window::handle_key_event(KeyEvent& event)
void Window::handle_resize_event(ResizeEvent& event)
{
auto new_size = event.size();
- if (m_back_store && m_back_store->size() != new_size)
- m_back_store = nullptr;
+
+ // When the user is done resizing, we receive a last resize event with our actual size.
+ m_resizing = new_size != m_rect_when_windowless.size();
+
if (!m_pending_paint_event_rects.is_empty()) {
m_pending_paint_event_rects.clear_with_capacity();
m_pending_paint_event_rects.append({ {}, new_size });
@@ -895,10 +917,19 @@ void Window::set_hovered_widget(Widget* widget)
update();
}
-void Window::set_current_backing_store(WindowBackingStore& backing_store, bool flush_immediately)
+void Window::set_current_backing_store(WindowBackingStore& backing_store, bool flush_immediately) const
{
auto& bitmap = backing_store.bitmap();
- ConnectionToWindowServer::the().set_window_backing_store(m_window_id, 32, bitmap.pitch(), bitmap.anonymous_buffer().fd(), backing_store.serial(), bitmap.has_alpha_channel(), bitmap.size(), flush_immediately);
+ ConnectionToWindowServer::the().set_window_backing_store(
+ m_window_id,
+ 32,
+ bitmap.pitch(),
+ bitmap.anonymous_buffer().fd(),
+ backing_store.serial(),
+ bitmap.has_alpha_channel(),
+ bitmap.size(),
+ backing_store.visible_size(),
+ flush_immediately);
}
void Window::flip(Vector<Gfx::IntRect, 32> const& dirty_rects)
@@ -908,8 +939,7 @@ void Window::flip(Vector<Gfx::IntRect, 32> const& dirty_rects)
set_current_backing_store(*m_front_store);
if (!m_back_store || m_back_store->size() != m_front_store->size()) {
- m_back_store = create_backing_store(m_front_store->size());
- VERIFY(m_back_store);
+ m_back_store = create_backing_store(m_front_store->size()).release_value_but_fixme_should_propagate_errors();
memcpy(m_back_store->bitmap().scanline(0), m_front_store->bitmap().scanline(0), m_front_store->bitmap().size_in_bytes());
m_back_store->bitmap().set_volatile();
return;
@@ -923,7 +953,7 @@ void Window::flip(Vector<Gfx::IntRect, 32> const& dirty_rects)
m_back_store->bitmap().set_volatile();
}
-OwnPtr<WindowBackingStore> Window::create_backing_store(Gfx::IntSize size)
+ErrorOr<NonnullOwnPtr<WindowBackingStore>> Window::create_backing_store(Gfx::IntSize size)
{
auto format = m_has_alpha_channel ? Gfx::BitmapFormat::BGRA8888 : Gfx::BitmapFormat::BGRx8888;
@@ -931,20 +961,11 @@ OwnPtr<WindowBackingStore> Window::create_backing_store(Gfx::IntSize size)
size_t pitch = Gfx::Bitmap::minimum_pitch(size.width(), format);
size_t size_in_bytes = size.height() * pitch;
- auto buffer_or_error = Core::AnonymousBuffer::create_with_size(round_up_to_power_of_two(size_in_bytes, PAGE_SIZE));
- if (buffer_or_error.is_error()) {
- perror("anon_create");
- return {};
- }
+ auto buffer = TRY(Core::AnonymousBuffer::create_with_size(round_up_to_power_of_two(size_in_bytes, PAGE_SIZE)));
// FIXME: Plumb scale factor here eventually.
- auto bitmap_or_error = Gfx::Bitmap::try_create_with_anonymous_buffer(format, buffer_or_error.release_value(), size, 1, {});
- if (bitmap_or_error.is_error()) {
- VERIFY(size.width() <= INT16_MAX);
- VERIFY(size.height() <= INT16_MAX);
- return {};
- }
- return make<WindowBackingStore>(bitmap_or_error.release_value());
+ auto bitmap = TRY(Gfx::Bitmap::try_create_with_anonymous_buffer(format, buffer, size, 1, {}));
+ return make<WindowBackingStore>(bitmap);
}
void Window::wm_event(WMEvent&)
diff --git a/Userland/Libraries/LibGUI/Window.h b/Userland/Libraries/LibGUI/Window.h
index 43652891b8..fbf1b552ea 100644
--- a/Userland/Libraries/LibGUI/Window.h
+++ b/Userland/Libraries/LibGUI/Window.h
@@ -7,7 +7,9 @@
#pragma once
#include <AK/DeprecatedString.h>
+#include <AK/Error.h>
#include <AK/Function.h>
+#include <AK/NonnullOwnPtr.h>
#include <AK/OwnPtr.h>
#include <AK/Variant.h>
#include <AK/WeakPtr.h>
@@ -262,8 +264,9 @@ private:
void server_did_destroy();
- OwnPtr<WindowBackingStore> create_backing_store(Gfx::IntSize);
- void set_current_backing_store(WindowBackingStore&, bool flush_immediately = false);
+ ErrorOr<NonnullOwnPtr<WindowBackingStore>> create_backing_store(Gfx::IntSize);
+ Gfx::IntSize backing_store_size(Gfx::IntSize) const;
+ void set_current_backing_store(WindowBackingStore&, bool flush_immediately = false) const;
void flip(Vector<Gfx::IntRect, 32> const& dirty_rects);
void force_update();
@@ -312,6 +315,7 @@ private:
bool m_visible { false };
bool m_moved_by_client { false };
bool m_blocks_emoji_input { false };
+ bool m_resizing { false };
};
}
diff --git a/Userland/Services/WindowServer/Compositor.cpp b/Userland/Services/WindowServer/Compositor.cpp
index 862d66ccf7..b0b55f119e 100644
--- a/Userland/Services/WindowServer/Compositor.cpp
+++ b/Userland/Services/WindowServer/Compositor.cpp
@@ -386,6 +386,10 @@ void Compositor::compose()
});
}
+ auto update_window_rect = window_rect.intersected(rect);
+ if (update_window_rect.is_empty())
+ return;
+
auto clear_window_rect = [&](const Gfx::IntRect& clear_rect) {
auto fill_color = wm.palette().window();
if (!window.is_opaque())
@@ -394,7 +398,7 @@ void Compositor::compose()
};
if (!backing_store) {
- clear_window_rect(window_rect.intersected(rect));
+ clear_window_rect(update_window_rect);
return;
}
@@ -407,7 +411,7 @@ void Compositor::compose()
// it was previously, and fill the rest of the window with its
// background color.
Gfx::IntRect backing_rect;
- backing_rect.set_size(backing_store->size());
+ backing_rect.set_size(window.backing_store_visible_size());
switch (WindowManager::the().resize_direction_of_window(window)) {
case ResizeDirection::None:
case ResizeDirection::Right:
@@ -434,8 +438,7 @@ void Compositor::compose()
break;
}
- Gfx::IntRect dirty_rect_in_backing_coordinates = rect.intersected(window_rect)
- .intersected(backing_rect)
+ Gfx::IntRect dirty_rect_in_backing_coordinates = update_window_rect.intersected(backing_rect)
.translated(-backing_rect.location());
if (!dirty_rect_in_backing_coordinates.is_empty()) {
@@ -459,7 +462,7 @@ void Compositor::compose()
}
}
- for (auto background_rect : window_rect.shatter(backing_rect))
+ for (auto background_rect : update_window_rect.shatter(backing_rect))
clear_window_rect(background_rect);
};
diff --git a/Userland/Services/WindowServer/ConnectionFromClient.cpp b/Userland/Services/WindowServer/ConnectionFromClient.cpp
index 213090ceb0..64fedd0e5b 100644
--- a/Userland/Services/WindowServer/ConnectionFromClient.cpp
+++ b/Userland/Services/WindowServer/ConnectionFromClient.cpp
@@ -733,7 +733,7 @@ void ConnectionFromClient::did_finish_painting(i32 window_id, Vector<Gfx::IntRec
void ConnectionFromClient::set_window_backing_store(i32 window_id, [[maybe_unused]] i32 bpp,
[[maybe_unused]] i32 pitch, IPC::File const& anon_file, i32 serial, bool has_alpha_channel,
- Gfx::IntSize size, bool flush_immediately)
+ Gfx::IntSize size, Gfx::IntSize visible_size, bool flush_immediately)
{
auto it = m_windows.find(window_id);
if (it == m_windows.end()) {
@@ -761,6 +761,7 @@ void ConnectionFromClient::set_window_backing_store(i32 window_id, [[maybe_unuse
}
window.set_backing_store(backing_store_or_error.release_value(), serial);
}
+ window.set_backing_store_visible_size(visible_size);
if (flush_immediately)
window.invalidate(false);
diff --git a/Userland/Services/WindowServer/ConnectionFromClient.h b/Userland/Services/WindowServer/ConnectionFromClient.h
index cb0b8e2717..85cb6b7ecb 100644
--- a/Userland/Services/WindowServer/ConnectionFromClient.h
+++ b/Userland/Services/WindowServer/ConnectionFromClient.h
@@ -121,7 +121,7 @@ private:
virtual void did_finish_painting(i32, Vector<Gfx::IntRect> const&) override;
virtual void set_global_mouse_tracking(bool) override;
virtual void set_window_opacity(i32, float) override;
- virtual void set_window_backing_store(i32, i32, i32, IPC::File const&, i32, bool, Gfx::IntSize, bool) override;
+ virtual void set_window_backing_store(i32, i32, i32, IPC::File const&, i32, bool, Gfx::IntSize, Gfx::IntSize, bool) override;
virtual void set_window_has_alpha_channel(i32, bool) override;
virtual void set_window_alpha_hit_threshold(i32, float) override;
virtual void move_window_to_front(i32) override;
diff --git a/Userland/Services/WindowServer/Window.cpp b/Userland/Services/WindowServer/Window.cpp
index c7068bc785..79bda6499e 100644
--- a/Userland/Services/WindowServer/Window.cpp
+++ b/Userland/Services/WindowServer/Window.cpp
@@ -153,6 +153,7 @@ void Window::set_rect(Gfx::IntRect const& rect)
} else if (is_internal() && (!m_backing_store || old_rect.size() != rect.size())) {
auto format = has_alpha_channel() ? Gfx::BitmapFormat::BGRA8888 : Gfx::BitmapFormat::BGRx8888;
m_backing_store = Gfx::Bitmap::try_create(format, m_rect.size()).release_value_but_fixme_should_propagate_errors();
+ m_backing_store_visible_size = m_rect.size();
}
if (m_floating_rect.is_empty())
diff --git a/Userland/Services/WindowServer/Window.h b/Userland/Services/WindowServer/Window.h
index 36eb2c5a88..abe0a76bf6 100644
--- a/Userland/Services/WindowServer/Window.h
+++ b/Userland/Services/WindowServer/Window.h
@@ -242,6 +242,9 @@ public:
m_backing_store_serial = serial;
}
+ Gfx::IntSize backing_store_visible_size() const { return m_backing_store_visible_size; }
+ void set_backing_store_visible_size(Gfx::IntSize visible_size) { m_backing_store_visible_size = visible_size; }
+
void swap_backing_stores()
{
swap(m_backing_store, m_last_backing_store);
@@ -434,6 +437,7 @@ private:
bool m_occluded { false };
RefPtr<Gfx::Bitmap> m_backing_store;
RefPtr<Gfx::Bitmap> m_last_backing_store;
+ Gfx::IntSize m_backing_store_visible_size {};
i32 m_backing_store_serial { -1 };
i32 m_last_backing_store_serial { -1 };
int m_window_id { -1 };
diff --git a/Userland/Services/WindowServer/WindowServer.ipc b/Userland/Services/WindowServer/WindowServer.ipc
index d159af695b..f40e4e1394 100644
--- a/Userland/Services/WindowServer/WindowServer.ipc
+++ b/Userland/Services/WindowServer/WindowServer.ipc
@@ -98,7 +98,7 @@ endpoint WindowServer
set_window_alpha_hit_threshold(i32 window_id, float threshold) =|
- set_window_backing_store(i32 window_id, i32 bpp, i32 pitch, IPC::File anon_file, i32 serial, bool has_alpha_channel, Gfx::IntSize size, bool flush_immediately) => ()
+ set_window_backing_store(i32 window_id, i32 bpp, i32 pitch, IPC::File anon_file, i32 serial, bool has_alpha_channel, Gfx::IntSize size, Gfx::IntSize visible_size, bool flush_immediately) => ()
set_window_has_alpha_channel(i32 window_id, bool has_alpha_channel) =|
move_window_to_front(i32 window_id) =|