Page MenuHome

Compositor: Merge SingleThreadedOperation with WriteBufferOperation.
Needs RevisionPublic

Authored by Jeroen Bakker (jbakker) on Mar 30 2021, 4:09 PM.

Details

Summary

Single threaded operations always creates a buffer around and write
buffer operation would sample that buffer one pixel at a time. This
refactor would push the samples in one go to the output buffer by
merging the write buffer and single threaded operation.

Less code complexity, less needed CPU cycles and less memory would be
needed.

AMD Ryzen 1700, Linux

branchscheduling modescheduling backendtime ExecutionSystem.execute
masterOutput to inputpthread_queue3.15s
temp-compositor-schedulingOutput to inputpthread_queue3.09s
temp-compositor-single-threaded-operationOutput to inputpthread_queue3.03s

Next steps (after patch lands)

  • Rename single_threaded to write_full_buffer.
  • Convert CalculateMeanOperation to use write_full_buffer. (Add as a first good task).
  • Convert FastGaussian*Operation to use write_full_buffer.
  • Convert Set*Operation to use write_full_buffer. Would speed up unattached input sockets of complex operations.
  • Add similar mechanism for write_partial_buffer.

Diff Detail

Repository
rB Blender
Branch
temp-compositor-single-threaded-operation
Build Status
Buildable 13827
Build 13827: arc lint + arc unit

Event Timeline

Jeroen Bakker (jbakker) requested review of this revision.Mar 30 2021, 4:09 PM
Jeroen Bakker (jbakker) created this revision.
  • Compositor: Merge SingleThreadedOperation with WriteBufferOperation.
  • Revert incorrect change.
  • Merge branch 'master' into temp-compositor-single-threaded-operation
  • Merge branch 'master' into temp-compositor-single-threaded-operation
  • Merge branch 'temp-compositor-scheduling' into temp-compositor-single-threaded-operation
  • Compositor: Added debugging tools.
  • Reduce allocation.
  • Cleanup: Don't use mutex. Isn't needed anymore.
  • Merge branch 'master' into temp-compositor-single-threaded-operation
  • Remove unused definition.
  • Merge branch 'master' into temp-compositor-single-threaded-operation

Single threaded operations always creates a buffer around and write buffer operation would sample that buffer one pixel at a time.

Around what? It seems like part of the sentence got accidentally deleted before posting. 🤗

  • Fixing filter_denoise.blend test case.
Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)Apr 6 2021, 2:13 PM
Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)Apr 7 2021, 11:05 AM
Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)
Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)Apr 7 2021, 11:09 AM
Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)

Merged with latest D10904

Updated the diff to only have local changes.

Jeroen Bakker (jbakker) retitled this revision from [WIP] Compositor: Merge SingleThreadedOperation with WriteBufferOperation. to Compositor: Merge SingleThreadedOperation with WriteBufferOperation..Apr 7 2021, 11:29 AM
Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)
Jeroen Bakker (jbakker) added inline comments.
source/blender/compositor/intern/COM_SingleThreadedOperation.cc
53

Pass along the write buffer to the createMemoryBuffer and rename it to something appropriate.

Can we initializeTileData of all input operations in SingleThreadedOperation::executeRegion and pass a span of input buffers to update_memory_buffer? Later on the concept of initializeTileData won't exist.

I decided to work on top of this patch as a good starting point: D11032

source/blender/compositor/operations/COM_DenoiseOperation.h
67

Change to something like update_memory_buffer(MemoryBuffer& memory_buffer, const rcti *rect, blender::Span<MemoryBuffer*> input_buffers)?

Manuel Castilla (manzanilla) requested changes to this revision.Apr 20 2021, 11:10 PM
Manuel Castilla (manzanilla) added inline comments.
source/blender/compositor/intern/COM_NodeOperationBuilder.cc
433

When connecting two operations that inherit from WriteBufferOperation, ReadBufferOperation is not added in between. Causes issues. This code fixes it:

NodeOperation &output_op = output->getOperation();
if (output_op.get_flags().is_write_buffer_operation) {
  return (WriteBufferOperation *)&output_op;
}
This revision now requires changes to proceed.Apr 20 2021, 11:10 PM