Some preliminary feedback.
Not sure we should keep adding things to existing RenderLayer from master branch. This is a subject of reworking in the render-layers branch. Doing changes in this area in master will likely lead to a duplicated work of porting this over to the new system.
Even if we want to do such change in master now, this change should be split into several smaller ones.
Dons't seems correct, debug passes are still are inside of a compile-time option. Meaning, for official releases we expose something to the interface which will never work.
Not really fond of such c++-ism. It's not necessarily easy to read and not strictly speaking needed here.
Also, don't use capitals for argument names.
Okay, the two issues you mentioned are fixed.
As for the render_layers branch - I'm not really up-to-date on the changes there, but these two patches don't really change anything regarding layers, so I'd expect that merging isn't too hard.
Anyways, render_layers is 2.8, and I don't see a reason why this patch would need to wait for 2.8 - it's fully forwards and backwards compatible and allows to add some long-awaited functionality - I already have locally:
- A patch for light groups - passes only including some lights
- An experimental patch for an AOV Output node that would allow the user to create as many passes as they want and plug any shader noodle into it
And could easily use the work from D2106 to implement a better ID matte system that supports DoF/MoBlur/transparency.
In addition, it allows external render plugin writers to implement passes in their own renderer.
Edit: Regarding splitting the changes, I already split them into 5 commits. Finer splitting isn't really possible without getting broken intermediate versions.
Some minor comments, overall seems fine.
Perhaps adding these kinds of features in master is ok if the person adding them then also does the merge into the 2.8 branch?
The order of these parameters seems a bit odd, particularly to have the number of channels and channel names separate. Perhaps use:
add_pass(name, num_channels, channel_names, view, layer);
Do we need to support adding a pass only to specific views? I'm not sure if there are practical use cases, and it goes beyond what users can do in the UI and what the compositor expects I think.
The code in this function could be simplified with BLI_findstring.
I guess someone who's familiar with the 2.8 changes should do a master->blender2.8 merge before merging this. Not only for this patch, but as a general practice now that blender2.8 and master started to differ quite a bit. Just to ensure merge conflicts in other areas are handled by people who are at least a bit familiar with them.