Is cv::Mat class flawed by design?
c++, oop, opencv, resources
Solution
Yes, this is a poor design. Because `Mat` implements shared ownership internally, it’s not compatible with the standard way of choosing ownership policy, namely smart pointers. The basic issue is that data and ownership are orthogonal, and ought to be separated.
Because it is mutable, even a `const Mat` is more like a `const shared_ptr<Mat>`, with no way of describing that the contained `Mat` ought to be mutable, i.e., `shared_ptr<const Mat>`. This bears great similarity to the problems with `final` in Java, if you’re familiar.
I believe you can skirt these issues by wrapping `Mat` in a class that exposes the same interface as `Mat`, but which implements copy-on-write behaviour atop the default shared implementation.
Problem
I work a lot with the OpenCV C++ interface and designed a number of classes which use Mat's as private resources. Recently, I got concerned about the Mat class, as it always uses image data as shared resource unless I explicitly call clone. Even if I write `const Mat` I can't be sure that the imagedata does not get changed later from the outside. So I need to clone to ensure encapsulation. But the problem with needing to explicitly clone a Mat is that it is often unnecessary and expensive. On the other hand I understand that the need for shared imagedata originates from roi selectors, and being able to write something like this: `Mat m_small = m_big(my_roi)`. My questions are: 1.) Should not the cv::Mat class be rather lazily cloned? So the user will not see Mat's as shared resource handlers from the outside. Should not the user explicitly instantiate a class called something like `SharedMat` when a real shared imagedata is needed? 2.) Do you have any better strategy than always cloning in case of cv::Mat as private resource for a class? UPDATE: "you do not use `Mat::clone()` unless you plan to modify the data." (by Vadim Pisarevsky) This idea has a problem. Consider the situation that you have this class: ``` class Res_handler{ public: const Mat emit_mat(){ return m_treasure; } // I argue you are compelled to clone here. private: Mat m_treasure; }; ``` If you do not `clone` in this case you can write ``` Mat m_pirate = res_handler.emit_mat(); m_pirate = Scalar(0,0,0); ``` which causes a full black-out in the `m_treasure` inside the `res_handler` through the shared image data between `m_pirate` and `m_treasure`. :) So to avoid accidental modification of the inner `m_treasure`, you need to `clone` it. On the other hand, this solution is also flawed: ``` const Mat m_pirate = res_handler.emit_mat(); ``` because `m_treasure` can get modified too, so the content of `m_pirate` gets changed in the background, causing great headache to the pirate's programmer. :)