From 6d92fba71b358fc62314eb9385349680915236e3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= <sybren@stuvel.eu>
Date: Tue, 31 Jan 2017 10:53:33 +0100
Subject: [PATCH] Server: progressive rendering is looking good

---
 .../blender_render_progressive.py             |  79 +++--
 packages/flamenco/tests/test_job_compilers.py | 331 ++++++++++++++++++
 2 files changed, 372 insertions(+), 38 deletions(-)

diff --git a/packages/flamenco/flamenco/job_compilers/blender_render_progressive.py b/packages/flamenco/flamenco/job_compilers/blender_render_progressive.py
index 142211b7..e5900139 100644
--- a/packages/flamenco/flamenco/job_compilers/blender_render_progressive.py
+++ b/packages/flamenco/flamenco/job_compilers/blender_render_progressive.py
@@ -19,11 +19,6 @@ class BlenderRenderProgressive(BlenderRender):
     REQUIRED_SETTINGS = ('blender_cmd', 'filepath', 'render_output', 'frames', 'chunk_size',
                          'format', 'cycles_sample_count', 'cycles_num_chunks')
 
-    def __init__(self):
-        import re
-
-        self.hash_re = re.compile('#+')
-
     def compile(self, job):
         import math
         from pathlib2 import Path
@@ -42,23 +37,23 @@ class BlenderRenderProgressive(BlenderRender):
         sample_count_per_chunk = int(math.ceil(float(cycles_sample_count) / self.cycles_num_chunks))
 
         next_merge_task_deps = []
-        prev_samples_from = prev_samples_to = 0
+        prev_samples_to = 0
         for cycles_chunk_idx in range(int(job_settings['cycles_num_chunks'])):
-            # Compute the Cycles sample range for this chunk, in base-0.
-            cycles_samples_from = cycles_chunk_idx * sample_count_per_chunk
+            # Compute the Cycles sample range for this chunk (chunk_idx in base-0), in base-1.
+            cycles_samples_from = cycles_chunk_idx * sample_count_per_chunk + 1
             cycles_samples_to = min((cycles_chunk_idx + 1) * sample_count_per_chunk,
-                                    cycles_sample_count - 1)
+                                    cycles_sample_count)
 
             # Create render tasks for each frame chunk. Only this function uses the base-0
             # chunk/sample numbers, so we also convert to the base-1 numbers that Blender
             # uses.
             render_task_ids = self._make_progressive_render_tasks(
                 job,
-                'render-smpl%i-%i-frm%%s' % (cycles_samples_from + 1, cycles_samples_to + 1),
+                'render-smpl%i-%i-frm%%s' % (cycles_samples_from, cycles_samples_to),
                 move_existing_task_id,
                 cycles_chunk_idx + 1,
-                cycles_samples_from + 1,
-                cycles_samples_to + 1,
+                cycles_samples_from,
+                cycles_samples_to,
                 task_priority=-cycles_chunk_idx * 10,
             )
             task_count += len(render_task_ids)
@@ -70,7 +65,7 @@ class BlenderRenderProgressive(BlenderRender):
             else:
                 merge_task_ids = self._make_merge_tasks(
                     job,
-                    'merge-to-smpl%i-frm%%s' % (cycles_samples_to + 1),
+                    'merge-to-smpl%i-frm%%s' % cycles_samples_to,
                     cycles_chunk_idx + 1,
                     next_merge_task_deps,
                     render_task_ids,
@@ -81,7 +76,6 @@ class BlenderRenderProgressive(BlenderRender):
                 )
                 task_count += len(merge_task_ids)
                 next_merge_task_deps = merge_task_ids
-            prev_samples_from = cycles_samples_from
             prev_samples_to = cycles_samples_to
 
         self._log.info('Created %i tasks for job %s', task_count, job['_id'])
@@ -97,6 +91,13 @@ class BlenderRenderProgressive(BlenderRender):
             raise exceptions.JobSettingError(
                 u'Job %s must use format="EXR", not %r' % (job[u'_id'], render_format))
 
