[gparted] Add a single ProgressBar for all OperationDetail objects (#760709)



commit c3669c3a9603d65ee771cf13df75874416944464
Author: Mike Fleetwood <mike fleetwood googlemail com>
Date:   Mon Jan 11 13:58:45 2016 +0000

    Add a single ProgressBar for all OperationDetail objects (#760709)
    
    1) Multiple progress bars
    
    The OperationDetail class contains member fraction which is used to feed
    data to the current operation progress bar shown in the Applying pending
    operations dialog.  Dialog_Progress::on_signal_update() gets called for
    every updated OperationDetail object and depending on whether fraction
    is > 0.0 or not, switches between showing a growing or pulsing progress
    bar.  This leads to the conclusion that every OperationDetail object
    currently being updated is effectively driving the single on screen
    progress bar with different data.
    
    The Copy_Blocks code is careful to update text and faction in a single
    OperationDetail object and everything is good.  The on screen progress
    bar is switched into growing mode and then grows to 100%.
    
    Since external command output is updated in real time [1] there are two
    OperationDetail objects, one for stdout and one for stderr, which are
    updated whenever data is read from the relevant stream.  Also now that
    progress is interpreted from some external command output [2][3][4] a
    separate OperationDetail object is getting updated with the progress
    fraction.  (Actually the grandparent OperationDetail of the ones
    receiving stdout and stderr updates as used by the file system specific
    *_progress() methods).  In the normal case of an external command
    which is reporting it's progress two OperationDetails are constantly
    being updated together, the OperationDetail object tracking stdout and
    it's grandparent receiving progress fraction updates.  This causes the
    the code in Dialog_Progress::on_signal_update() to constantly switch
    between growing and pulsing progress bar mode.  The only reason this
    doesn't flash the progress bar is because the stdout OperationDetail
    object is updated first and before the 100 ms timeout fires to pulse the
    bar, it's grandparent is updated with the new fraction to keep growing
    the bar instead.
    
    2) Common code
    
    The Copy_Blocks code currently tracks the progress of blocks copied
    against target amount, which it has to do anyway.  That information is
    then used to generate the text and fraction to update into the
    OperationDetail object and drive the on screen progress bar.  This same
    level of tracking is wanted for the XFS and ext2/3/4 file system
    specific copy methods.
    
    Conclusion and solution
    
    Having multiple sources of progress bar data is a problem and makes it
    clear that there must be only one source of progress data.  Also some
    code can be shared for tracking the amount of blocks copied and
    generating the display.
    
    Therefore have a single ProgressBar object which is used everywhere.
    
    This commit
    
    It just creates a single ProgressBar object which is available via all
    OperationDetail objects and Copy_Blocks is updated accordingly.  Note
    that the ProgressBar still contains debugging and that the GUI progress
    bar of the current operation is still driven via the fraction member in
    any OperationDetail object.
    
    Referenced commits:
    
    [1] 52a2a9b00a32996921ace055e71d0e09fb33c5fe
        Reduce threading (#685740)
    
    [2] ae434579e160ca2c306921b12d4211bf9abacef7
        Display progress for e2fsck (#467925)
    [3] baea186138cc08cac1f13f28300ca0f4b09ae16d
        Display progress for mke2fs (#467925)
    [4] 57b028bb8e7c9fa5e750624f405cc268dfa3b4af
        Display progress during resize (#467925)
    
    Bug 760709 - Add progress bars to XFS and EXT2/3/4 file system specific
                 copy methods

 include/Copy_Blocks.h     |    2 --
 include/OperationDetail.h |    3 +++
 src/Copy_Blocks.cc        |    6 +++---
 src/OperationDetail.cc    |   11 +++++++++++
 4 files changed, 17 insertions(+), 5 deletions(-)
---
diff --git a/include/Copy_Blocks.h b/include/Copy_Blocks.h
index fcf81ef..1037f33 100644
--- a/include/Copy_Blocks.h
+++ b/include/Copy_Blocks.h
@@ -18,7 +18,6 @@
 #define GPARTED_COPY_BLOCKS_H
 
 #include "../include/OperationDetail.h"
-#include "../include/ProgressBar.h"
 #include "../include/Utils.h"
 
 #include <glibmm/ustring.h>
@@ -45,7 +44,6 @@ class copy_blocks {
        void copy_thread();
        bool cancel;
        bool cancel_safe;
-       ProgressBar progressbar;
        void set_cancel( bool force );
 public:
        bool set_progress_info();
diff --git a/include/OperationDetail.h b/include/OperationDetail.h
index e8fb802..f119e00 100644
--- a/include/OperationDetail.h
+++ b/include/OperationDetail.h
@@ -18,6 +18,8 @@
 #ifndef GPARTED_OPERATIONDETAIL_H
 #define GPARTED_OPERATIONDETAIL_H
 
+#include "../include/ProgressBar.h"
+
 #include <glibmm/ustring.h>
 #include <glibmm/markup.h>
 
@@ -63,6 +65,7 @@ public:
        std::vector<OperationDetail*> & get_childs() ;
        const std::vector<OperationDetail*> & get_childs() const ;
        OperationDetail & get_last_child() ;
+       ProgressBar & get_progressbar() const;
 
        double fraction ;
        Glib::ustring progress_text ;
diff --git a/src/Copy_Blocks.cc b/src/Copy_Blocks.cc
index c1a0fb3..b5d608d 100644
--- a/src/Copy_Blocks.cc
+++ b/src/Copy_Blocks.cc
@@ -62,7 +62,7 @@ copy_blocks::copy_blocks( const Glib::ustring & in_src_device,
 bool copy_blocks::set_progress_info()
 {
        Byte_Value done = llabs(this->done);
-       progressbar.update( (double)done );
+       operationdetail.get_progressbar().update( (double)done );
        OperationDetail &operationdetail = this->operationdetail.get_last_child().get_last_child();
        operationdetail.fraction = done / static_cast<double>( length );
 
@@ -166,7 +166,7 @@ bool copy_blocks::copy()
                        String::ucompose( _("copy %1 using a block size of %2"),
                                          Utils::format_size( length, 1 ),
                                          Utils::format_size( blocksize, 1 ) ) ) );
-       progressbar.start( (double)length, PROGRESSBAR_TEXT_COPY_BYTES );
+       operationdetail.get_progressbar().start( (double)length, PROGRESSBAR_TEXT_COPY_BYTES );
 
        done = length % blocksize;
 
@@ -202,7 +202,7 @@ bool copy_blocks::copy()
        else
                error_message = Glib::strerror( errno );
 
-       progressbar.stop();
+       operationdetail.get_progressbar().stop();
        operationdetail.get_last_child().set_status( success ? STATUS_SUCCES : STATUS_ERROR );
        return success;
 }
diff --git a/src/OperationDetail.cc b/src/OperationDetail.cc
index 34d5a1e..b66c4ef 100644
--- a/src/OperationDetail.cc
+++ b/src/OperationDetail.cc
@@ -16,11 +16,15 @@
 
 
 #include "../include/OperationDetail.h"
+#include "../include/ProgressBar.h"
 #include "../include/Utils.h"
 
 namespace GParted
 {
 
+// The single progress bar for the current operation
+static ProgressBar single_progressbar;
+
 OperationDetail::OperationDetail() : fraction( -1 ), cancelflag( 0 ), status( STATUS_NONE ), time_start( -1 
),
                                     time_elapsed( -1 )
 {
@@ -156,6 +160,13 @@ OperationDetail & OperationDetail::get_last_child()
        return *sub_details[sub_details.size() - 1];
 }
 
+ProgressBar & OperationDetail::get_progressbar() const
+{
+       return single_progressbar;
+}
+
+// Private methods
+
 void OperationDetail::on_update( const OperationDetail & operationdetail ) 
 {
        if ( ! treepath .empty() )


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]