Skip to content

(core): new pointer types to manage data in services

Description

Two new pointer types are introduced, which the only aim is to be used as service class members:

  • sight::data::ptr: single data
  • sight::data::ptr_vector: group of data

For instance,t they can be declared this way in a class declaration:

class STest : public IService
{
    ...
private:
    data::ptr<data::Image, data::Access::in> m_input {this, "image"};
    data::ptr_vector<data::Mesh, data::Access::inout> m_inoutGroup {this, "meshes", true, 2};
    data::ptr<data::Object, data::Access::out> m_output {this, "genericOutput", false, true};
};

this must be passed as the first argument, with a class instance inheriting from IHasData. So far, only IService inherits from this interface, but other cases might appear later. It is used to register the pointers in the OSR and get/set their content automatically, mainly with the AppManager (Xml based and C++ based). This prevents from calling registerObject() for each data in the service constructor (it was almost never done because this only breaks C++-based apps, but normally it should have been done everywhere). Actually, this registration method was removed from the public interface of IService so you can no longer call it and there is no risk of conflict. All occurrences were refactored to use these new pointer types.

To retrieve the data, it is simple as using a data::mt::weak_ptr, so you can simply call

    auto input = m_input.lock(); // returns a data::mt::locked_ptr<data::Image>
    const auto data = *input;  // returns a data::Image&
    const auto data_shared = input.get_shared(); // returns a data::Image::csptr 

    // Access data in a group using indexes
    for(int i = 0; i < m_inoutGroup.size(); ++i)
    {
       auto data = m_inoutGroup[i].lock();
       ...
    }

    // Access data in a group using for range loop - this gives access to the underlying map, 
    // that's why '.second' should be specified, while '.first' gives the index
    for(const auto& data : m_inoutGroup)
    {
        auto data = data.second.lock();
        ...
    }

    // Alternative using structured binding
    for(const auto&[index, data] : m_inoutGroup)
    {
       auto lockedData = data.lock();
       ...
    }

Closes #626 (closed) #620 (closed)

Extra modifications

About usage of shared_ptr

We tend to use shared_ptr everywhere with sight::data. This is a poor convention. Indeed, we should limit its usage in services. In libraries, we should use regular references or pointers, and not assume that we need a ref counter in functions. Anyway, most functions use refs on shared_ptr, which is pretty useless. It may confuse people thinking that they use shared_ptr whereas anyway, they rely on the caller to protect the life of the pointee. On top of that, when using locked_ptr like here, it forces to use the get_shared() method everywhere, whereas a simple * would be enough. So I started to experiment with switching to standard ref or pointer types in geometry::data::Matrix functions to demonstrate this point and show the benefit of this approach. This led to modify several services.

service::IService refactoring

A lot has been done on IService to clarify its API. This is still far from perfect since we still have all the data getters variants, but clarification has been made to highlight the difference between the registration of data and the assignment of data. Since the registration is now done with the data_ptr class, we now expose setters instead of registerInput/registerInOut. This is a breaking change, but every occurrence has been replaced, thus implying the usage of data_ptr in the modified services.

What's next?

The conversion of the whole codebase is of course not achieved in this MR. There is still a lot of work to do that. However, these new pointer types are completely compatible with the old system. Converting getLocked*/getWeak* is fairly straightforward, but converting getInput/Inout/Output can be challenging because of potential deadlocks.

I suggest finishing the whole conversion with a coding challenge during the summer.

How to test it?

  • Test the applications using services that have been modified: SightViewer, DicomXPlorer, and many tutorials
  • Experiment with a conversion inside a service of your choice 😃
Edited by Flavien BRIDAULT

Merge request reports