+        # This is quite a limitation, but makes our code to predict the
+        # filename that Blender will use a lot simpler.
+        render_output = job['settings']['render_output']
+        if u'######' not in render_output or u'#######' in render_output:
+            raise exceptions.JobSettingError(
+                u'Setting "render_output" must contain exactly 6 "#" marks.')
+
     def _make_progressive_render_tasks(self,
                                        job, name_fmt, parents,
                                        cycles_chunk_idx,
@@ -106,6 +107,7 @@ class BlenderRenderProgressive(BlenderRender):
 
         :param parents: either a list of parents, one for each task, or a
             single parent used for all tasks.
+        :param cycles_chunk_idx: base-1 sample chunk index
 
         :returns: created task IDs, one render task per frame chunk.
         :rtype: list
@@ -147,8 +149,7 @@ class BlenderRenderProgressive(BlenderRender):
                 parent_task_id = parents
 
             if not isinstance(parent_task_id, ObjectId):
-                raise TypeError('parents should be list of ObjectIds or ObjectId, not %s',
-                                parents)
+                raise TypeError('parents should be list of ObjectIds or ObjectId, not %s' % parents)
 
             task_id = self.task_manager.api_create_task(
                 job, task_cmds, name, parents=[parent_task_id],
@@ -158,14 +159,14 @@ class BlenderRenderProgressive(BlenderRender):
         return task_ids
 
     def _render_output(self, cycles_samples_from, cycles_samples_to):
-        """Intermediate render output path"""
-        render_fname = u'render-smpl%i-%i-frm-######' % (cycles_samples_from, cycles_samples_to)
+        """Intermediate render output path, with ###### placeholder for the frame nr"""
+        render_fname = u'render-smpl-%i-%i-frm-######' % (cycles_samples_from, cycles_samples_to)
         render_output = self.intermediate_path / render_fname
         return render_output
 
     def _merge_output(self, cycles_samples_to):
-        """Intermediate merge output path"""
-        merge_fname = u'merge-smpl%i-frm-######' % cycles_samples_to
+        """Intermediate merge output path, with ###### placeholder for the frame nr"""
+        merge_fname = u'merge-smpl-%i-frm-######' % cycles_samples_to
         merge_output = self.intermediate_path / merge_fname
         return merge_output
 
@@ -176,7 +177,14 @@ class BlenderRenderProgressive(BlenderRender):
                           cycles_samples_from2,
                           cycles_samples_to2,
                           task_priority):
-        """Creates merge tasks for each chunk, consisting of merges for each frame."""
+        """Creates merge tasks for each chunk, consisting of merges for each frame.
+
+        :param cycles_chunk_idx: base-1 sample chunk index
+
+        """
+
+        # Merging cannot happen unless we have at least two chunks
+        assert cycles_chunk_idx >= 2
 
         from flamenco.utils import iter_frame_range, frame_range_merge
 
@@ -186,7 +194,7 @@ class BlenderRenderProgressive(BlenderRender):
 
         cycles_num_chunks = int(job_settings['cycles_num_chunks'])
 
-        weight1 = cycles_samples_from2
+        weight1 = cycles_samples_to1
         weight2 = cycles_samples_to2 - cycles_samples_from2 + 1
 
         # Replace Blender formatting with Python formatting in render output path
@@ -196,32 +204,27 @@ class BlenderRenderProgressive(BlenderRender):
             input1 = self._render_output(1, cycles_samples_to1)
         else:
             input1 = self._merge_output(cycles_samples_to1)
-        input1 = unicode(input1).replace(u'######', u'%06i')
-
         input2 = self._render_output(cycles_samples_from2, cycles_samples_to2)
-        input2 = unicode(input2).replace(u'######', u'%06i')
 
         if cycles_chunk_idx == cycles_num_chunks:
             # At the last merge, we merge to the actual render output, not to intermediary.
-            output =
+            output = job['settings']['render_output']
         else:
             output = self._merge_output(cycles_samples_to2)
 
-        output = unicode(output).replace(u'######', u'%06i')
-
         frame_chunk_iter = iter_frame_range(job_settings['frames'], job_settings['chunk_size'])
         for chunk_idx, chunk_frames in enumerate(frame_chunk_iter):
             # Create a merge command for every frame in the chunk.
-            task_cmds = [
-                commands.MergeProgressiveRenders(
-                    input1=input1 % framenr,
-                    input2=input2 % framenr,
-                    output=output % framenr,
-                    weight1=weight1,
-                    weight2=weight2,
-                )
-                for framenr in chunk_frames
-                ]
+            task_cmds = []
+            for framenr in chunk_frames:
+                task_cmds.append(
+                    commands.MergeProgressiveRenders(
+                        input1=unicode(input1).replace(u'######', u'%06i') % framenr,
+                        input2=unicode(input2).replace(u'######', u'%06i') % framenr,
+                        output=unicode(output).replace(u'######', u'%06i') % framenr,
+                        weight1=weight1,
+                        weight2=weight2,
+                    ))
 
             name = name_fmt % frame_range_merge(chunk_frames)
 
diff --git a/packages/flamenco/tests/test_job_compilers.py b/packages/flamenco/tests/test_job_compilers.py
index b8e549f8..e5adea68 100644
--- a/packages/flamenco/tests/test_job_compilers.py
+++ b/packages/flamenco/tests/test_job_compilers.py
@@ -60,3 +60,334 @@ class CommandTest(unittest.TestCase):
                 'message': u'Preparing to sleep',
             }
         }, cmd.to_dict())
