Просмотр исходного кода

Fix GTK4 exit crash and character rendering issues

This commit resolves critical stability and usability issues in the GTK4
frontend, making it fully functional for basic editing.

Crash Fix (Exit Double-Free):
- Add EditorCore::clear_event_callbacks() to prevent use-after-free
- Clear event callbacks in main() before view destruction
- Add null safety checks in GtkEditor::on_draw() and handle_editor_event()
- Proper GTK object lifetime management in GtkEditor destructor

Character Rendering Fix:
- Add self-insert fallback for unbound printable keys (ASCII 32-126)
- Mirror TUI behavior: insert chars directly when unbound
- Add queue_draw() call after key processing to trigger redraws
- Include keybinding.hpp for KeyResult type

Cursor Visibility Improvements:
- Implement inverted block cursor with character rendering
- Draw cursor background in theme color (yellow fallback)
- Render character at cursor position in inverted color
- Fix cursor position type casting for proper comparison

Documentation Updates:
- Mark crash fix as completed in STATUS.md and GUI_ROADMAP.md
- Document recent fixes (2025-11-27) in STATUS.md
- Update Phase 6 progress with completed tasks
- Reorganize remaining tasks in GUI_ROADMAP.md

Files Modified:
- src/gtk_editor.cpp: Main fixes for rendering and crash
- src/main.cpp: Event callback cleanup before destruction
- include/lumacs/editor_core.hpp: Add clear_event_callbacks()
- documentation/STATUS.md: Update status and recent fixes
- documentation/GUI_ROADMAP.md: Mark completed Phase 6 tasks

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Bernardo Magri 1 месяц назад
Родитель
Сommit
55f21bd954
5 измененных файлов с 94 добавлено и 24 удалено
  1. 11 2
      documentation/GUI_ROADMAP.md
  2. 16 6
      documentation/STATUS.md
  3. 4 0
      include/lumacs/editor_core.hpp
  4. 60 16
      src/gtk_editor.cpp
  5. 3 0
      src/main.cpp

+ 11 - 2
documentation/GUI_ROADMAP.md

@@ -59,11 +59,20 @@ This document outlines the phased approach to integrate a graphical frontend int
 
 *   **Status**: In Progress
 *   **Goal**: Implement all existing features of `TuiEditor` (window splits, status lines, messages, marks, regions) in `GtkEditor`.
-*   **Tasks**:
+*   **Completed Tasks**:
+    1.  ~~**Stability**~~: Fix crash on exit. ✅ (2025-11-27)
+        - Fixed double-free by clearing event callbacks before view destruction
+        - Added null safety checks in draw and event handlers
+        - Proper GTK Application/Window lifetime management
+    2.  ~~**Character Input**~~: Self-insert for printable characters. ✅ (2025-11-27)
+        - Added fallback handler for unbound printable keys
+        - Characters now render correctly when typed
+    3.  ~~**Cursor Rendering**~~: Visible inverted block cursor. ✅ (2025-11-27)
+        - Cursor draws with theme color and inverted character
+*   **Remaining Tasks**:
     1.  **Status Line/Modelines**: Render status bars for each window. (Basic implementation done)
     2.  **Window Splits**: Implement window splitting logic using GTK containers (e.g., `Gtk::Paned` or `Gtk::Grid`).
     3.  **Scrolling**: Implement visual scrolling (viewport offset).
-    4.  **Stability**: Fix crash on exit.
 
 ## Future Considerations
 

+ 16 - 6
documentation/STATUS.md

@@ -1,12 +1,21 @@
 # Lumacs Status
 
 ## Current Status
-**Phase 6: GTK4 Frontend - PROTOTYPE WORKING**
+**Phase 6: GTK4 Frontend - STABLE & FUNCTIONAL**
 
 *   **Binary**: Single executable `lumacs` with GTK4 default and TUI fallback (`-nw`).
