Compositor: Only read input constants to determine input area
AbandonedPublic
Actions

Authored by Manuel Castilla (manzanilla) on Apr 26 2021, 8:51 PM.

Details

Reviewers

Group Reviewers

Summary

Scale, Translate and Rotate read inputs first pixel to determine other input area of interest, creating a rendering interdependency between inputs. For the full-frame system it would mean more memory usage as it wouldn't let to determine beforehand all areas operations must render. It would require a specific inputs rendering order and operations life (init/deinit) will be longer as we cannot just render all areas on first read.
It makes sense to read input first pixel if it's a single constant pixel value but if it has variable pixels next ones may cause a greater dependency area. Current implemention seems incorrect.

This patch adds a method for trying to read an input as a constant value that doesn't need rendering. If it's not constant, we can just assume whole input area is used for sake of correctness.

Diff Detail

Repository

rB Blender

Branch

cmp-set-operation (branched from master)

Build Status

Buildable 14241
Build 14241: arc lint + arc unit

Event Timeline

Manuel Castilla (manzanilla) requested review of this revision.Apr 26 2021, 8:51 PM

Manuel Castilla (manzanilla) created this revision.

Harbormaster completed remote builds in B14241: Diff 36536.Apr 26 2021, 8:51 PM

Alternatively to using typeid I could create a SetOperation base class with a read_single_pixel(result) method that all set operations must implement and check the flag is_set_operation. But would require more changes, let me know if you think it's a better solution.

Manuel Castilla (manzanilla) added a reviewer: Compositing.Apr 26 2021, 9:01 PM

Fix: translate and rotate were still reading inputs normally

Harbormaster completed remote builds in B14245: Diff 36542.Apr 26 2021, 10:23 PM

Manuel Castilla (manzanilla) retitled this revision from Compositor: Only read input constants to determine input area. to Compositor: Only read input constants to determine input area.May 9 2021, 7:01 PM

Manuel Castilla (manzanilla) mentioned this in T88150: Full Frame Compositor.May 9 2021, 7:15 PM

Jeroen Bakker (jbakker) requested changes to this revision.May 10 2021, 11:00 AM

Jeroen Bakker (jbakker) added a subscriber: Jeroen Bakker (jbakker).

Jeroen Bakker (jbakker) added inline comments.

source/blender/compositor/intern/COM_NodeOperation.cc
55	I would rather see a system where the responsibility is in the input operation. This solution doesn't check for actual constant values. eg having a node tree with math nodes the result can still be constant. Or even with reroutes/node groups. Is it an idea to check if that part of the tree is constant and if so evaluate it (somehow) In the forward scheduling mechanism this would be easier to implement.

This revision now requires changes to proceed.May 10 2021, 11:00 AM

Would it be similar to cycles nodes "constant folding" idea?
Is this valid approach?:

Add a "can_be_constant" operation flag. Maybe false by default to ensure correctness but many operations are constant when all their inputs are, most distort ones may not.
Before determining input areas, from inputs search for parts of tree with constant operations, evaluate them by rendering a single pixel and replace with a Set<Color/Vector/Value>Operation.

I'll pass responsibility to input operation and better do this patch only for full-frame to don't affect current implementation.

To address review comments, other patch was created:
D11490: Compositor: Constant folding

Manuel Castilla (manzanilla) abandoned this revision.Sep 4 2021, 10:32 AM