WindowServer: Change rendering drag&drop to use the Overlay class

This enables flicker-free rendering.

Userland/Services/WindowServer/ClientConnection.cpp

@@ -58,6 +58,10 @@ ClientConnection::ClientConnection(NonnullRefPtr<Core::LocalSocket> client_socke
 
 ClientConnection::~ClientConnection()
 {
+    auto& wm = WindowManager::the();
+    if (wm.dnd_client() == this)
+        wm.end_dnd_drag();
+
     if (m_has_display_link)
         Compositor::the().decrement_display_link_count({});
 

Userland/Services/WindowServer/Compositor.cpp

@@ -156,22 +156,6 @@ void Compositor::compose()
     VERIFY(!m_overlay_rects_changed);
 
     auto dirty_screen_rects = move(m_dirty_screen_rects);
-    auto* dnd_client = wm.dnd_client();
-    if (!m_last_dnd_rect.is_empty() || (m_invalidated_cursor && dnd_client)) {
-        Screen::for_each([&](auto& screen) {
-            if (!m_last_dnd_rect.is_empty()) {
-                auto rect = m_last_dnd_rect.intersected(screen.rect());
-                if (!rect.is_empty())
-                    dirty_screen_rects.add(rect);
-            }
-            if (m_invalidated_cursor && dnd_client) {
-                auto rect = wm.dnd_rect().intersected(screen.rect());
-                if (!rect.is_empty())
-                    dirty_screen_rects.add(rect);
-            }
-            return IterationDecision::Continue;
-        });
-    }
 
     // Mark window regions as dirty that need to be re-rendered
     wm.window_stack().for_each_visible_window_from_back_to_front([&](Window& window) {
@@ -543,60 +527,6 @@ void Compositor::compose()
     m_invalidated_window = false;
     m_invalidated_cursor = false;
 
-    if (wm.dnd_client()) {
-        auto dnd_rect = wm.dnd_rect();
-
-        Screen::for_each([&](auto& screen) {
-            auto screen_rect = screen.rect();
-            auto render_dnd_rect = screen_rect.intersected(dnd_rect);
-            if (render_dnd_rect.is_empty())
-                return IterationDecision::Continue;
-            auto& screen_data = m_screen_data[screen.index()];
-            auto& back_painter = *screen_data.m_back_painter;
-
-            // TODO: render once into a backing bitmap, then just blit...
-            auto render_dnd = [&]() {
-                back_painter.fill_rect(dnd_rect, wm.palette().selection().with_alpha(200));
-                back_painter.draw_rect(dnd_rect, wm.palette().selection());
-                if (!wm.dnd_text().is_empty()) {
-                    auto text_rect = dnd_rect;
-                    if (wm.dnd_bitmap())
-                        text_rect.translate_by(wm.dnd_bitmap()->width() + 8, 0);
-                    back_painter.draw_text(text_rect, wm.dnd_text(), Gfx::TextAlignment::CenterLeft, wm.palette().selection_text());
-                }
-                if (wm.dnd_bitmap()) {
-                    back_painter.blit(dnd_rect.top_left().translated(4, 4), *wm.dnd_bitmap(), wm.dnd_bitmap()->rect());
-                }
-            };
-
-            dirty_screen_rects.for_each_intersected(dnd_rect, [&](const Gfx::IntRect& render_rect) {
-                auto screen_render_rect = render_rect.intersected(screen_rect);
-                if (screen_render_rect.is_empty())
-                    return IterationDecision::Continue;
-                Gfx::PainterStateSaver saver(back_painter);
-                back_painter.add_clip_rect(screen_render_rect);
-                render_dnd();
-                return IterationDecision::Continue;
-            });
-            screen_data.m_flush_transparent_rects.for_each_intersected(dnd_rect, [&](const Gfx::IntRect& render_rect) {
-                auto screen_render_rect = render_rect.intersected(screen_rect);
-                if (screen_render_rect.is_empty())
-                    return IterationDecision::Continue;
-                Gfx::PainterStateSaver saver(back_painter);
-                back_painter.add_clip_rect(screen_render_rect);
-                render_dnd();
-                return IterationDecision::Continue;
-            });
-            m_last_dnd_rect = dnd_rect;
-            return IterationDecision::Continue;
-        });
-    } else {
-        if (!m_last_dnd_rect.is_empty()) {
-            invalidate_screen(m_last_dnd_rect);
-            m_last_dnd_rect = {};
-        }
-    }
-
     bool did_render_animation = false;
     Screen::for_each([&](auto& screen) {
         auto& screen_data = m_screen_data[screen.index()];

Userland/Services/WindowServer/Compositor.h

@@ -181,8 +181,6 @@ private:
     Gfx::DisjointRectSet m_dirty_screen_rects;
     Gfx::DisjointRectSet m_opaque_wallpaper_rects;
 
-    Gfx::IntRect m_last_dnd_rect;
-
     String m_wallpaper_path { "" };
     WallpaperMode m_wallpaper_mode { WallpaperMode::Unchecked };
     RefPtr<Gfx::Bitmap> m_wallpaper;

Userland/Services/WindowServer/Overlays.cpp

@@ -41,11 +41,12 @@ void Overlay::set_rect(Gfx::IntRect const& rect)
 {
     if (m_rect == rect)
         return;
+    auto previous_rect = m_rect;
     m_rect = rect;
     invalidate();
     if (is_enabled())
         Compositor::the().overlay_rects_changed();
-    rect_changed();
+    rect_changed(previous_rect);
 }
 
 BitmapOverlay::BitmapOverlay()
@@ -53,10 +54,11 @@ BitmapOverlay::BitmapOverlay()
     clear_bitmaps();
 }
 
-void BitmapOverlay::rect_changed()
+void BitmapOverlay::rect_changed(Gfx::IntRect const& previous_rect)
 {
+    if (rect().size() != previous_rect.size())
         clear_bitmaps();
-    Overlay::rect_changed();
+    Overlay::rect_changed(previous_rect);
 }
 
 void BitmapOverlay::clear_bitmaps()
@@ -84,8 +86,9 @@ RectangularOverlay::RectangularOverlay()
     clear_bitmaps();
 }
 
-void RectangularOverlay::rect_changed()
+void RectangularOverlay::rect_changed(Gfx::IntRect const& previous_rect)
 {
+    if (m_rerender_on_location_change || rect().size() != previous_rect.size())
         clear_bitmaps();
 }
 
@@ -208,6 +211,7 @@ Gfx::IntRect ScreenNumberOverlay::calculate_content_rect_for_screen(Screen& scre
 WindowGeometryOverlay::WindowGeometryOverlay(Window& window)
     : m_window(window)
 {
+    rerender_on_location_change(true);
     update_rect();
 }
 
@@ -253,4 +257,49 @@ void WindowGeometryOverlay::window_rect_changed()
     invalidate();
 }
 
+DndOverlay::DndOverlay(String const& text, Gfx::Bitmap const* bitmap)
+    : m_bitmap(bitmap)
+    , m_text(text)
+{
+    update_rect();
+}
+
+Gfx::Font const& DndOverlay::font()
+{
+    return WindowManager::the().font();
+}
+
+void DndOverlay::update_rect()
+{
+    int bitmap_width = m_bitmap ? m_bitmap->width() : 0;
+    int bitmap_height = m_bitmap ? m_bitmap->height() : 0;
+    auto& font = this->font();
+    int width = font.width(m_text) + bitmap_width;
+    int height = max((int)font.glyph_height(), bitmap_height);
+    auto location = Compositor::the().current_cursor_rect().center().translated(8, 8);
+    set_rect(Gfx::IntRect(location, { width, height }).inflated(16, 8));
+}
+
+RefPtr<Gfx::Bitmap> DndOverlay::create_bitmap(int scale_factor)
+{
+    auto new_bitmap = Gfx::Bitmap::create(Gfx::BitmapFormat::BGRA8888, rect().size(), scale_factor);
+    if (!new_bitmap)
+        return {};
+
+    auto& wm = WindowManager::the();
+    Gfx::Painter bitmap_painter(*new_bitmap);
+    auto bitmap_rect = new_bitmap->rect();
+    bitmap_painter.fill_rect(bitmap_rect, wm.palette().selection().with_alpha(200));
+    bitmap_painter.draw_rect(bitmap_rect, wm.palette().selection());
+    if (!m_text.is_empty()) {
+        auto text_rect = bitmap_rect;
+        if (m_bitmap)
+            text_rect.translate_by(m_bitmap->width() + 8, 0);
+        bitmap_painter.draw_text(text_rect, m_text, Gfx::TextAlignment::CenterLeft, wm.palette().selection_text());
+    }
+    if (m_bitmap)
+        bitmap_painter.blit(bitmap_rect.top_left().translated(4, 4), *m_bitmap, m_bitmap->rect());
+    return new_bitmap;
+}
+
 }

Userland/Services/WindowServer/Overlays.h

@@ -26,6 +26,7 @@ public:
 
     enum class ZOrder {
         WindowGeometry,
+        Dnd,
         ScreenNumber,
     };
     [[nodiscard]] virtual ZOrder zorder() const = 0;
@@ -39,7 +40,7 @@ public:
 
     virtual void theme_changed()
     {
-        rect_changed();
+        rect_changed(m_rect);
     }
 
     bool invalidate();
@@ -49,7 +50,7 @@ protected:
 
     void set_rect(Gfx::IntRect const&);
 
-    virtual void rect_changed() {};
+    virtual void rect_changed(Gfx::IntRect const&) {};
 
 private:
     void clear_invalidated() { m_invalidated = false; }
@@ -76,7 +77,7 @@ protected:
     BitmapOverlay();
 
     void clear_bitmaps();
-    virtual void rect_changed() override;
+    virtual void rect_changed(Gfx::IntRect const&) override;
 
 private:
     RefPtr<MultiScaleBitmaps> m_bitmaps;
@@ -97,10 +98,16 @@ protected:
     void set_content_rect(Gfx::IntRect const&);
 
     void clear_bitmaps();
-    virtual void rect_changed() override;
+    virtual void rect_changed(Gfx::IntRect const&) override;
+
+    void rerender_on_location_change(bool value)
+    {
+        m_rerender_on_location_change = value;
+    }
 
 private:
     RefPtr<MultiScaleBitmaps> m_rendered_bitmaps;
+    bool m_rerender_on_location_change { false };
 };
 
 class ScreenNumberOverlay : public RectangularOverlay {
@@ -143,4 +150,25 @@ private:
     Gfx::IntRect m_label_rect;
 };
 
+class DndOverlay : public BitmapOverlay {
+public:
+    DndOverlay(String const&, Gfx::Bitmap const*);
+
+    void cursor_moved()
+    {
+        update_rect();
+    }
+
+    virtual ZOrder zorder() const override { return ZOrder::Dnd; }
+    virtual RefPtr<Gfx::Bitmap> create_bitmap(int) override;
+
+private:
+    Gfx::Font const& font();
+    void update_rect();
+
+    RefPtr<Gfx::Bitmap> m_bitmap;
+    String m_text;
+    Gfx::IntRect m_label_rect;
+};
+
 }

Userland/Services/WindowServer/WindowManager.cpp

@@ -776,6 +776,8 @@ bool WindowManager::process_ongoing_drag(MouseEvent& event)
         return false;
 
     if (event.type() == Event::MouseMove) {
+        m_dnd_overlay->cursor_moved();
+
         // We didn't let go of the drag yet, see if we should send some drag move events..
         m_window_stack.for_each_visible_window_from_front_to_back([&](Window& window) {
             if (!window.rect().contains(event.position()))
@@ -1523,7 +1525,8 @@ void WindowManager::start_dnd_drag(ClientConnection& client, String const& text,
     VERIFY(!m_dnd_client);
     m_dnd_client = client;
     m_dnd_text = text;
-    m_dnd_bitmap = bitmap;
+    m_dnd_overlay = Compositor::the().create_overlay<DndOverlay>(text, bitmap);
+    m_dnd_overlay->set_enabled(true);
     m_dnd_mime_data = mime_data;
     Compositor::the().invalidate_cursor();
     m_active_input_tracking_window = nullptr;
@@ -1535,17 +1538,7 @@ void WindowManager::end_dnd_drag()
     Compositor::the().invalidate_cursor();
     m_dnd_client = nullptr;
     m_dnd_text = {};
-    m_dnd_bitmap = nullptr;
+    m_dnd_overlay = nullptr;
-}
-
-Gfx::IntRect WindowManager::dnd_rect() const
-{
-    int bitmap_width = m_dnd_bitmap ? m_dnd_bitmap->width() : 0;
-    int bitmap_height = m_dnd_bitmap ? m_dnd_bitmap->height() : 0;
-    int width = font().width(m_dnd_text) + bitmap_width;
-    int height = max((int)font().glyph_height(), bitmap_height);
-    auto location = Compositor::the().current_cursor_rect().center().translated(8, 8);
-    return Gfx::IntRect(location, { width, height }).inflated(16, 8);
 }
 
 void WindowManager::invalidate_after_theme_or_font_change()

Userland/Services/WindowServer/WindowManager.h

@@ -37,6 +37,7 @@ class Window;
 class ClientConnection;
 class WindowSwitcher;
 class Button;
+class DndOverlay;
 class WindowGeometryOverlay;
 
 enum class ResizeDirection {
@@ -84,10 +85,7 @@ public:
     Gfx::IntRect maximized_window_rect(Window const&, bool relative_to_window_screen = false) const;
 
     ClientConnection const* dnd_client() const { return m_dnd_client.ptr(); }
-    String const& dnd_text() const { return m_dnd_text; }
     Core::MimeData const& dnd_mime_data() const { return *m_dnd_mime_data; }
-    Gfx::Bitmap const* dnd_bitmap() const { return m_dnd_bitmap; }
-    Gfx::IntRect dnd_rect() const;
 
     void start_dnd_drag(ClientConnection&, String const& text, Gfx::Bitmap const*, Core::MimeData const&);
     void end_dnd_drag();
@@ -345,10 +343,11 @@ private:
 
     RefPtr<Core::ConfigFile> m_config;
 
+    OwnPtr<DndOverlay> m_dnd_overlay;
     WeakPtr<ClientConnection> m_dnd_client;
     String m_dnd_text;
+
     RefPtr<Core::MimeData> m_dnd_mime_data;
-    RefPtr<Gfx::Bitmap> m_dnd_bitmap;
 };
 
 template<typename Callback>