Re: [lmi] progress display (was: wx_test regression)

[Vadim Zeitlin]

On Wed, 29 Oct 2014 19:15:01 +0000 Greg Chicares <address@hidden> wrote:

GC> On 2014-10-29 14:42Z, Vadim Zeitlin wrote:
GC> > On Wed, 29 Oct 2014 00:08:49 +0000 Greg Chicares <address@hidden> wrote:
GC> [...]
GC> > GC> Now the progress meter counts only the IllustrationView windows. That
GC> > GC> takes care of one TODO. As for the others:
GC> > GC> 
GC> > GC> // TODO ?? CALCULATION_SUMMARY Instead, why not just update the
GC> > GC> // topmost window first, then update other windows, putting some
GC> > GC> // progress indication on the statusbar?
GC> > 
GC> >  I had seen this comment as well but, to be honest, I don't really
GC> > understand it, mostly because I don't see what is meant by the "topmost
GC> > window" here. Illustration views don't have to be stacked with exactly one
GC> > of them being on top, so there isn't necessarily a single such window. If
GC> > anything, I'd display progress in the status bar of the parent frame, 
which
GC> > is the bottom most one in Z-order.
GC> 
GC> Yes, that's the right status bar to use: the parent frame's.

 Here is an experimental patch showing the gauge in it, please let me know
what do you think. The main problem I see with it is that it's impossible
to cancel it any more. But this might be not a real problem in practice if
it's really always as fast as you measured. And if it is, we can always add
a small "x" button near the gauge, also in the status bar, to allow
cancelling it.

 The other problem is that it's really just too fast to even for this
progress meter to be useful. There is not much I can do here other than
hope that some of your users have slower machines or maybe bigger, more
complex censuses.


GC> The comment meant to say that, if five IllustrationView windows
GC> are shown, with one on top of the others, then it's arguably
GC> ideal to update that most prominent "top" view window first:
GC> it's probably the one the user is looking at, and updating it
GC> first would make the GUI slightly snappier. But I withdraw
GC> the comment: I no longer think it's worth the bother to worry
GC> about this minute detail.

 What I wanted to say was just that you could well have 4 tiled windows and
the whole concept of a top window didn't make much sense in this situation
-- and hence generally. But I promise to drop this subject now.


 The important thing is the patch below. BTW, I don't know how do you apply
the patches, but if you have git now, you should be able to just do "git am
< email_body".

 Please let me know what do you think,
VZ

---------------------------------- >8 --------------------------------------
commit 3cf22a981cb88447484f7383fb716a67041f632a
Author: Vadim Zeitlin <address@hidden>
Date:   Thu Oct 30 01:52:17 2014 +0100

    Use a gauge in the status bar to show progress instead of a dialog.
    
    Experimentally replace the modal progress dialog with a less intrusive gauge
    shown in the main window status bar.

diff --git a/progress_meter_wx.cpp b/progress_meter_wx.cpp
index d9e5779..a4dfd76 100644
--- a/progress_meter_wx.cpp
+++ b/progress_meter_wx.cpp
@@ -31,7 +31,9 @@
 #include "force_linking.hpp"
 #include "wx_utility.hpp"               // TopWindow()
 
-#include <wx/progdlg.h>
+#include <wx/frame.h>
+#include <wx/gauge.h>
+#include <wx/statusbr.h>
 #include <wx/utils.h>                   // wxMilliSleep()
 
 #include <ios>                          // std::fixed, 
