[00/11] ipa: libipa: Vector and Pwl improvements

20272 mbox series

Hello,

This patch series tries to improve the ipa::Vector and ipa::Pwl classes
that have been recently merged. They started from an attempt to replace
the readYaml() member of those two classes with a better (in my opinion)
alternative, and grew beyond that.

The main change in this series is the removal of the readYaml() function
from the Vector and Pwl classes. The rationale for this is that the two
classes are generic math objects, and should not be tied to a particular
serialization mechanism. For the same reason, we have currently no
Size::readYaml() function, but a specialization of YamlObject::get() for
the Size type.

Copying the implementation of YamlObject::get() for the Size type would
have been the most straightforward option, but isn't an option as the
Vector and Pwl classes, unlike the Size class, are part of libipa, while
YamlObject is part of libcamera. libipa links to libcamera, but
libcamera doesn't link to libipa.

As the YamlObject::get() function is a template function with support
for different data types implemented through template specializations, I
decided to try and implement specializations for the Vector and Pwl
classes inside libipa. This was blocked by std::enable_if_t guards in
YamlObject::get(), which are removed by patch 01/11.

The second issue is that specializing the function template for another
template type (e.g. ipa::Vector<T, Rows>) is a partial specialization of
a function, which is not allowed by C++. Only partial specializations of
classes and variables are supported by the language. Patch 02/11 works
around that issue by delegating the YamlObject::get() function template
to a new YamlObject::Getter structure template, which implements a
non-template get() function.

With that in place, a specialization of the getter for ipa::Vector is
implemented in patch 03/11. The ipa::Vector::readYaml() is then dropped
in patch 04/11 without further ado, as it turned out the function wasn't
used. I would be fine dropping patch 03/11 until reading a Vector from
YAML data becomes needed if preferred.

Patches 05/11 to 08/11 follow with miscellaneous cleanups and
enhancements for the ipa::Pwl class, in preparation of patch 09/11 that
adds a getter specialization for ipa::Pwl. Patch 10/11 switches the
users of the ipa::Pwl::readYaml() function to YamlObject::get() (unlike
ipa::Vector, we read ipa::Pwl instances from YAML data). This showcases
usage of the new API. Finally, patch 11/11 drops the
ipa::Pwl::readYaml() function.

Beside thinking that C++ and/or myself are crazy, does anyone have an
opinion ?

Laurent Pinchart (11):
  libcamera: yaml_parser: Drop std::enable_if_t guards for get()
    function
  libcamera: yaml_parser: Delegate YamlObject::get() to helper structure
  ipa: libipa: vector: Specialize YamlObject getter
  ipa: libipa: vector: Drop readYaml() function
  ipa: libipa: pwl: Suffix \param with direction
  ipa: libipa: pwl: Make the empty() function inline
  ipa: libipa: pwl: Add a size() function
  ipa: libipa: pwl: Add a constructor that moves a Point vector
  ipa: libipa: pwl: Specialize YamlObject getter
  ipa: rpi: controller: Replace Pwl::readYaml() with YamlObject::get()
  ipa: libipa: pwl: Drop readYaml() function

 include/libcamera/internal/yaml_parser.h   |  27 ++---
 src/ipa/libipa/pwl.cpp                     | 132 +++++++++++----------
 src/ipa/libipa/pwl.h                       |   6 +-
 src/ipa/libipa/vector.cpp                  |  30 +++--
 src/ipa/libipa/vector.h                    |  48 ++++----
 src/ipa/rpi/controller/rpi/af.cpp          |   2 +-
 src/ipa/rpi/controller/rpi/agc_channel.cpp |   9 +-
 src/ipa/rpi/controller/rpi/awb.cpp         |   3 +-
 src/ipa/rpi/controller/rpi/ccm.cpp         |   6 +-
 src/ipa/rpi/controller/rpi/contrast.cpp    |   4 +-
 src/ipa/rpi/controller/rpi/geq.cpp         |   6 +-
 src/ipa/rpi/controller/rpi/hdr.cpp         |   4 +-
 src/ipa/rpi/controller/rpi/tonemap.cpp     |   2 +-
 src/libcamera/yaml_parser.cpp              |  78 ++++++------
 14 files changed, 193 insertions(+), 164 deletions(-)


base-commit: 20b8538a197c45597eecf5c7a51058263e816eb9