From bb9ae20c32150d93aa1aa6ae2c38c7d544263f13 Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Sun, 14 Mar 2021 16:25:56 +0100 Subject: [PATCH] Rework of the progress reporting scheme: LVS got a log now, the layout is horizontal for multiple progress objects, less flicker and more quiet display, cancel function should work more reliably now. --- src/db/db/dbLayout.cc | 4 +- src/lay/lay/layHelpSource.cc | 3 +- src/lay/lay/layProgress.cc | 85 ++++++++++++------- src/lay/lay/layProgress.h | 5 +- src/lay/lay/layProgressWidget.cc | 62 +++++++++----- src/lay/lay/layProgressWidget.h | 4 +- .../lvs/built-in-macros/lvs_interpreters.lym | 5 ++ src/tl/tl/tlProgress.cc | 82 +++++++++++------- src/tl/tl/tlProgress.h | 30 +++---- 9 files changed, 178 insertions(+), 102 deletions(-) diff --git a/src/db/db/dbLayout.cc b/src/db/db/dbLayout.cc index f08b0e6ce..fcc6b29ba 100644 --- a/src/db/db/dbLayout.cc +++ b/src/db/db/dbLayout.cc @@ -1671,7 +1671,9 @@ Layout::do_update () // HINT: because of some gcc bug, automatic destruction of the tl::Progress // object does not work. We overcome this problem by creating the object with new // and catching exceptions. - tl::RelativeProgress *pr = new tl::RelativeProgress (tl::to_string (tr ("Sorting layout")), m_cells_size, 1000); + // As this operation is critical we don't want to have it cancelled. Plus: do_update is called during ~LayoutLocker and + // if we throw exceptions then, we'll get a runtime assertion. + tl::RelativeProgress *pr = new tl::RelativeProgress (tl::to_string (tr ("Sorting layout")), m_cells_size, 0, false /*can't cancel*/); pr->set_desc (""); try { diff --git a/src/lay/lay/layHelpSource.cc b/src/lay/lay/layHelpSource.cc index d87714774..4b6f236f1 100644 --- a/src/lay/lay/layHelpSource.cc +++ b/src/lay/lay/layHelpSource.cc @@ -326,8 +326,7 @@ HelpSource::produce_index_file (const std::string &path) m_title_map.clear (); m_parent_of.clear (); - tl::AbsoluteProgress progress (tl::to_string (QObject::tr ("Initializing help index")), 1); - progress.can_cancel (false); + tl::AbsoluteProgress progress (tl::to_string (QObject::tr ("Initializing help index")), 1, false /*can't cancel*/); scan ("/index.xml", progress); try { diff --git a/src/lay/lay/layProgress.cc b/src/lay/lay/layProgress.cc index 36624f20d..46da70895 100644 --- a/src/lay/lay/layProgress.cc +++ b/src/lay/lay/layProgress.cc @@ -57,8 +57,10 @@ static bool is_marked_alive (QObject *obj) // -------------------------------------------------------------------- +const double visibility_delay = 1.0; + ProgressReporter::ProgressReporter () - : m_start_time (), mp_pb (0), m_pw_visible (false) + : mp_pb (0), m_pw_visible (false) { // .. nothing yet .. } @@ -94,22 +96,21 @@ ProgressReporter::register_object (tl::Progress *progress) tl::ProgressAdaptor::register_object (progress); - if (m_start_time == tl::Clock () && ! m_pw_visible) { - m_start_time = tl::Clock::current (); - } - - // make dialog visible after some time has passed - if (! m_pw_visible && (tl::Clock::current () - m_start_time).seconds () > 1.0) { - set_visible (true); - } - if (progress->is_abstract ()) { + + m_active.insert (progress); + + if (! m_pw_visible) { + set_visible (true); + } + if (mp_pb) { mp_pb->update_progress (progress); } process_events (); + } else { - update_and_yield (); + m_queued.insert (std::make_pair (progress, tl::Clock::current ())); } } @@ -121,46 +122,64 @@ ProgressReporter::unregister_object (tl::Progress *progress) // close or refresh window if (begin () == end ()) { + m_queued.clear (); + m_active.clear (); + if (m_pw_visible) { set_visible (false); } - m_start_time = tl::Clock (); - if (mp_pb) { mp_pb->update_progress (0); } - process_events (); QApplication::instance ()->removeEventFilter (this); } else { - update_and_yield (); + + m_queued.erase (progress); + + std::set::iterator a = m_active.find (progress); + if (a != m_active.end ()) { + m_active.erase (a); + update_and_yield (); + } + } } void -ProgressReporter::trigger (tl::Progress * /*progress*/) +ProgressReporter::trigger (tl::Progress *progress) { - if (begin () != end ()) { - // make dialog visible after some time has passed - if (! m_pw_visible && (tl::Clock::current () - m_start_time).seconds () > 1.0) { + std::map::iterator q = m_queued.find (progress); + if (q != m_queued.end () && (tl::Clock::current () - q->second).seconds () > visibility_delay) { + if (! m_pw_visible) { set_visible (true); } + m_active.insert (progress); + m_queued.erase (q); + } + + if (m_active.find (progress) != m_active.end ()) { update_and_yield (); } } void -ProgressReporter::yield (tl::Progress * /*progress*/) +ProgressReporter::yield (tl::Progress *progress) { - // make dialog visible after some time has passed - if (! m_pw_visible && (tl::Clock::current () - m_start_time).seconds () > 1.0) { - set_visible (true); + std::map::iterator q = m_queued.find (progress); + if (q != m_queued.end () && (tl::Clock::current () - q->second).seconds () > visibility_delay) { + if (! m_pw_visible) { + set_visible (true); + } + m_active.insert (progress); + m_queued.erase (q); update_and_yield (); - } else if (m_pw_visible) { - // process events if necessary + } + + if (m_active.find (progress) != m_active.end ()) { process_events (); } } @@ -172,11 +191,17 @@ ProgressReporter::update_and_yield () return; } - if (mp_pb && first ()) { - mp_pb->update_progress (first ()); - QWidget *w = mp_pb->progress_get_widget (); - if (w) { - first ()->render_progress (w); + if (mp_pb) { + if (first ()) { + mp_pb->update_progress (first ()); + QWidget *w = mp_pb->progress_get_widget (); + if (w) { + first ()->render_progress (w); + } + } else if (begin () != end ()) { + mp_pb->update_progress (begin ().operator-> ()); + } else { + mp_pb->update_progress (0); } } diff --git a/src/lay/lay/layProgress.h b/src/lay/lay/layProgress.h index 7bb7023c0..de0634fe4 100644 --- a/src/lay/lay/layProgress.h +++ b/src/lay/lay/layProgress.h @@ -28,6 +28,8 @@ #include #include +#include +#include #include "tlProgress.h" #include "tlObject.h" @@ -71,9 +73,10 @@ public: void set_progress_bar (lay::ProgressBar *pb); private: - tl::Clock m_start_time; lay::ProgressBar *mp_pb; bool m_pw_visible; + std::map m_queued; + std::set m_active; void process_events (); void update_and_yield (); diff --git a/src/lay/lay/layProgressWidget.cc b/src/lay/lay/layProgressWidget.cc index 3c4cfe670..bd0f711f4 100644 --- a/src/lay/lay/layProgressWidget.cc +++ b/src/lay/lay/layProgressWidget.cc @@ -90,13 +90,13 @@ QSize ProgressBarWidget::sizeHint () const { QFontMetrics fm (font ()); - return QSize (m_width, fm.height () + 2); + return QSize (fm.width (QString::fromUtf8("100%")) * 4, fm.height () + 2); } QSize ProgressBarWidget::minimumSizeHint () const { - return QSize (m_width, 1); + return QSize (50, 1); } void @@ -149,6 +149,11 @@ ProgressWidget::ProgressWidget (ProgressReporter *pr, QWidget *parent, bool fw) QVBoxLayout *log_layout = new QVBoxLayout (mp_log_frame); + mp_log_label = new QLabel (mp_log_frame); + mp_log_label->setText (QString ()); + mp_log_label->setSizePolicy (QSizePolicy (QSizePolicy::Ignored, QSizePolicy::Preferred)); + log_layout->addWidget (mp_log_label); + QListView *log_view = new QListView (this); log_view->setModel (&m_log_file); log_view->setUniformItemSizes (true); @@ -200,22 +205,22 @@ ProgressWidget::ProgressWidget (ProgressReporter *pr, QWidget *parent, bool fw) layout->addItem (new QSpacerItem (8, 8, QSizePolicy::Fixed, QSizePolicy::Fixed), 0, col++, 1, 1); - QFrame *progress_bar_frame = new QFrame (bar_frame); - progress_bar_frame->setFrameStyle (QFrame::Box | QFrame::Plain); - progress_bar_frame->setSizePolicy (QSizePolicy::Expanding, QSizePolicy::Expanding); - layout->addWidget (progress_bar_frame, 0, col, 1, 1); + mp_progress_bar_frame = new QFrame (bar_frame); + mp_progress_bar_frame->setFrameStyle (QFrame::Box | QFrame::Plain); + mp_progress_bar_frame->setSizePolicy (QSizePolicy::Expanding, QSizePolicy::Expanding); + layout->addWidget (mp_progress_bar_frame, 0, col, 1, 1); - QGridLayout *pbf_layout = new QGridLayout (progress_bar_frame); - progress_bar_frame->setLayout (pbf_layout); + QGridLayout *pbf_layout = new QGridLayout (mp_progress_bar_frame); + mp_progress_bar_frame->setLayout (pbf_layout); pbf_layout->setMargin (0); pbf_layout->setSpacing (0); - mp_progress_bar1 = new ProgressBarWidget (progress_bar_frame); - pbf_layout->addWidget (mp_progress_bar1, 0, 0, 1, 1); - mp_progress_bar2 = new ProgressBarWidget (progress_bar_frame); - pbf_layout->addWidget (mp_progress_bar2, 1, 0, 1, 1); - mp_progress_bar3 = new ProgressBarWidget (progress_bar_frame); - pbf_layout->addWidget (mp_progress_bar3, 2, 0, 1, 1); + mp_progress_bar1 = new ProgressBarWidget (mp_progress_bar_frame); + pbf_layout->addWidget (mp_progress_bar1, 0, 2, 1, 1); + mp_progress_bar2 = new ProgressBarWidget (mp_progress_bar_frame); + pbf_layout->addWidget (mp_progress_bar2, 0, 1, 1, 1); + mp_progress_bar3 = new ProgressBarWidget (mp_progress_bar_frame); + pbf_layout->addWidget (mp_progress_bar3, 0, 0, 1, 1); layout->addItem (new QSpacerItem (8, 8, QSizePolicy::Fixed, QSizePolicy::Fixed), 0, col++, 1, 1); @@ -236,11 +241,12 @@ ProgressWidget::ProgressWidget (ProgressReporter *pr, QWidget *parent, bool fw) } void -ProgressWidget::set_log_visible (bool f) +ProgressWidget::set_log_visible (tl::Progress *progress) { - if (f != m_log_visible) { - m_log_visible = f; - mp_log_frame->setVisible (f); + if ((progress != 0) != m_log_visible) { + m_log_visible = (progress != 0); + mp_log_frame->setVisible (m_log_visible); + mp_log_label->setText (progress ? tl::to_qstring (progress->desc ()) : QString ()); set_full_width (m_full_width); } } @@ -290,11 +296,23 @@ ProgressWidget::remove_widget () void ProgressWidget::set_progress (tl::Progress *progress) { + lay::ProgressBarWidget *progress_bars[] = { mp_progress_bar1, mp_progress_bar2, mp_progress_bar3 }; + if (! progress || progress->is_abstract ()) { - m_log_file.clear (); + + if (! progress) { + m_log_file.clear (); + } m_log_file.set_max_entries (progress ? 1000 : 0); - set_log_visible (progress != 0); + + set_log_visible (progress); + + mp_progress_bar_frame->hide (); + mp_cancel_button->setEnabled (true); + mp_label->setText (QString ()); + return; + } bool can_cancel = false; @@ -308,8 +326,6 @@ ProgressWidget::set_progress (tl::Progress *progress) mp_cancel_button->setEnabled (can_cancel); mp_label->setText (tl::to_qstring (text)); - lay::ProgressBarWidget *progress_bars[] = { mp_progress_bar1, mp_progress_bar2, mp_progress_bar3 }; - for (size_t i = 0; i < sizeof (progress_bars) / sizeof (progress_bars[0]); ++i) { lay::ProgressBarWidget *pb = progress_bars[i]; @@ -330,6 +346,8 @@ ProgressWidget::set_progress (tl::Progress *progress) } + mp_progress_bar_frame->show (); + // according to the doc this should not be required, but without, the progress bar does not resize mp_progress_bar1->parentWidget ()->updateGeometry (); } diff --git a/src/lay/lay/layProgressWidget.h b/src/lay/lay/layProgressWidget.h index 0600f7e5d..ff9702956 100644 --- a/src/lay/lay/layProgressWidget.h +++ b/src/lay/lay/layProgressWidget.h @@ -72,6 +72,7 @@ public slots: private: QLabel *mp_label; + QFrame *mp_progress_bar_frame; ProgressBarWidget *mp_progress_bar1, *mp_progress_bar2, *mp_progress_bar3; QWidget *mp_widget; int m_widget_col; @@ -79,12 +80,13 @@ private: QToolButton *mp_cancel_button; ProgressReporter *mp_pr; lay::LogFile m_log_file; + QLabel *mp_log_label; QFrame *mp_log_frame; bool m_full_width; int m_left_col, m_right_col; bool m_log_visible; - void set_log_visible (bool f); + void set_log_visible (tl::Progress *progress); }; } diff --git a/src/lvs/lvs/built-in-macros/lvs_interpreters.lym b/src/lvs/lvs/built-in-macros/lvs_interpreters.lym index 5c6e36eca..bf3ef5631 100644 --- a/src/lvs/lvs/built-in-macros/lvs_interpreters.lym +++ b/src/lvs/lvs/built-in-macros/lvs_interpreters.lym @@ -25,6 +25,8 @@ module LVS lvs._l2ndb_index = l2ndb_index lvs._generator = generator + lvs_progress = RBA::AbstractProgress::new("LVS: " + macro.path) + begin # Set a debugger scope so that our errors end up with the debugger set to the LVS's line @@ -46,6 +48,9 @@ module LVS # cleans up and creates layout and report views lvs._finish + # unlocks the UI + lvs_progress._destroy + end timer.stop diff --git a/src/tl/tl/tlProgress.cc b/src/tl/tl/tlProgress.cc index 88d7e4ee0..1315ad65e 100644 --- a/src/tl/tl/tlProgress.cc +++ b/src/tl/tl/tlProgress.cc @@ -49,7 +49,11 @@ ProgressAdaptor::~ProgressAdaptor () void ProgressAdaptor::register_object (Progress *progress) { + bool cancelled = ! mp_objects.empty () && mp_objects.first ()->break_scheduled (); mp_objects.push_back (progress); // this keeps the outmost one visible. push_front would make the latest one visible. + if (cancelled) { + progress->signal_break (); + } } void @@ -130,15 +134,19 @@ ProgressGarbageCollector::~ProgressGarbageCollector () // store a pointer but a pointer to a pointer. static tl::ThreadStorage s_thread_data; -Progress::Progress (const std::string &desc, size_t yield_interval) +const double yield_timeout = 0.3; +const size_t default_yield_interval = 1000; + +Progress::Progress (const std::string &desc, size_t yield_interval, bool can_cancel) : m_desc (desc), m_title (desc), m_interval_count (0), - m_yield_interval (yield_interval), + m_yield_interval (yield_interval == 0 ? default_yield_interval : yield_interval), m_last_value (-1.0), - m_can_cancel (true), - m_cancelled (false) + m_can_cancel (can_cancel), + m_cancelled (false), + m_registered (false) { - // .. nothing yet .. + m_last_yield = tl::Clock::current (); } Progress::~Progress () @@ -149,9 +157,19 @@ Progress::~Progress () void Progress::initialize () { + // The abstract progress does not get test() calls so we need to register it now. ProgressAdaptor *a = adaptor (); if (a) { + a->register_object (this); + m_registered = true; + + // A pending cancel request may immediately kill the operation - "register_object" will set the cancelled flag then. + if (m_cancelled) { + m_cancelled = false; + throw tl::BreakException (); + } + } } @@ -159,7 +177,7 @@ void Progress::shutdown () { ProgressAdaptor *a = adaptor (); - if (a) { + if (a && m_registered) { a->unregister_object (this); } } @@ -192,29 +210,27 @@ Progress::adaptor () void Progress::signal_break () { - m_cancelled = true; + if (m_can_cancel) { + m_cancelled = true; + } } void Progress::set_desc (const std::string &d) { - ProgressAdaptor *a = adaptor (); - if (a && d != m_desc) { - + if (d != m_desc) { m_desc = d; - a->trigger (this); - a->yield (this); - - if (m_cancelled) { - m_cancelled = false; - throw tl::BreakException (); - } - + test (true); } } -bool Progress::test(bool force_yield) +bool Progress::test (bool force_yield) { + if (m_cancelled) { + m_cancelled = false; + throw tl::BreakException (); + } + if (++m_interval_count >= m_yield_interval || force_yield) { ProgressAdaptor *a = adaptor (); @@ -226,22 +242,28 @@ bool Progress::test(bool force_yield) needs_trigger = true; } + if (m_desc != m_last_desc) { + m_last_desc = m_desc; + needs_trigger = true; + } + m_interval_count = 0; if (a) { + tl::Clock now = tl::Clock::current (); - if ((now - m_last_yield).seconds () > 0.1) { + if ((now - m_last_yield).seconds () > yield_timeout) { + m_last_yield = now; + if (needs_trigger) { a->trigger (this); } - a->yield (this); - } - } - if (m_cancelled) { - m_cancelled = false; - throw tl::BreakException (); + a->yield (this); + + } + } return true; @@ -268,8 +290,8 @@ AbstractProgress::~AbstractProgress () // --------------------------------------------------------------------------------------------- // RelativeProgress implementation -RelativeProgress::RelativeProgress (const std::string &desc, size_t max_count, size_t yield_interval) - : Progress (desc, yield_interval) +RelativeProgress::RelativeProgress (const std::string &desc, size_t max_count, size_t yield_interval, bool can_cancel) + : Progress (desc, yield_interval, can_cancel) { m_format = "%.0f%%"; m_unit = double (max_count) / 100.0; @@ -313,8 +335,8 @@ RelativeProgress::set (size_t count, bool force_yield) // --------------------------------------------------------------------------------------------- // Progress implementation -AbsoluteProgress::AbsoluteProgress (const std::string &desc, size_t yield_interval) - : Progress (desc, yield_interval) +AbsoluteProgress::AbsoluteProgress (const std::string &desc, size_t yield_interval, bool can_cancel) + : Progress (desc, yield_interval, can_cancel) { m_format = "%.0f"; m_unit = 1.0; diff --git a/src/tl/tl/tlProgress.h b/src/tl/tl/tlProgress.h index e0001cd28..13dce6791 100644 --- a/src/tl/tl/tlProgress.h +++ b/src/tl/tl/tlProgress.h @@ -185,8 +185,9 @@ public: * * @param desc The description and title string * @param yield_interval See above. + * @param can_cancel If set to true, the progress may be cancelled which results in a BreakException begin raised */ - Progress (const std::string &desc, size_t yield_interval = 1000); + Progress (const std::string &desc, size_t yield_interval = 0, bool can_cancel = true); /** * @brief The destructor @@ -228,17 +229,6 @@ public: */ virtual void render_progress (QWidget * /*widget*/) const { } - /** - * @brief Set a value indicating whether the operation can be cancelled - * - * The progress object will throw a BreakException is cancelled and this - * flag is set to true. The default is "true". - */ - void can_cancel (bool f) - { - m_can_cancel = f; - } - /** * @brief Gets a value indicating whether the operation can be cancelled */ @@ -273,6 +263,14 @@ public: */ void signal_break (); + /** + * @brief Returns true, if a break is scheduled + */ + bool break_scheduled () const + { + return m_cancelled; + } + protected: /** * @brief Indicates that a new value has arrived @@ -296,13 +294,14 @@ private: friend class ProgressAdaptor; friend class ProgressGarbageCollector; - std::string m_desc; + std::string m_desc, m_last_desc; std::string m_title; size_t m_interval_count; size_t m_yield_interval; double m_last_value; bool m_can_cancel; bool m_cancelled; + bool m_registered; tl::Clock m_last_yield; static tl::ProgressAdaptor *adaptor (); @@ -365,8 +364,9 @@ public: * @param desc The description and title string * @param max_count The limit "max" value. 0 for absolute display of values. * @param yield_interval See above. + * @param can_cancel If set to true, the progress may be cancelled which results in a BreakException begin raised */ - RelativeProgress (const std::string &desc, size_t max_count = 0, size_t yield_interval = 1000); + RelativeProgress (const std::string &desc, size_t max_count = 0, size_t yield_interval = 0, bool can_cancel = true); ~RelativeProgress (); @@ -442,7 +442,7 @@ public: * @param desc The description and title string * @param yield_interval See above. */ - AbsoluteProgress (const std::string &desc, size_t yield_interval = 1000); + AbsoluteProgress (const std::string &desc, size_t yield_interval = 0, bool can_cancel = true); /** * @brief Destructor