GitHub #326: browser scrolling should be much improved

Code now convinces browser to rebuild when the tree changes by UI.
When widgets are move, the current widget should always be visible.
It's the responsibility of the UI callback to update the browser.

8 files changed, 121 insertions, 32 deletions

diff --git a/fluid/Fl_Group_Type.cxx b/fluid/Fl_Group_Type.cxx
index d5a832b17..eab62cb77 100644
--- a/fluid/Fl_Group_Type.cxx
+++ b/fluid/Fl_Group_Type.cxx
@@ -23,6 +23,7 @@
 #include "fluid.h"
 #include "file.h"
 #include "code.h"
+#include "widget_browser.h"
 
 #include <FL/Fl.H>
 #include <FL/Fl_Group.H>
@@ -90,6 +91,7 @@ void group_cb(Fl_Widget *, void *) {
     t = nxt;
   }
   fix_group_size(n);
+  widget_browser->rebuild();
 }
 
 void ungroup_cb(Fl_Widget *, void *) {
@@ -114,6 +116,7 @@ void ungroup_cb(Fl_Widget *, void *) {
     n = nxt;
   }
   delete q;
+  widget_browser->rebuild();
 }
 
 ////////////////////////////////////////////////////////////////
diff --git a/fluid/Fl_Type.cxx b/fluid/Fl_Type.cxx
index 5608969ea..cbaab7b5e 100644
--- a/fluid/Fl_Type.cxx
+++ b/fluid/Fl_Type.cxx
@@ -112,7 +112,9 @@ void select_none_cb(Fl_Widget *,void *) {
   selection_changed(p);
 }
 
-// move selected widgets in their parent's list:
+/**
+ Callback to move all selected items before their previous unselected sibling.
+ */
 void earlier_cb(Fl_Widget*,void*) {
   Fl_Type *f;
   int mod = 0;
@@ -129,8 +131,13 @@ void earlier_cb(Fl_Widget*,void*) {
     f = nxt;
   }
   if (mod) set_modflag(1);
+  widget_browser->display(Fl_Type::current);
+  widget_browser->rebuild();
 }
 