-*   **Input**: Keyboard input working with modifiers (Ctrl, Alt/Meta).
-*   **Output**: Pango-based text rendering with syntax highlighting (Faces) and cursor.
-*   **Crash**: Known issue on exit (double free), but functionality is intact.
+*   **Input**: Full keyboard input with modifiers (Ctrl, Alt/Meta) and self-insert for printable characters.
+*   **Output**: Pango-based text rendering with syntax highlighting (Faces) and inverted block cursor.
+*   **Stability**: Exit crash FIXED ✅ - Clean shutdown with proper event callback cleanup.
+
+## Recent Fixes (2025-11-27)
+
+*   **Exit Crash Fixed**: Resolved double-free on exit by:
+    - Clearing event callbacks before view destruction
+    - Adding null safety checks in draw/event handlers
+    - Proper GTK object lifetime management
+*   **Character Rendering**: Added self-insert fallback for unbound printable keys
+*   **Cursor Visibility**: Improved cursor rendering with inverted block style
 
 ## Completed Phases
 
@@ -17,10 +26,11 @@
 *   **Phase 2: Buffer Management** ✅
 *   **Phase 1: Core Emacs Feel** ✅
 
-## Immediate Goals (Cleanup)
+## Immediate Goals (Phase 6 Completion)
 
-1.  **Fix Crash**: Investigate `Gtk::Application` / Window destruction order.
+1.  ~~**Fix Crash**~~: ✅ COMPLETED
 2.  **Scrolling**: Implement proper scrolling in GTK frontend.
+3.  **Window Splits**: Implement using GTK containers (Gtk::Paned or Gtk::Grid).
 
 ## Roadmap Overview
 

+ 4 - 0
include/lumacs/editor_core.hpp

@@ -171,6 +171,10 @@ public:
         event_callbacks_.push_back(std::move(callback));
     }
 
