Jelajahi Sumber

fix(gtk): clear render cache on layout rebuild

Prevent potential memory leaks or stale pointer usage by clearing the per-window render cache whenever the window layout is rebuilt.
Bernardo Magri 1 bulan lalu
induk
melakukan
2eff2727db
1 mengubah file dengan 71 tambahan dan 40 penghapusan
  1. 71 40
      src/gtk_editor.cpp

+ 71 - 40
src/gtk_editor.cpp

@@ -166,6 +166,18 @@ private:
     double line_height_ = 0;
     double ascent_ = 0;
     
+    // Render Caching
+    struct LineCache {
+        std::string text;
+        Glib::RefPtr<Pango::Layout> layout;
+    };
+    
+    struct WindowCache {
+        std::vector<LineCache> lines;
+    };
+    
+    std::map<Window*, WindowCache> render_cache_;
+    
     // Layout padding
     static constexpr double PADDING_LEFT = 8.0;
     static constexpr double PADDING_TOP = 8.0;
@@ -363,6 +375,9 @@ protected:
         
         // Clear the drawing area reference since we're rebuilding
         drawing_area_ = nullptr;
+        
+        // Clear render cache to prevent stale window pointers
+        render_cache_.clear();
 
         // Create new layout based on the tree
         content_widget_ = create_widget_for_layout_node(root_layout);
@@ -382,9 +397,9 @@ protected:
             // Set up drawing for this specific window
             // Use a weak reference to the window to avoid crashes if the layout is rebuilt
             std::weak_ptr<Window> weak_window = node->window;