+/**
+ Callback to move all selected items after their next unselected sibling.
+ */
 void later_cb(Fl_Widget*,void*) {
   Fl_Type *f;
   int mod = 0;
@@ -147,6 +154,8 @@ void later_cb(Fl_Widget*,void*) {
     f = prv;
   }
   if (mod) set_modflag(1);
+  widget_browser->display(Fl_Type::current);
+  widget_browser->rebuild();
 }
 
 static void delete_children(Fl_Type *p) {
@@ -154,7 +163,6 @@ static void delete_children(Fl_Type *p) {
   for (f = p; f && f->next && f->next->level > p->level; f = f->next) {/*empty*/}
   for (; f != p; ) {
     Fl_Type *g = f->prev;
-    widget_browser->deleting(f);
     delete f;
     f = g;
   }
@@ -166,7 +174,6 @@ void delete_all(int selected_only) {
     if (f->selected || !selected_only) {
       delete_children(f);
       Fl_Type *g = f->next;
-      widget_browser->deleting(f);
       delete f;
       f = g;
     } else f = f->next;
@@ -275,7 +282,6 @@ Fl_Type::Fl_Type() {
  */
 Fl_Type::~Fl_Type() {
   // warning: destructor only works for widgets that have been add()ed.
-  if (widget_browser) widget_browser->deleting(this);
   if (prev) prev->next = next; else first = next;
   if (next) next->prev = prev; else last = prev;
   if (Fl_Type::last==this) Fl_Type::last = prev;
@@ -394,8 +400,6 @@ void Fl_Type::add(Fl_Type *p, Strategy strategy) {
 
   // run the p tree a last time to make sure the widget_browser updates correctly
   Fl_Type *a = p;
-  for (Fl_Type *t = this; t && a != end; a = t, t = t->next)
-    widget_browser->inserting(a, t);
   widget_browser->redraw();
 }
 
@@ -430,9 +434,6 @@ void Fl_Type::insert(Fl_Type *g) {
   if (parent) parent->add_child(this, g);
   // run this tree a last time to make sure the widget_browser updates correctly
   Fl_Type *a = prev;
-  for (Fl_Type *t = this; t && a != end; a = t, t = t->next)
-    if (a)
-      widget_browser->inserting(a, t);
   widget_browser->redraw();
 }
 
@@ -485,8 +486,6 @@ Fl_Type *Fl_Type::remove() {
   if (parent) parent->remove_child(this);
   parent = 0;
   // tell the widget_browser that we removed some nodes
-  for (Fl_Type *t = this; t; t = t->next)
-    widget_browser->deleting(t);
   widget_browser->redraw();
   selection_changed(0);
   return r;
@@ -532,18 +531,17 @@ void Fl_Type::open() {
 
 /**
  Move this node (and its children) into list before g.
+ Both `this` and `g` must be in the widget browser.
+ The caller must make sure that the widget browser is rebuilt correctly.
  \param[in] g move \c this tree before \c g
  */
 void Fl_Type::move_before(Fl_Type* g) {
   if (level != g->level) printf("move_before levels don't match! %d %d\n",
                                 level, g->level);
   // Find the last child in the list
-  Fl_Type* n;
+  Fl_Type *n;
   for (n = next; n && n->level > level; n = n->next) ;
   if (n == g) return;
-  // Tell the widget browser that we delete them
-  for (n = next; n && n->level > level; n = n->next)
-    widget_browser->deleting(n);
   // now link this tree before g
   Fl_Type *l = n ? n->prev : Fl_Type::last;
   prev->next = n;
@@ -554,13 +552,6 @@ void Fl_Type::move_before(Fl_Type* g) {
   g->prev = l;
   // tell parent that it has a new child, so it can update itself
   if (parent && is_widget()) parent->move_child(this,g);
-  // run this tree a last time to make sure the widget_browser updates correctly
-  Fl_Type *a = prev;
-  for (Fl_Type *t = this; t && a != n; a = t, t = t->next)
-    if (a)
-      widget_browser->inserting(a, t);
-  widget_browser->display(this);
-  widget_browser->redraw();
 }
 
 
diff --git a/fluid/Fl_Window_Type.cxx b/fluid/Fl_Window_Type.cxx
index 21eefe868..59ab189aa 100644
--- a/fluid/Fl_Window_Type.cxx
+++ b/fluid/Fl_Window_Type.cxx
@@ -1239,6 +1239,8 @@ int Fl_Window_Type::handle(int event) {
       popupx = 0x7FFFFFFF;
       popupy = 0x7FFFFFFF; // mark as invalid (MAXINT)
       in_this_only = NULL;
+      widget_browser->display(Fl_Type::current);
+      widget_browser->rebuild();
       return 1;
     }
   case FL_PUSH:
diff --git a/fluid/Shortcut_Button.cxx b/fluid/Shortcut_Button.cxx
index b22d385ce..cb18aa2cc 100644
--- a/fluid/Shortcut_Button.cxx
+++ b/fluid/Shortcut_Button.cxx
@@ -37,6 +37,7 @@ copied or otherwise examined.
 #include "Fl_Window_Type.h"
 #include "factory.h"
 #include "widget_panel.h"
+#include "widget_browser.h"
 
 #include <FL/platform.H>
 #include <FL/Fl_Button.H>
@@ -185,6 +186,8 @@ int Widget_Bin_Window_Button::handle(int inEvent)
             w->position(Fl::event_x_root(), Fl::event_y_root());
           }
         }
+        widget_browser->display(Fl_Type::current);
+        widget_browser->rebuild();
       }
       return Fl_Button::handle(inEvent);
   }
diff --git a/fluid/fluid.cxx b/fluid/fluid.cxx
index f584e182d..df8a713a2 100644
--- a/fluid/fluid.cxx
+++ b/fluid/fluid.cxx
@@ -517,9 +517,11 @@ void revert_cb(Fl_Widget *,void *) {
   undo_suspend();
   if (!read_file(filename, 0)) {
     undo_resume();
+    widget_browser->rebuild();
     fl_message("Can't read %s: %s", filename, strerror(errno));
     return;
   }
+  widget_browser->rebuild();
   undo_resume();
   set_modflag(0, 0);
   undo_clear();
@@ -652,6 +654,7 @@ void open_cb(Fl_Widget *, void *v) {
   undo_suspend();
   if (!read_file(c, v!=0)) {
     undo_resume();
+    widget_browser->rebuild();
     fl_message("Can't read %s: %s", c, strerror(errno));
     free((void *)filename);
     filename = oldfilename;
@@ -659,6 +662,7 @@ void open_cb(Fl_Widget *, void *v) {
     return;
   }
   undo_resume();
+  widget_browser->rebuild();
   if (v) {
     // Inserting a file; restore the original filename...
     free((void *)filename);
@@ -697,6 +701,7 @@ void open_history_cb(Fl_Widget *, void *v) {
   if (!read_file(filename, 0)) {
     undo_resume();
     undo_clear();
+    widget_browser->rebuild();
     fl_message("Can't read %s: %s", filename, strerror(errno));
     free((void *)filename);
     filename = oldfilename;
@@ -706,6 +711,7 @@ void open_history_cb(Fl_Widget *, void *v) {
   set_modflag(0, 0);
   undo_resume();
   undo_clear();
+  widget_browser->rebuild();
   if (oldfilename) {
     free((void *)oldfilename);
     oldfilename = 0L;
@@ -737,6 +743,7 @@ void new_cb(Fl_Widget *, void *v) {
   delete_all();
   set_filename(NULL);
   set_modflag(0, 0);
+  widget_browser->rebuild();
 }
 
 /**
@@ -836,6 +843,7 @@ void new_from_template_cb(Fl_Widget *w, void *v) {
     }
   }
 
+  widget_browser->rebuild();
   set_modflag(0);
   undo_clear();
 }
@@ -975,7 +983,7 @@ void cut_cb(Fl_Widget *, void *) {
   while (p && p->selected) p = p->parent;
   delete_all(1);
   if (p) select_only(p);
-  //widget_browser->redraw_lines();
+  widget_browser->rebuild();
 }
 
 /**
@@ -993,6 +1001,7 @@ void delete_cb(Fl_Widget *, void *) {
   while (p && p->selected) p = p->parent;
   delete_all(1);
   if (p) select_only(p);
+  widget_browser->rebuild();
 }
 
 /**
@@ -1011,9 +1020,12 @@ void paste_cb(Fl_Widget*, void*) {
   if (Fl_Type::current && Fl_Type::current->is_group())
     strategy = kAddAsLastChild;
   if (!read_file(cutfname(), 1, strategy)) {
+    widget_browser->rebuild();
     fl_message("Can't read %s: %s", cutfname(), strerror(errno));
   }
   undo_resume();
+  widget_browser->display(Fl_Type::current);
+  widget_browser->rebuild();
   pasteoffset = 0;
   ipasteoffset += 10;
   force_parent = 0;
@@ -1042,6 +1054,8 @@ void duplicate_cb(Fl_Widget*, void*) {
     fl_message("Can't read %s: %s", cutfname(1), strerror(errno));
   }
   fl_unlink(cutfname(1));
+  widget_browser->display(Fl_Type::current);
+  widget_browser->rebuild();
   undo_resume();
 
   force_parent = 0;
@@ -1052,6 +1066,7 @@ void duplicate_cb(Fl_Widget*, void*) {
  */
 static void sort_cb(Fl_Widget *,void *) {
   sort((Fl_Type*)NULL);
+  widget_browser->rebuild();
 }
 
 /**
diff --git a/fluid/undo.cxx b/fluid/undo.cxx
index 09f2867d9..b7377b89f 100644
--- a/fluid/undo.cxx
+++ b/fluid/undo.cxx
@@ -82,6 +82,7 @@ void redo_cb(Fl_Widget *, void *) {
   undo_suspend();
   if (!read_file(undo_filename(undo_current + 1), 0)) {
     // Unable to read checkpoint file, don't redo...
+    widget_browser->rebuild();
     undo_resume();
     return;
   }
@@ -90,6 +91,7 @@ void redo_cb(Fl_Widget *, void *) {
 
   // Update modified flag...
   set_modflag(undo_current != undo_save);
+  widget_browser->rebuild();
 
   // Update undo/redo menu items...
   if (undo_current >= undo_last) Main_Menu[redo_item].deactivate();
@@ -109,18 +111,17 @@ void undo_cb(Fl_Widget *, void *) {
 
   undo_suspend();
   // Undo first deletes all widgets which resets the widget_tree browser.
-  // Save the current scroll position, so we don;t scroll back to 0 at undo.
-  int x = widget_browser->hposition();
-  int y = widget_browser->position();
+  // Save the current scroll position, so we don't scroll back to 0 at undo.
+  if (widget_browser) widget_browser->save_scroll_position();
   if (!read_file(undo_filename(undo_current - 1), 0)) {
     // Unable to read checkpoint file, don't undo...
+    widget_browser->rebuild();
     undo_resume();
     return;
   }
   // Restore old browser position.
   // Ideally, we would save the browser position insied the undo file.
-  widget_browser->hposition(x);
-  widget_browser->position(y);
+  if (widget_browser) widget_browser->restore_scroll_position();
 
   undo_current --;
 
@@ -130,6 +131,7 @@ void undo_cb(Fl_Widget *, void *) {
   // Update undo/redo menu items...
   if (undo_current <= 0) Main_Menu[undo_item].deactivate();
   Main_Menu[redo_item].activate();
+  widget_browser->rebuild();
   undo_resume();
 }
 
diff --git a/fluid/widget_browser.cxx b/fluid/widget_browser.cxx
index 901680205..ac1e9da2e 100644
--- a/fluid/widget_browser.cxx
+++ b/fluid/widget_browser.cxx
@@ -175,9 +175,11 @@ static char *copy_trunc(char *p, const char *str, int maxl, int quote)
  \todo It would be nice to be able to grab one or more nodes and mmove them
     within the hierarchy.
  */
-Widget_Browser::Widget_Browser(int X,int Y,int W,int H,const char*l)
-: Fl_Browser_(X,Y,W,H,l),
-pushedtitle(NULL)
+Widget_Browser::Widget_Browser(int X,int Y,int W,int H,const char*l) :
+  Fl_Browser_(X,Y,W,H,l),
+  pushedtitle(NULL),
+  saved_h_scroll_(0),
+  saved_v_scroll_(0)
 {
   type(FL_MULTI_BROWSER);
   Fl_Widget::callback(callback_stub);
@@ -504,4 +506,70 @@ int Widget_Browser::handle(int e) {
   return Fl_Browser_::handle(e);
 }
 
+/**
+ Save the current scrollbar postion during rebuild.
+ */
+void Widget_Browser::save_scroll_position() {
+  saved_h_scroll_ = hposition();
+  saved_v_scroll_ = position();
+}
+
+/**
+ Restore the previous scrollbar postion after rebuild.
+ */
+void Widget_Browser::restore_scroll_position() {
+  hposition(saved_h_scroll_);
+  position(saved_v_scroll_);
+}
+
+/**
+ Rebuild the browser layout to reflect multiple changes.
+ This clears internal caches, recalculates the scroll bar sizes, and
+ sends a redraw() request to the widget.
+ */
+void Widget_Browser::rebuild() {
+  save_scroll_position();
+  new_list();
+  damage(FL_DAMAGE_SCROLL);
+  redraw();
+  restore_scroll_position();
+}
+
+/**
+ Rebuild the browser layout and make sure that the given item is visible.
+ \param[in] inNode pointer to a widget node derived from Fl_Type.
+ */
+void Widget_Browser::display(Fl_Type *inNode) {
+  if (!inNode) {
+    // Alternative: find the first (last?) visible selected item.
+    return;
+  }
+  // remeber our current scroll position
+  int currentV = position(), newV = currentV;
+  int nodeV = 0;
+  // find the inNode in the tree and check, if it is already visible
+  Fl_Type *p=Fl_Type::first;
+  for ( ; p && p!=inNode; p=p->next) {
+    if (p->visible)
+      nodeV += item_height(p);
+  }
+  if (p) {
+    int xx, yy, ww, hh;
+    bbox(xx, yy, ww, hh);
+    int frame_top = xx-x();
+    int frame_bottom = frame_top + hh;
+    int node_height = item_height(inNode);
+    int margin_height = 2 * item_quick_height(inNode);
+    if (margin_height>hh/2) margin_height = hh/2;
+    // is the inNode above the current scroll position?
+    if (nodeV<currentV+margin_height)
+      newV = nodeV - margin_height;
+    else if (nodeV>currentV+frame_bottom-margin_height-node_height)
+      newV = nodeV - frame_bottom + margin_height + node_height;
+    if (newV<0)
+      newV = 0;
+  }
+  if (newV!=currentV)
+    position(newV);
+}
 
diff --git a/fluid/widget_browser.h b/fluid/widget_browser.h
index dfcd4db35..f0e68f364 100644
--- a/fluid/widget_browser.h
+++ b/fluid/widget_browser.h
@@ -41,6 +41,8 @@ class Widget_Browser : public Fl_Browser_
   }
 
   Fl_Type* pushedtitle;
+  int saved_h_scroll_;
+  int saved_v_scroll_;
 
   // required routines for Fl_Browser_ subclass:
   void *item_first() const ;
@@ -57,7 +59,10 @@ public:
   Widget_Browser(int,int,int,int,const char * =NULL);
   int handle(int);
   void callback();
-  void deleting(Fl_Type *inType) { Fl_Browser_::deleting((void*)inType); }
+  void save_scroll_position();
+  void restore_scroll_position();
+  void rebuild();
+  void display(Fl_Type *);
 };
 
 #endif // _FLUID_WIDGET_BROWSER_H