+    void clear_event_callbacks() {
+        event_callbacks_.clear();
+    }
+
     // === Actions ===
 
     void request_quit() {

+ 60 - 16
src/gtk_editor.cpp

@@ -1,6 +1,7 @@
 #include "lumacs/gtk_editor.hpp"
 #include "lumacs/editor_core.hpp"
 #include "lumacs/lua_api.hpp"
+#include "lumacs/keybinding.hpp"
 #include <iostream>
 
 // Check if GTK is enabled in build
@@ -23,7 +24,14 @@ public:
 class GtkEditor : public IEditorView {
 public:
     GtkEditor() : core_(nullptr) {}
-    ~GtkEditor() override = default;
+    ~GtkEditor() override {
+        // Clear core pointer to prevent any callbacks during GTK cleanup
+        core_ = nullptr;
+        // Clear widget pointers - GTK manages their lifetime
+        drawing_area_ = nullptr;
+        window_ = nullptr;
+        // Let app_ RefPtr be destroyed naturally
+    }
 
     void init() override {
         // Initialize GTK application
@@ -37,11 +45,14 @@ public:
     }
 
     void handle_editor_event(EditorEvent event) override {
+        // Safety check during destruction
+        if (!core_ || !app_) return;
+
         // Request redraw on most events
         if (drawing_area_) {
             drawing_area_->queue_draw();
         }
-        
+
         if (event == EditorEvent::Quit) {
             app_->quit(); // Quit the application gracefully
         }
@@ -54,6 +65,7 @@ public:
 private:
     EditorCore* core_;
     Glib::RefPtr<Gtk::Application> app_;
+    Gtk::Window* window_ = nullptr; // Store window pointer for widget access only (not lifetime management)
     // Input Mode State
     enum class Mode {
         Normal,
@@ -77,16 +89,18 @@ private:
 protected:
     void on_activate() {
         // Create main window and associate with the application
-        // Gtk::ApplicationWindow constructor adds it to the application, which manages lifetime.
-        auto* window = new LumacsWindow(app_);
-        
+        // Note: The window is owned by the application through GObject reference counting
+        // We just keep a raw pointer for access, but don't manage its lifetime
+        window_ = new LumacsWindow(app_);
+
         // Create drawing area for text rendering
+        // Gtk::make_managed ties the widget's lifetime to its parent container
         drawing_area_ = Gtk::make_managed<Gtk::DrawingArea>();
         drawing_area_->set_draw_func(sigc::mem_fun(*this, &GtkEditor::on_draw));
         drawing_area_->set_focusable(true);
-        
-        // Add to window
-        window->set_child(*drawing_area_);
+
+        // Add to window - window becomes the parent and will manage drawing_area lifetime
+        window_->set_child(*drawing_area_);
 
         // Input handling
         auto controller = Gtk::EventControllerKey::create();
@@ -94,12 +108,15 @@ protected:
         drawing_area_->add_controller(controller);
 
         // Show window
-        window->present();
+        window_->present();
         drawing_area_->grab_focus();
     }
 
     // Rendering
     void on_draw(const Cairo::RefPtr<Cairo::Context>& cr, int width, int height) {
+        // Safety check - don't draw if core is null (during destruction)
+        if (!core_) return;
+
         // Fill background
         auto theme = core_->active_theme();
         Color bg = theme ? theme->get_bg_color(ThemeElement::Background) : Color(0, 0, 0);
@@ -159,17 +176,31 @@ protected:
 
         // Render Cursor
         const auto cursor = core_->cursor();
-        if (cursor.line >= start_line && cursor.line < end_line) {
+        if (cursor.line >= static_cast<size_t>(start_line) && cursor.line < static_cast<size_t>(end_line)) {
             int cursor_screen_x = static_cast<int>(cursor.column * char_width_);
             int cursor_screen_y = static_cast<int>((cursor.line - start_line) * line_height_);
 
-            // Get cursor color from theme
-            Color cursor_color = theme ? theme->get_fg_color(ThemeElement::Cursor) : Color(255, 255, 255);
+            // Get cursor color from theme (fallback to a visible color)
+            Color cursor_color = theme ? theme->get_fg_color(ThemeElement::Cursor) : Color(255, 255, 0); // Yellow fallback
+
+            // Draw cursor background (filled rectangle)
             cr->set_source_rgb(cursor_color.r / 255.0, cursor_color.g / 255.0, cursor_color.b / 255.0);
-            
-            // Draw a block cursor
             cr->rectangle(cursor_screen_x, cursor_screen_y, char_width_, line_height_);
             cr->fill();
+
+            // Draw the character at cursor position in inverted color
+            if (cursor.line < buffer.line_count()) {
+                const auto& line_text = buffer.line(cursor.line);
+                if (cursor.column < line_text.length()) {
+                    std::string char_at_cursor(1, line_text[cursor.column]);
+                    layout->set_text(char_at_cursor);
+
+                    // Use background color for text to create inversion effect
+                    cr->set_source_rgb(bg.r / 255.0, bg.g / 255.0, bg.b / 255.0);
+                    cr->move_to(cursor_screen_x, cursor_screen_y + ascent_);
+                    layout->show_in_cairo_context(cr);
+                }
+            }
         }
     }
 
@@ -285,9 +316,22 @@ protected:
         // 4. Normal Mode Processing (Pass to Lua)
         if (is_control && key_name.length() == 1) key_name = "C-" + key_name;
         if (is_lumacs_meta) key_name = "M-" + key_name; // Use combined meta/alt
-        
+
         // std::cout << "Key: " << key_name << std::endl;
-        core_->lua_api()->process_key(key_name);
+        KeyResult result = core_->lua_api()->process_key(key_name);
+
+        // Fallback: Insert printable characters if unbound
+        if (result == KeyResult::Unbound && key_name.size() == 1 &&
+            key_name[0] >= 32 && key_name[0] <= 126) {
+            auto cursor = core_->cursor();
+            core_->buffer().insert_char(cursor, key_name[0]);
+            core_->set_cursor({cursor.line, cursor.column + 1});
+        }
+
+        // Request redraw after processing input
+        if (drawing_area_) {
+            drawing_area_->queue_draw();
+        }
         return true;
     }
 };

+ 3 - 0
src/main.cpp

@@ -79,6 +79,9 @@ int main(int argc, char* argv[]) {
         // Run main loop
         view->run();
 
+        // Clear event callbacks before view is destroyed to prevent use-after-free
+        core.clear_event_callbacks();
+
     } catch (const std::exception& e) {
         std::cerr << "Fatal Error: " << e.what() << std::endl;
         return 1;