Closed shadeofblue closed 4 years ago
any pull requests related to this issue should go into the b0.20.1
branch for now
There is also an open question: if provided subtasks_count is illegal, should creating task be aborted, or subtasks_count
corrected?
Because right now it looks like different authors of different parts had different answers to this question.
There is also an open question: if provided subtasks_count is illegal, should creating task be aborted, or
subtasks_count
corrected? Because right now it looks like different authors of different parts had different answers to this question.
So I think it should be like this:
comp.task.subtasks.count
rpc method is too specific and bound to a single task type.more interestingly,
RenderingTaskBuilder._calculate_total
(overridden inFrameRenderingTaskBuilder
) contains some of the same code that's featured inframerenderingtask.calculate_subtasks_count
... is that even used?
The RenderingTask...
stuff, that is overridden by FrameRenderingTask...
is not used. Currently there is no other use for RenderingTask...
than to be derived by FrameRenderingTask...
. We can cleanup our code by merging those two into one layer.
We can cleanup our code by merging those two into one layer.
I've made an issue for that: #4645
Lets get to the second point of this task:
establish what the subtask count limits function should be
@magdasta said that an image should not be cut in stripes thinner than 10 pixels (it should be 8 pixels, but there were some problems with edge cases, so 10 is safe).
I think that this sets quite clearly an upper limit for subtasks. For a 1920x1080 image it would be 108 subtasks. This also scales linearly with the number of frames.
identified on 0.19.0, still present on 0.20 RC
[x] verify the exact conditions governing the current (blender ?) subtask count limits
apps.rendering.task.framerenderingtask.calculate_subtasks_count
:_validate_task_dict
ingolem.task.rpc
called fromprepare_and_validate_task_dict
which in turn is called by thecomp.task.create
,comp.task.restart
and indirectly bycomp.task.subtasks.restart
-> this one is pretty much broken since thetask_dict
'soptions
does not have theframe_count
property (which is filled only when the appropriate task builder is run later in the process of task creation) - the result is that the logic that should be used for animations (rounding towards highest value, lower or equal to the requested number of subtasks which divides by the number of frames without remainder - in case subtask count > frames count - and a different calculation for subtasks < frames) is never used and both single frames and animations use the single-frame logic for task dictionary validation (1 < number of subtasks < 50
) -> most weirdly (for both single frames and animations) when the subtask count is outside the limits, a default value of20
is outputted as "optimal" (imvho, a maximum should be used when the number was above 50...)FrameRenderingTaskBuilder._calculate_total
... which most likely uses the logic "correctly" and, again correctly (though silently) applies this logic to the enqueued task -> effectily enforcing the aforementioned animation logic to an already-"validated" task dict ... -> which causes weird artifacts -> e.g. when you define a task with 5 frames and 4 subtasks, 3 fragments are created, which means 3 subtasks are run...RenderingTaskBuilder._calculate_total
(overridden inFrameRenderingTaskBuilder
) contains some of the same code that's featured inframerenderingtask.calculate_subtasks_count
... is that even used?[ ] establish what the subtask count limits function should be - maybe ask: @badb @mfranciszkiewicz
[ ] fix/implement reasonable subtask count limits