-            drawing_area->set_draw_func([this, weak_window](const Cairo::RefPtr<Cairo::Context>& cr, int width, int height) {
+            drawing_area->set_draw_func([this, weak_window, drawing_area](const Cairo::RefPtr<Cairo::Context>& cr, int width, int height) {
                 if (auto window = weak_window.lock()) {
-                    draw_window(cr, width, height, window);
+                    draw_window(cr, width, height, window, drawing_area);
                 }
             });
             
@@ -461,7 +476,7 @@ protected:
     }
 
     // Draw a specific window (factored out from on_draw)
-    void draw_window(const Cairo::RefPtr<Cairo::Context>& cr, int width, int height, std::shared_ptr<Window> window) {
+    void draw_window(const Cairo::RefPtr<Cairo::Context>& cr, int width, int height, std::shared_ptr<Window> window, Gtk::DrawingArea* widget) {
         if (!core_ || !window) return;
 
         const auto cursor = window->cursor();
@@ -473,28 +488,24 @@ protected:
         cr->set_source_rgb(bg.r / 255.0, bg.g / 255.0, bg.b / 255.0);
         cr->paint();
 
-        // Create Pango layout - create context directly from Cairo
-        auto layout = Pango::Layout::create(cr);
+        // Create temporary layout for metrics if needed (using widget context is better but this works with cache)
+        // Note: We use widget->create_pango_layout() for cached lines to persist them
         
-        // Font configuration (cached)
+        // Ensure metrics are initialized
         if (!font_initialized_) {
-            font_desc_ = Pango::FontDescription("Monospace 12");
-            layout->set_font_description(font_desc_);
-
-            // Get font metrics
-            Pango::FontMetrics metrics = layout->get_context()->get_metrics(font_desc_);
-            line_height_ = (double)metrics.get_height() / PANGO_SCALE;
-            ascent_ = (double)metrics.get_ascent() / PANGO_SCALE;
-            
-            // Measure character width (for a single 'm' character)
-            layout->set_text("m");
-            Pango::Rectangle ink_rect, logical_rect;
-            layout->get_pixel_extents(ink_rect, logical_rect);
-            char_width_ = (double)logical_rect.get_width(); // Already in pixels
-            
-            font_initialized_ = true;
-        } else {
-            layout->set_font_description(font_desc_);
+             auto layout = widget->create_pango_layout("m");
+             font_desc_ = Pango::FontDescription("Monospace 12");
+             layout->set_font_description(font_desc_);
+             
+             Pango::FontMetrics metrics = layout->get_context()->get_metrics(font_desc_);
+             line_height_ = (double)metrics.get_height() / PANGO_SCALE;
+             ascent_ = (double)metrics.get_ascent() / PANGO_SCALE;
+             
+             Pango::Rectangle ink_rect, logical_rect;
+             layout->get_pixel_extents(ink_rect, logical_rect);
+             char_width_ = (double)logical_rect.get_width();
+             
+             font_initialized_ = true;
         }
 
         // Update window's viewport size based on actual font metrics and padding
@@ -513,31 +524,50 @@ protected:
         Color fg = theme ? theme->get_fg_color(ThemeElement::Normal) : Color(255, 255, 255);
         cr->set_source_rgb(fg.r / 255.0, fg.g / 255.0, fg.b / 255.0);
 
-        // Render visible lines
+        // Render visible lines using cache
         const auto& buffer = window->buffer();
         auto [start_line, end_line] = window->visible_line_range();
         int horizontal_offset = window->viewport().horizontal_offset;
+        
+        // Get or create window cache
+        WindowCache& cache = render_cache_[window.get()];
+        if (cache.lines.size() < static_cast<size_t>(editor_lines)) {
+            cache.lines.resize(editor_lines);
+        }
 
-        for (int screen_y = 0; screen_y < editor_lines && start_line + screen_y < end_line; ++screen_y) {
+        for (int screen_y = 0; screen_y < editor_lines; ++screen_y) {
+            if (start_line + screen_y >= end_line) break;
+            
             size_t buffer_line_idx = start_line + screen_y;
             const auto& line_text = buffer.line(buffer_line_idx);
 
-            // Apply horizontal scrolling - show only the visible portion of the line
+            // Apply horizontal scrolling
             std::string visible_text;
             if (horizontal_offset < static_cast<int>(line_text.length())) {
                 visible_text = line_text.substr(horizontal_offset);
             }
             
-            layout->set_text(visible_text);
+            // Check cache
+            LineCache& line_cache = cache.lines[screen_y];
+            if (!line_cache.layout || line_cache.text != visible_text) {
+                // Update cache
+                if (!line_cache.layout) {
+                    line_cache.layout = widget->create_pango_layout(visible_text);
+                    line_cache.layout->set_font_description(font_desc_);
+                } else {
+                    line_cache.layout->set_text(visible_text);
+                }
+                line_cache.text = visible_text;
+            }
 
             // Render text at proper position
             double text_x = PADDING_LEFT;
             double text_y = PADDING_TOP + screen_y * line_height_;
             cr->move_to(text_x, text_y);
-            layout->show_in_cairo_context(cr);
+            line_cache.layout->show_in_cairo_context(cr);
         }
 
-        // Render Cursor if this is the active window
+        // Render Cursor (reuse last layout if possible, or create temp)
         bool should_show_cursor = (window == core_->active_window()) && cursor_visible_;
         if (should_show_cursor && 
             cursor.line >= static_cast<size_t>(start_line) && 
@@ -545,40 +575,41 @@ protected:
             
             int screen_y = cursor.line - start_line;
             double cursor_y = PADDING_TOP + screen_y * line_height_;
-            
-            // Calculate the exact X position
             double cursor_screen_x = PADDING_LEFT + (static_cast<int>(cursor.column) - horizontal_offset) * char_width_;
             
-            // Only render cursor if it's visible horizontally
             if (cursor_screen_x >= PADDING_LEFT && cursor_screen_x < (width - PADDING_RIGHT)) {
-                // Get the character under cursor for rendering with inverted colors
                 size_t buffer_line_idx = cursor.line;
                 const auto& cursor_line_text = buffer.line(buffer_line_idx);
                 char cursor_char = (cursor.column < cursor_line_text.length()) ? cursor_line_text[cursor.column] : ' ';
                 
-                // Draw block cursor background (inverted background color)
                 Color cursor_bg = theme ? theme->get_fg_color(ThemeElement::Normal) : Color(255, 255, 255);
                 cr->set_source_rgb(cursor_bg.r / 255.0, cursor_bg.g / 255.0, cursor_bg.b / 255.0);
                 cr->rectangle(cursor_screen_x, cursor_y, char_width_, line_height_);
                 cr->fill();
                 
-                // Draw the character with inverted color (background color as foreground)
                 if (cursor_char != '\0' && cursor_char != ' ') {
                     Color cursor_fg = theme ? theme->get_bg_color(ThemeElement::Background) : Color(0, 0, 0);
                     cr->set_source_rgb(cursor_fg.r / 255.0, cursor_fg.g / 255.0, cursor_fg.b / 255.0);
                     
-                    layout->set_text(std::string(1, cursor_char));
+                    // Just create a temp layout for the cursor char - it's small and changes often
+                    auto cursor_layout = Pango::Layout::create(cr);
+                    cursor_layout->set_font_description(font_desc_);
+                    cursor_layout->set_text(std::string(1, cursor_char));
+                    
                     cr->move_to(cursor_screen_x, cursor_y);
-                    layout->show_in_cairo_context(cr);
+                    cursor_layout->show_in_cairo_context(cr);
                 }
             }
         }
+        
+        // Use a temporary layout for modeline/minibuffer as they are dynamic and not part of the main text grid
+        auto temp_layout = Pango::Layout::create(cr);
+        temp_layout->set_font_description(font_desc_);
 
-        // Render modeline for all windows, but minibuffer only for the main window
-        render_modeline_for_window(cr, width, height, layout, window);
+        render_modeline_for_window(cr, width, height, temp_layout, window);
         
         if (is_main_window) {
-            render_minibuffer(cr, width, height, layout);
+            render_minibuffer(cr, width, height, temp_layout);
         }
     }