std::ios_base::precision()
@@ -50,17 +52,6 @@
 class concrete_progress_meter
     :public progress_meter
 {
-    enum
-        {progress_dialog_style =
-                wxPD_APP_MODAL
-            |   wxPD_AUTO_HIDE
-            |   wxPD_CAN_ABORT
-            |   wxPD_ELAPSED_TIME
-            |   wxPD_ESTIMATED_TIME
-            |   wxPD_REMAINING_TIME
-            |   wxPD_SMOOTH
-        };
-
   public:
     concrete_progress_meter
         (int                max_count
@@ -78,7 +69,7 @@ class concrete_progress_meter
     virtual bool show_progress_message();
     virtual void culminate_ui();
 
-    wxProgressDialog progress_dialog_;
+    wxGauge* gauge_;
 };
 
 // TODO ?? CALCULATION_SUMMARY Resolve this issue.
@@ -93,22 +84,61 @@ class concrete_progress_meter
     ,progress_meter::enum_display_mode display_mode
     )
     :progress_meter(max_count, title, display_mode)
-    ,progress_dialog_
-        (title
-        ,progress_message()
-        ,max_count
-        ,&TopWindow()
-        ,progress_dialog_style
-        )
+    ,gauge_(NULL)
 {
-    if(0 == max_count)
+    // Throughout this function we prefer to silently return rather than throw
+    // an exception if anything goes wrong, as the failure to show a progress
+    // meter is not fatal.
+    wxFrame* const frame = dynamic_cast<wxFrame*>(&TopWindow());
+    if(!frame)
+        {
+        return;
+        }
+
+    wxStatusBar* const bar = frame->GetStatusBar();
+    if(!bar)
+        {
+        return;
+        }
+
+    // Find the position to display the gauge at.
+    wxRect r;
+    if(!bar->GetFieldRect(0, r))
         {
-        progress_dialog_.Update(0);
+        return;
         }
+
+    // Arbitrarily limit the gauge width to 200 dialog units, i.e. about 50
+    // characters.
+    static int const gauge_width = bar->ConvertDialogToPixels(wxPoint(200, 
0)).x;
+
+    // And leave a not less arbitrary margin of 2px between the gauge and the
+    // status bar.
+    r.Deflate(2);
+    r.SetLeft(r.GetRight() - gauge_width);
+    r.SetWidth(gauge_width);
+
+    // The range of progress_meter itself is 0..max_count-1 but it's better to
+    // use 1..max_count for the UI: first, this ensures that we start with a
+    // not completely empty gauge, which looks better, and second, it ensures
+    // that we do fill the gauge entirely at the end.
+    gauge_ = new wxGauge
+        (bar
+        ,wxID_ANY
+        ,max_count + 1
+        ,r.GetPosition()
+        ,r.GetSize()
+        ,wxGA_HORIZONTAL | wxGA_PROGRESS
+        );
+    gauge_->SetValue(1);
 }
 
 concrete_progress_meter::~concrete_progress_meter()
 {
+    if(gauge_)
+        {
+        gauge_->Destroy();
+        }
 }
 
 /// Sleep for the number of seconds given in the argument.
@@ -121,28 +151,28 @@ concrete_progress_meter::~concrete_progress_meter()
 
 void concrete_progress_meter::do_dawdle(int seconds)
 {
-    std::ostringstream oss;
-    oss.precision(1);
-    oss << std::fixed;
-    for(int i = 10 * seconds; 0 < i && !progress_dialog_.WasCancelled(); --i)
+    for(int i = 10 * seconds; 0 < i; --i)
         {
         wxMilliSleep(100);
-        oss.str("");
-        oss << "Waiting " << 0.1 * i << " seconds";
-        progress_dialog_.Update(count(), oss.str());
+        show_progress_message();
         }
 }
 
 std::string concrete_progress_meter::progress_message() const
 {
-    std::ostringstream oss;
-    oss << "Completed " << count() << " of " << max_count();
-    return oss.str();
+    return std::string();
 }
 
 bool concrete_progress_meter::show_progress_message()
 {
-    return progress_dialog_.Update(count(), progress_message());
+    if(gauge_)
+        {
+        // As explained in the code creating the gauge, we offset the
+        // internally used value by 1 to look better.
+        gauge_->SetValue(count() + 1);
+        }
+
+    return true;
 }
 
 void concrete_progress_meter::culminate_ui()
---------------------------------- >8 --------------------------------------