Compositor: Rename compositor build option
ClosedPublic
Actions

Authored by Omar Emara (OmarSquircleArt) on Aug 4 2022, 5:08 PM.

Details

Reviewers

Jeroen Bakker (jbakker)
Ray Molenkamp (LazyDodo)

Commits

rB9990792e8723: Compositor: Rename compositor build option

Summary

Currently, the compositor can be disabled using the WITH_COMPOSITOR
build option. Since, we intent to always build the realtime compositor,
we need to make the distinction between both compositors clear.

So this patch renames the option to WITH_COMPOSITOR_CPU. Additionally,
the check for the option was moved inside the compositor modules' own
CMake file in preparation for the realtime compositor code.

Diff Detail

Repository

rB Blender

Branch

rename-compositor-cmake-flag (branched from master)

Build Status

Buildable 23270
Build 23270: arc lint + arc unit

Event Timeline

Omar Emara (OmarSquircleArt) requested review of this revision.Aug 4 2022, 5:08 PM

Omar Emara (OmarSquircleArt) created this revision.

Harbormaster completed remote builds in B23260: Diff 54393.Aug 4 2022, 5:08 PM

Correct indentation.

Harbormaster completed remote builds in B23270: Diff 54433.Aug 5 2022, 10:00 AM

Jeroen Bakker (jbakker) added inline comments.Aug 8 2022, 11:34 AM

CMakeLists.txt
199	Note to self as it is not really related to this change: we should remove the term tile based here.
extern/CMakeLists.txt
99	Note for viewport compositor. For now OK to still hide this for the view port compositor. When the anti-aliasing node is converted it should always be compiled.
source/blender/compositor/CMakeLists.txt
612–627	Note for viewport compositor: same as previous note.
681	For the viewport compositor project I will expect an `else()` here, but that is currently not the case as the viewport compositor hasn't landed in master yet. Am I correct?
source/blender/python/intern/bpy_app_build_options.c
21	This should become "compositor_cpu" like mentioned in the comment above.

Jeroen Bakker (jbakker) edited reviewers, added: Ray Molenkamp (LazyDodo); removed: Clément Foucault (fclem).Aug 8 2022, 11:35 AM

Jeroen Bakker (jbakker) added a project: Compositing.

Jeroen Bakker (jbakker) added a subscriber: Clément Foucault (fclem).

I added @Ray Molenkamp (LazyDodo) as blocking reviewer to make sure everything is according to out CMake standards.
I didn't see any major issues with the patch just some comments and (mental) notes.

source/blender/compositor/CMakeLists.txt
681	Perhaps mention this in the description of the patch as well so anyone seeing this change is aware why it isn't added.

Omar Emara (OmarSquircleArt) added inline comments.Aug 8 2022, 11:50 AM

extern/CMakeLists.txt
99	The draw module also has an SMAA implementation if I remember correctly, so we will likely use that one instead.
source/blender/compositor/CMakeLists.txt
681	It hasn't landed yet, correct. Though I am not sure why there would be an `else()` here. I assume we will just add the `add_subdirectory(realtime_compositor)` outside of the `if` since it will always be built.

Merge branch 'master' into rename-compositor-cmake-flag
Update build option name in python API.

Harbormaster completed remote builds in B23290: Diff 54487.Aug 8 2022, 1:40 PM

Seems like an improvement, further cleanup could be done by moving things into sub folders but this change (hinging on this diff passing tests on the bots) seems an improvement enough on its own to accept.

This revision is now accepted and ready to land.Aug 8 2022, 3:33 PM

Paul Larson (GeorgiaPacific) added a subscriber: Paul Larson (GeorgiaPacific).Aug 8 2022, 7:19 PM

Jeroen Bakker (jbakker) accepted this revision.Aug 9 2022, 6:26 AM

Closed by commit rB9990792e8723: Compositor: Rename compositor build option (authored by Omar Emara (OmarSquircleArt)). · Explain WhyAug 9 2022, 4:00 PM

This revision was automatically updated to reflect the committed changes.

Omar Emara (OmarSquircleArt) added a commit: rB9990792e8723: Compositor: Rename compositor build option.