Fix Shift+F5 causing False Colors to be applied twice

[laurelkeys]

The color map post-processing stage, applied when False Colors is enabled, is being called twice on Stop Rendering (Shift+F5).

---

This happens by on_rendering_abort() being called and emitting a signal_rendering_abort() at:

appleseed/src/appleseed/renderer/kernel/rendering/masterrenderer.cpp

Line 173 in 964c5a9

MasterRenderer::RenderingResult render(IRendererController& renderer_controller)

which also results in emitting signal_rendering_failed() on:

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

Lines 113 to 117 in 964c5a9

	const MasterRenderer::RenderingResult rendering_result =
	m_master_renderer.render(m_renderer_controller);
	if (rendering_result.m_status != MasterRenderer::RenderingResult::Succeeded)
	emit signal_rendering_failed();

Thus, slot_rendering_end() runs twice.

---

I'm submitting a "conservative" fix, as it only prevents False Colors from being applied when a "failed" signal is emitted (other previous behavior stay the same).

However, it would be interesting to remove unnecessary computations that are being run twice. Creating a new signal_rendering_aborted() on renderingmanager.cpp could help splitting what should be run on "aborting" from "failing", e.g.:

- if (rendering_result.m_status != MasterRenderer::RenderingResult::Succeeded)
-     emit signal_rendering_failed();
+ switch (rendering_result.m_status)
+ {
+   case MasterRenderer::RenderingResult::Succeeded:
+     break;
+ 
+   case MasterRenderer::RenderingResult::Aborted:
+     emit signal_rendering_aborted();
+     break;
+ 
+   default:
+     emit signal_rendering_failed();
+     break;
+ }

[oktomus]

Thanks !

Creating a new signal_rendering_aborted() on renderingmanager.cpp could help splitting what should be run on "aborting" from "failing"

You mean a new signal_rendering_failed ? If so, I think it's a good idea.

[laurelkeys]

Thanks !

Creating a new signal_rendering_aborted() on renderingmanager.cpp could help splitting what should be run on "aborting" from "failing"

You mean a new signal_rendering_failed ? If so, I think it's a good idea.

Sorry, on_rendering_abort() emits signal_rendering_abort() (not signal_rendering_aborted() as I had previously written at the PR description, this isn't declared).

The MasterRendererThread inside renderingmanager.cpp has a single "failed" signal, which it emits both when m_status is Aborted and Failed (line 116 of the second code snippet in the description above).

So, adding an alternative for signal_rendering_failed to be called when the render is aborted (e.g. signal_rendering_aborted) would give us more information:

[oktomus]

@laurelkeys We tested and modified the fix with @dictoon. Instead of adding a new flag for the failure, we prevented the slot slot_rendering_end from being triggered twice.

The main fix is in mainwindow/rendering/renderingmanager.cpp.

We were also reporting an abort when it was actually a failure.