Make SubRenderFeature.AttachRootRenderFeature public

[Redwarx008]

PR Details

allow custom render features outside the engine assembly to reuse internal SubRenderFeature attachment logic

Related Issue

Types of changes

• Docs change / refactoring / dependency upgrade
• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• My change requires a change to the documentation.
• I have added tests to cover my changes.
• All new and existing tests passed.
• I have built and run the editor to try this change out.

[Ethereal77]

As far as I can tell, SubRenderFeature is intended to be used to implement custom rendering for a render feature that operates on something (e.g., like ForwardLightingRenderFeature, which is a SubRenderFeature of MeshRenderFeature).

So I think it is internal because the intent is not to be called publicly, but for its containing render feature to "register it" for itself (so the child knows its parent). There is usually high coupling between a feature and its sub-features, so I see no value in exposing this mechanism publicly.

If you are developing a RootRenderFeature that needs to register its own sub-features, maybe it will make more sense to declare that method as protected internal instead of public. Then, you would implement your custom attaching / registering logic for your specific SubRenderFeature and there call the inherited protected void AttachRootRenderFeature(RootRenderFeature rootRenderFeature).

[Ethereal77]

Maybe some people that has some more experience with the rendering part can chime in.
@xen2 @johang88

[Redwarx008]

protected internal

Sometimes I want to be able to directly reuse SubRenderFeature under MeshRenderFeature instead of inheriting it first.

[Ethereal77]

Yes, I understand the convenience. But I think they were designed that way because they are (were) tightly coupled to the MeshRenderFeature class. I don't know if making it public breaks encapsulation.

Making them internal and protected may also break encapsulation (I want a second opinion), but at least allows you to derive it and call the attachment from your own logic. So, for example, if you have MyRenderFeature : RootRenderFeature and

public class MyLightingSubRenderFeature : ForwardLightingRenderFeature
{
    internal /* or public */ void Attach(MyRenderFeature myParentRenderFeature)
    {
        // You can call it here because it is protected
        base.AttachRootRenderFeature(myParentRenderFeature);
        /* ... your custom logic */
    }
}