Interface to access reflection probe data from external modules

[Bindless-Chicken]

In #7189 the reflection probe private header was moved to the private folder breaking a 3rd party module (PopcornFX) that was making use of some of the private functions being exposed that way.
This issue will be used to discuss solutions that can be put in place to solve that particular problem but also in a more generic fashion, expose data that live behind a feature processor, as other system may need to offer the same functionality in the future.

[Bindless-Chicken]

The function that is causing the issue at hand is used in two places in the 3rdParty module:

• https://github.com/PopcornFX/O3DEPopcornFXPlugin/blob/development/Code/Source/Integration/Render/AtomIntegration/PopcornFXFeatureProcessor.cpp#L216
• https://github.com/PopcornFX/O3DEPopcornFXPlugin/blob/development/Code/Source/Integration/Render/AtomIntegration/PopcornFXFeatureProcessor.cpp#L281

The user in that use case query the list of probes registered to the feature processor, to then get the bounds information and texture that can later be used during the shading of the particles.

The FindReflectionProbes function being used here is not exposed through the ReflectionProbeFeatureProcessorInterface and is only defined in the specific implementation of that interface ReflectionProbeFeatureProcessor. The function performs a AABB bound check against every probe registered and returns an unsorted array of pointers to the probes.

To make this behaviour work again, we would need to define the function FindReflectionProbes in the interface as well as define a new ReflectionProbeInterface with appropriates getters (and maybe setters) for every fields.

I would like to present a different approach that does not require making such changes. A feature processor is, if I understood the architecture correctly, responsible for creating the draw packets for the objects that were passed to it by the components. As such, I don't think a feature processor should answer this type of spatial queries.

Based on that, I would propose the following alternate flow:

1. Spatial Query using the IVisibilitySystem
2. Filter the results for the entities with the correct components
3. Get the handle for the feature processor from the component
4. Query the data through the feature processor using the component handle

This approach present the benefit of not exposing the internal data or its implementation thus reducing the potential for API breaking changes, but it most importantly uses an common spatial query system that is already optimized saving every system developer from implementing and maintaining their own.

[dmcdiarmid-ly]

I have most of this work already planned for the ReflectionProbeFeatureProcessorInterface, with the exception of the separate interface for the ReflectionProbe. Feature processors are generally functional/handle based, so I need to add a few more accessors for various probe properties.

I'm also planning to convert the query to use the visibility system. When this was originally written the visibility system was not ready for it, but it can be converted now.

Feature processors are not limited to draw call actions, they can basically do whatever is needed to provide an external interface to the system, so the query can stay in the ReflectionProbeFeatureProcessor.

[dmcdiarmid-ly]

I would also recommend moving the ReflectionProbeFeatureProcessor.h file back to its original location to unblock PopcornFX until this work can be completed.

[AMZN-alexpete]

I would also recommend moving the ReflectionProbeFeatureProcessor.h file back to its original location to unblock PopcornFX until this work can be completed.

I agree, we should restore the API until the other interface supports the PopcornFX use case.

[Bindless-Chicken]

I totally agree with you @dmcdiarmid-ly, couple of getter functions missing in the feature processor and we should be good to go.

But shouldn't the visibility queries be handled by the client, in that case the 3rd party module? It would remove any kind of dependency on Atom side, thus keeping it free to release on its own. It would also make for a much simpler change for us to do. The 3rd party module would just need to add a small snippet, something as simple as that:

          {
              AZ::Aabb aabb = AZ::Aabb::CreateFromMinMax({ -10, -10, -10 }, { 10, 10, 10 });
              AZ::Interface<AzFramework::IVisibilitySystem>::Get()->GetDefaultVisibilityScene()->Enumerate(
                  aabb,
                  [](const AzFramework::IVisibilityScene::NodeData& node)
                  {
                      for (const auto& entry : node.m_entries)
                      {
                          if (entry->m_typeFlags == AzFramework::VisibilityEntry::TYPE_Entity)
                          {
                              AZ::Entity* entity = static_cast<AZ::Entity*>(entry->m_userData);
                              AZ::Component* comp = entity->FindComponent("{6EBF2E41-2918-48B8-ACC3-FB115ED09E64}");
                              if (comp != nullptr)
                              {
                                  // Enqueue the reflection probe component and/or use the handles to get the resources
                              }
                          }
                      }
                  });
          }

and then the feature processor would just expose the few getters needed for the 3rd party module to access through the component's handle.

It's a change we could very easily and quickly implement to unblock PopcornFX without revering to exposing that private file.
What do you think?

[dmcdiarmid-ly]

I'd like to move the file back for now since it's a trivial fix to unblock PopcornFX.

There are other considerations I need to look into with regard to the use of the visibility system by reflection probes, and some restructuring I want to do to address other issues.