Expose pause and resume in studio #2096

oktomus · 2018-06-19T10:58:05Z

Allow to pause rendering in appleseed.studio #170

The new stopwatch model:

It contains following values:

m_start is the time when the user call start or resume
m_pause_time is the time when the user call pause
m_elapsed is the spent time of the current run + previous runs
m_committed is the spent time of previous runs

Actions:

Pressing start will set the starting time
Pressing start again will act as a restart, it will set the starting time again and clear the watch. start will also clear the paused state
Pressing pause change the state of the stopwatch. Call not allowed if the watch is already paused. This also stores the current time in m_pause_time for future measures.
Pressing resume change the state of the stopwatch and sets the starting time. Call not allowed if the watch isn't in the paused state. It also update start time and committed spent time
committed time = committed_time + (now - start) - (now - pause_time)
Pressing clear will set all spent times to 0 but the watch will keep running if it's not paused
When calling measure, the watch stores the elapsed time. The whole spend time is computed as following
whole spent time = committed_time + (now - start) - (now - pause_time)
get_ticks returns the whole spent time (elapsed) adjusted by the overhead

dictoon · 2018-06-22T08:12:42Z

src/appleseed.studio/mainwindow/rendering/renderingmanager.cpp

@@ -217,7 +227,8 @@ void RenderingManager::start_rendering(

 bool RenderingManager::is_rendering() const
 {
-    return m_master_renderer.get() != nullptr;
+    return m_master_renderer.get() != nullptr


This is going to be problematic.

When a render is paused, we're still rendering in the sense that a lot of operations (such as editing the scene) are still forbidden.

This probably needs to be reverted.

dictoon · 2018-06-22T08:13:48Z

src/appleseed/foundation/utility/stopwatch.h

@@ -95,6 +103,10 @@ class Stopwatch

 template <typename Timer>
 Stopwatch<Timer>::Stopwatch(const size_t overhead_measures)
+  : m_paused(false)


Initialization order must follow declaration order.

dictoon · 2018-06-22T08:14:01Z

src/appleseed/foundation/utility/stopwatch.h

@@ -83,6 +89,8 @@ class Stopwatch
    uint64  m_overhead;     // measured overhead of calling start() + measure()
    uint64  m_start;        // timer value when start() is called
    uint64  m_elapsed;      // elapsed time when measure() is called, adjusted for overhead
+    uint64  m_stock;        // elapsed time until pause() is called, adjusted for overhead


What do you mean (in French maybe) by stock?

All the time spent before calling pause

Ok, we'll need to find a better, more explicit name. I could help with naming but I need to fully understand how it works.

Can we call pause() multiple times?

Now we can't, I added an assert. But once you resume, you can call pause again

Ok, that's what I meant. It's fine to assert if pause() is called while already in pause, but of course we should allow calling pause() and resume() multiple times. If that's what we do then great.

So, here's an idea:

In measure(), you we accumulate (add) elapsed time to m_elapsed.

We move the overhead subtraction to get_ticks().

We get rid of m_stock.

Thanks for the idea, I haven't tried to use the overhead in get_ticks which is why I had problems.

The only issue with that, is that we won't be able to call measure if the timer is paused. Otherwise ticks would be wrong

Unless measure() doesn't do anything if paused?

That's what I implemented :) pushing tomorrow

dictoon · 2018-06-22T08:16:02Z

src/appleseed/foundation/utility/stopwatch.h

@@ -155,7 +183,10 @@ inline Stopwatch<Timer>& Stopwatch<Timer>::measure()
 template <typename Timer>
 inline uint64 Stopwatch<Timer>::get_ticks() const
 {
-    return m_elapsed;
+    if (m_paused)


I suggest writing a bunch of unit tests to make sure that pausing a stopwatch works in all cases. You can use a custom timer that returns predetermined values to make your test deterministic and robust.

dictoon · 2018-06-22T08:16:13Z

src/appleseed/renderer/kernel/rendering/defaultrenderercontroller.h

@@ -59,6 +59,9 @@ class APPLESEED_DLLSYMBOL DefaultRendererController
    // This method is called after rendering was aborted.
    void on_rendering_abort() override;

+    void on_rendering_paused() override;


dictoon · 2018-06-22T08:16:20Z

src/appleseed.studio/mainwindow/rendering/qtrenderercontroller.h

@@ -62,6 +62,9 @@ class QtRendererController
    // This method is called after rendering was aborted.
    void on_rendering_abort() override;

+    void on_rendering_paused() override;


oktomus · 2018-06-22T11:42:11Z

src/appleseed.studio/mainwindow/mainwindow.cpp

@@ -1630,6 +1638,14 @@ void MainWindow::slot_start_rendering_once(const QString& filepath, const QStrin
    }
 }

+void MainWindow::slot_pause_or_resume_rendering()
+{
+    if (m_rendering_manager.is_rendering() && !m_rendering_manager.is_rendering_paused())


I need an advice for this. I suppose the slot will be called only while rendering (otherwise the widget is disabled). So the first condition is_rendering can be removed. What do you think ?

I think so, yes. You can check is_rendering() in an assert.

dictoon · 2018-06-25T10:03:11Z

src/appleseed.studio/mainwindow/mainwindow.ui

@@ -404,6 +405,14 @@
    <string>F5</string>
   </property>
  </action>
+  <action name="action_rendering_pause_rendering">
+   <property name="text">
+    <string>Pause &amp;Rendering</string>


That conflicts with &Rendering Settings. I suggest putting the & on the P of Pause.

by the way, what's the purpose of & ?

It creates a keyboard shortcut: the character following the & gets underlined:

That way you can do Alt+F followed by N to create a new project.

dictoon · 2018-06-25T10:11:47Z

src/appleseed/foundation/utility/stopwatch.h

 }

 template <typename Timer>
 inline void Stopwatch<Timer>::start()
 {
+    m_paused = false;


From your description, start() cannot be called in pause state. This should be an assert.

Actually, I would prefer updating my description because it's really convenient to call start to restart everything

dictoon · 2018-06-25T10:12:49Z

src/appleseed/foundation/utility/stopwatch.h

+{
+    assert(m_paused);
+
+    // Update comitted time.


Typo: committed

dictoon · 2018-06-25T10:15:02Z

src/appleseed/renderer/kernel/rendering/defaultrenderercontroller.h

@@ -59,6 +59,12 @@ class APPLESEED_DLLSYMBOL DefaultRendererController
    // This method is called after rendering was aborted.
    void on_rendering_abort() override;

+    // This method is called after rendering was paused.
+    void on_rendering_paused() override;


For consistency with the other slots, should be on_rendering_pause() and on_rendering_resume().

oktomus force-pushed the resumable-renders branch from 609c818 to 0943746 Compare June 21, 2018 07:18

dictoon reviewed Jun 22, 2018

View reviewed changes

oktomus commented Jun 22, 2018

View reviewed changes

dictoon reviewed Jun 25, 2018

View reviewed changes

Expose pause and resume in studio

9fd4554

oktomus force-pushed the resumable-renders branch from 4562cd3 to 9fd4554 Compare June 25, 2018 15:32

Use a checkbox for pause and resume actions

4b1e25a

dictoon approved these changes Jun 27, 2018

View reviewed changes

dictoon merged commit a11da58 into appleseedhq:master Jun 27, 2018

oktomus deleted the resumable-renders branch July 10, 2018 06:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Expose pause and resume in studio #2096

Expose pause and resume in studio #2096

oktomus commented Jun 19, 2018 •

edited

Loading

dictoon Jun 22, 2018

dictoon Jun 22, 2018

dictoon Jun 22, 2018

oktomus Jun 22, 2018 •

edited

Loading

dictoon Jun 22, 2018

oktomus Jun 22, 2018

dictoon Jun 22, 2018

oktomus Jun 22, 2018

oktomus Jun 22, 2018

dictoon Jun 22, 2018

oktomus Jun 22, 2018

dictoon Jun 22, 2018

dictoon Jun 22, 2018

dictoon Jun 22, 2018

oktomus Jun 22, 2018

dictoon Jun 22, 2018

dictoon Jun 25, 2018

oktomus Jun 25, 2018

dictoon Jun 25, 2018

dictoon Jun 25, 2018

dictoon Jun 25, 2018

oktomus Jun 25, 2018

dictoon Jun 25, 2018

dictoon Jun 25, 2018

dictoon Jun 25, 2018

Expose pause and resume in studio #2096

Expose pause and resume in studio #2096

Conversation

oktomus commented Jun 19, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oktomus Jun 22, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oktomus commented Jun 19, 2018 •

edited

Loading

oktomus Jun 22, 2018 •

edited

Loading