+
+
+class BlenderRenderProgressiveTest(unittest.TestCase):
+    def test_nonexr_job(self):
+        from flamenco.job_compilers import blender_render_progressive
+        from flamenco.exceptions import JobSettingError
+
+        job_doc = {
+            u'_id': ObjectId(24 * 'f'),
+            u'settings': {
+                u'frames': u'1-6',
+                u'chunk_size': 2,
+                u'render_output': u'/render/out/frames-######',
+                u'format': u'JPEG',
+                u'filepath': u'/agent327/scenes/someshot/somefile.blend',
+                u'blender_cmd': u'/path/to/blender --enable-new-depsgraph',
+                u'cycles_sample_count': 30,
+                u'cycles_num_chunks': 3,
+            }
+        }
+        task_manager = mock.Mock()
+        compiler = blender_render_progressive.BlenderRenderProgressive(task_manager=task_manager)
+
+        self.assertRaises(JobSettingError, compiler.compile, job_doc)
+
+    def test_small_job(self):
+        from flamenco.job_compilers import blender_render_progressive, commands
+
+        job_doc = {
+            u'_id': ObjectId(24 * 'f'),
+            u'settings': {
+                u'frames': u'1-6',
+                u'chunk_size': 2,
+                u'render_output': u'/render/out/frames-######',
+                u'format': u'EXR',
+                u'filepath': u'/agent327/scenes/someshot/somefile.blend',
+                u'blender_cmd': u'/path/to/blender --enable-new-depsgraph',
+                u'cycles_sample_count': 30,
+                u'cycles_num_chunks': 3,
+            }
+        }
+        task_manager = mock.Mock()
+
+        # We expect:
+        # - 1 move-out-of-way task
+        # - 3 frame chunks of 2 frames each
+        # - 3 progressive renders per frame
+        # - 2 merge tasks per frame chunk
+        # so that's a 9 render tasks and 6 merge tasks, giving 16 tasks in total.
+        task_ids = [ObjectId() for _ in range(16)]
+        task_manager.api_create_task.side_effect = task_ids
+
+        compiler = blender_render_progressive.BlenderRenderProgressive(task_manager=task_manager)
+        compiler.compile(job_doc)
+
+        task_manager.api_create_task.assert_has_calls([
+            mock.call(job_doc,
+                      [commands.MoveOutOfWay(src=u'/render/out')],
+                      u'move-existing-frames'),
+
+            # First Cycles chunk
+            mock.call(
+                job_doc,
+                [commands.BlenderRenderProgressive(
+                    blender_cmd=u'/path/to/blender --enable-new-depsgraph',
+                    filepath=u'/agent327/scenes/someshot/somefile.blend',
+                    format=u'EXR',
+                    render_output=u'/render/out/_intermediate/render-smpl-1-10-frm-######',
+                    frames=u'1,2',
+                    cycles_num_chunks=3,
+                    cycles_chunk=1,
+                    cycles_samples_from=1,
+                    cycles_samples_to=10)],
+                u'render-smpl1-10-frm1,2',
+                parents=[task_ids[0]],
+                priority=0),
+            mock.call(
+                job_doc,
+                [commands.BlenderRenderProgressive(
+                    blender_cmd=u'/path/to/blender --enable-new-depsgraph',
+                    filepath=u'/agent327/scenes/someshot/somefile.blend',
+                    format=u'EXR',
+                    render_output=u'/render/out/_intermediate/render-smpl-1-10-frm-######',
+                    frames=u'3,4',
+                    cycles_num_chunks=3,
+                    cycles_chunk=1,
+                    cycles_samples_from=1,
+                    cycles_samples_to=10)],
+                u'render-smpl1-10-frm3,4',
+                parents=[task_ids[0]],
+                priority=0),
+            mock.call(
+                job_doc,
+                [commands.BlenderRenderProgressive(
+                    blender_cmd=u'/path/to/blender --enable-new-depsgraph',
+                    filepath=u'/agent327/scenes/someshot/somefile.blend',
+                    format=u'EXR',
+                    render_output=u'/render/out/_intermediate/render-smpl-1-10-frm-######',
+                    frames=u'5,6',
+                    cycles_num_chunks=3,
+                    cycles_chunk=1,
+                    cycles_samples_from=1,
+                    cycles_samples_to=10)],
+                u'render-smpl1-10-frm5,6',
+                parents=[task_ids[0]],
+                priority=0),
+
+            # Second Cycles chunk
+            mock.call(
+                job_doc,
+                [commands.BlenderRenderProgressive(
+                    blender_cmd=u'/path/to/blender --enable-new-depsgraph',
+                    filepath=u'/agent327/scenes/someshot/somefile.blend',
+                    format=u'EXR',
+                    render_output=u'/render/out/_intermediate/render-smpl-11-20-frm-######',
+                    frames=u'1,2',
+                    cycles_num_chunks=3,
+                    cycles_chunk=2,
+                    cycles_samples_from=11,
+                    cycles_samples_to=20)],
+                u'render-smpl11-20-frm1,2',
+                parents=[task_ids[0]],
+                priority=-10),
+            mock.call(
+                job_doc,
+                [commands.BlenderRenderProgressive(
+                    blender_cmd=u'/path/to/blender --enable-new-depsgraph',
+                    filepath=u'/agent327/scenes/someshot/somefile.blend',
+                    format=u'EXR',
+                    render_output=u'/render/out/_intermediate/render-smpl-11-20-frm-######',
+                    frames=u'3,4',
+                    cycles_num_chunks=3,
+                    cycles_chunk=2,
+                    cycles_samples_from=11,
+                    cycles_samples_to=20)],
+                u'render-smpl11-20-frm3,4',
+                parents=[task_ids[0]],
+                priority=-10),
+            mock.call(
+                job_doc,
+                [commands.BlenderRenderProgressive(
+                    blender_cmd=u'/path/to/blender --enable-new-depsgraph',
+                    filepath=u'/agent327/scenes/someshot/somefile.blend',
+                    format=u'EXR',
+                    render_output=u'/render/out/_intermediate/render-smpl-11-20-frm-######',
+                    frames=u'5,6',
+                    cycles_num_chunks=3,
+                    cycles_chunk=2,
+                    cycles_samples_from=11,
+                    cycles_samples_to=20)],
+                u'render-smpl11-20-frm5,6',
+                parents=[task_ids[0]],
+                priority=-10),
+
+            # First merge pass
+            mock.call(
+                job_doc,
+                [
+                    commands.MergeProgressiveRenders(
+                        input1=u'/render/out/_intermediate/render-smpl-1-10-frm-000001',
+                        input2=u'/render/out/_intermediate/render-smpl-11-20-frm-000001',
+                        output=u'/render/out/_intermediate/merge-smpl-20-frm-000001',
+                        weight1=10,
+                        weight2=10,
+                    ),
+                    commands.MergeProgressiveRenders(
+                        input1=u'/render/out/_intermediate/render-smpl-1-10-frm-000002',
+                        input2=u'/render/out/_intermediate/render-smpl-11-20-frm-000002',
+                        output=u'/render/out/_intermediate/merge-smpl-20-frm-000002',
+                        weight1=10,
+                        weight2=10,
+                    ),
+                ],
+                u'merge-to-smpl20-frm1,2',
+                parents=[task_ids[1], task_ids[4]],
+                priority=-11),
+            mock.call(
+                job_doc,
+                [
+                    commands.MergeProgressiveRenders(
+                        input1=u'/render/out/_intermediate/render-smpl-1-10-frm-000003',
+                        input2=u'/render/out/_intermediate/render-smpl-11-20-frm-000003',
+                        output=u'/render/out/_intermediate/merge-smpl-20-frm-000003',
+                        weight1=10,
+                        weight2=10,
+                    ),
+                    commands.MergeProgressiveRenders(
+                        input1=u'/render/out/_intermediate/render-smpl-1-10-frm-000004',
+                        input2=u'/render/out/_intermediate/render-smpl-11-20-frm-000004',
+                        output=u'/render/out/_intermediate/merge-smpl-20-frm-000004',
+                        weight1=10,
+                        weight2=10,
+                    ),
+                ],
+                u'merge-to-smpl20-frm3,4',
+                parents=[task_ids[2], task_ids[5]],
+                priority=-11),
+            mock.call(
+                job_doc,
+                [
+                    commands.MergeProgressiveRenders(
+                        input1=u'/render/out/_intermediate/render-smpl-1-10-frm-000005',
+                        input2=u'/render/out/_intermediate/render-smpl-11-20-frm-000005',
+                        output=u'/render/out/_intermediate/merge-smpl-20-frm-000005',
+                        weight1=10,
+                        weight2=10,
+                    ),
+                    commands.MergeProgressiveRenders(
+                        input1=u'/render/out/_intermediate/render-smpl-1-10-frm-000006',
+                        input2=u'/render/out/_intermediate/render-smpl-11-20-frm-000006',
+                        output=u'/render/out/_intermediate/merge-smpl-20-frm-000006',
+                        weight1=10,
+                        weight2=10,
+                    ),
+                ],
+                u'merge-to-smpl20-frm5,6',
+                parents=[task_ids[3], task_ids[6]],
+                priority=-11),
+
+            # Third Cycles chunk
+            mock.call(
+                job_doc,
+                [commands.BlenderRenderProgressive(
+                    blender_cmd=u'/path/to/blender --enable-new-depsgraph',
+                    filepath=u'/agent327/scenes/someshot/somefile.blend',
+                    format=u'EXR',
+                    render_output=u'/render/out/_intermediate/render-smpl-21-30-frm-######',
+                    frames=u'1,2',
+                    cycles_num_chunks=3,
+                    cycles_chunk=3,
+                    cycles_samples_from=21,
+                    cycles_samples_to=30)],
+                u'render-smpl21-30-frm1,2',
+                parents=[task_ids[0]],
+                priority=-20),
+            mock.call(
+                job_doc,
+                [commands.BlenderRenderProgressive(
+                    blender_cmd=u'/path/to/blender --enable-new-depsgraph',
+                    filepath=u'/agent327/scenes/someshot/somefile.blend',
+                    format=u'EXR',
+                    render_output=u'/render/out/_intermediate/render-smpl-21-30-frm-######',
+                    frames=u'3,4',
+                    cycles_num_chunks=3,
+                    cycles_chunk=3,
+                    cycles_samples_from=21,
+                    cycles_samples_to=30)],
+                u'render-smpl21-30-frm3,4',
+                parents=[task_ids[0]],
+                priority=-20),
+            mock.call(
+                job_doc,
+                [commands.BlenderRenderProgressive(
+                    blender_cmd=u'/path/to/blender --enable-new-depsgraph',
+                    filepath=u'/agent327/scenes/someshot/somefile.blend',
+                    format=u'EXR',
+                    render_output=u'/render/out/_intermediate/render-smpl-21-30-frm-######',
+                    frames=u'5,6',
+                    cycles_num_chunks=3,
+                    cycles_chunk=3,
+                    cycles_samples_from=21,
+                    cycles_samples_to=30)],
+                u'render-smpl21-30-frm5,6',
+                parents=[task_ids[0]],
+                priority=-20),
+
+            # Final merge pass
+            mock.call(
+                job_doc,
+                [
+                    commands.MergeProgressiveRenders(
+                        input1=u'/render/out/_intermediate/merge-smpl-20-frm-000001',
+                        input2=u'/render/out/_intermediate/render-smpl-21-30-frm-000001',
+                        output=u'/render/out/frames-000001',
+                        weight1=20,
+                        weight2=10,
+                    ),
+                    commands.MergeProgressiveRenders(
+                        input1=u'/render/out/_intermediate/merge-smpl-20-frm-000002',
+                        input2=u'/render/out/_intermediate/render-smpl-21-30-frm-000002',
+                        output=u'/render/out/frames-000002',
+                        weight1=20,
+                        weight2=10,
+                    ),
+                ],
+                u'merge-to-smpl30-frm1,2',
+                parents=[task_ids[7], task_ids[10]],
+                priority=-21),
+            mock.call(
+                job_doc,
+                [
+                    commands.MergeProgressiveRenders(
+                        input1=u'/render/out/_intermediate/merge-smpl-20-frm-000003',
+                        input2=u'/render/out/_intermediate/render-smpl-21-30-frm-000003',
+                        output=u'/render/out/frames-000003',
+                        weight1=20,
+                        weight2=10,
+                    ),
+                    commands.MergeProgressiveRenders(
+                        input1=u'/render/out/_intermediate/merge-smpl-20-frm-000004',
+                        input2=u'/render/out/_intermediate/render-smpl-21-30-frm-000004',
+                        output=u'/render/out/frames-000004',
+                        weight1=20,
+                        weight2=10,
+                    ),
+                ],
+                u'merge-to-smpl30-frm3,4',
+                parents=[task_ids[8], task_ids[11]],
+                priority=-21),
+            mock.call(
+                job_doc,
+                [
+                    commands.MergeProgressiveRenders(
+                        input1=u'/render/out/_intermediate/merge-smpl-20-frm-000005',
+                        input2=u'/render/out/_intermediate/render-smpl-21-30-frm-000005',
+                        output=u'/render/out/frames-000005',
+                        weight1=20,
+                        weight2=10,
+                    ),
+                    commands.MergeProgressiveRenders(
+                        input1=u'/render/out/_intermediate/merge-smpl-20-frm-000006',
+                        input2=u'/render/out/_intermediate/render-smpl-21-30-frm-000006',
+                        output=u'/render/out/frames-000006',
+                        weight1=20,
+                        weight2=10,
+                    ),
+                ],
+                u'merge-to-smpl30-frm5,6',
+                parents=[task_ids[9], task_ids[12]],
+                priority=-21),
+        ])
-- 
GitLab