[libcamera-devel] [PATCH v5 07/12] libcamera: transform: Add operations with Orientation

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Fri, Sep 01, 2023 at 05:02:10PM +0200, Jacopo Mondi via libcamera-devel wrote:
> Add two operations that allows to combine Transform with Orientation.
> 
> - Transform operator/(Orientation1, Orientation2)

Let's write valid C++ code here:

	Transform operator/(const Orientation &o1, const Orientation &o2);

>   allows to easily get back the Transform that needs to be applied to
>   Orientation2 to get Orientation1
> 
> - Orientation operator*(Orientation1, Transform)
>   allows to apply a Transform to an Orientation and obtain the
>   combination of the two

As commented in 06/12, this could be

	Orientation operator()(const Orientation &o);

The downside is that the division operator won't be the natural inverse
operation. There's no easy shorthand notation for inverting a function
call. It may not be much of an issue though.

> These two operations allow applications to use Transforms to
> manipulate the Orientation inside the CameraConfiguration, if they
> wish.

I'll restate what I mentioned in what was (I think) a face to face
conversation, to avoid future surprises. Once we'll have a C API, I
expect these convenience helpers (as well as the geometry helpers for
instance) to be dropped from the libcamera official C ABI. We could
still provide them to applications as part of the C++ bindings though.

> Signed-off-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
> Reviewed-by: David Plowman <david.plowman at raspberrypi.com>
> ---
>  include/libcamera/transform.h |  3 +++
>  src/libcamera/transform.cpp   | 37 +++++++++++++++++++++++++++++++++++
>  2 files changed, 40 insertions(+)
> 
> diff --git a/include/libcamera/transform.h b/include/libcamera/transform.h
> index 14f1db0e8572..847a1f94713c 100644
> --- a/include/libcamera/transform.h
> +++ b/include/libcamera/transform.h
> @@ -74,6 +74,9 @@ Transform transformFromRotation(int angle, bool *success = nullptr);
>  Transform transformFromOrientation(const Orientation &orientation);
>  Orientation transformToOrientation(const Transform &transform);
>  
> +Transform operator/(const Orientation &o1, const Orientation &o2);
> +Orientation operator*(const Orientation &o1, const Transform &t);
> +
>  const char *transformToString(Transform t);
>  
>  } /* namespace libcamera */
> diff --git a/src/libcamera/transform.cpp b/src/libcamera/transform.cpp
> index d192d63d9290..80cd804ca530 100644
> --- a/src/libcamera/transform.cpp
> +++ b/src/libcamera/transform.cpp
> @@ -362,6 +362,43 @@ Orientation transformToOrientation(const Transform &transform)
>  	return Orientation::rotate0;
>  }
>  
> +/**
> + * \brief Return the Transform that applied to \a o2 gives \a o1
> + * \param o1 The Orientation to obtain
> + * \param o2 The base Orientation
> + *
> + * This operation can be used to easily compute the Transform to apply to a
> + * base orientation \a o2 to get the desired orientation \a o1.
> + *
> + * The CameraSensor class uses this function to compute what Transform to apply
> + * to a camera with mounting rotation \a o2 (the base) to obtain the user
> + * requested orientation \a o1.

I would drop this paragraph, it's an internal implementation detail, and
the CameraSensor class is not part of the public API.

> + *
> + * \return A Transform that applied to \a o2 gives \a o1
> + */
> +Transform operator/(const Orientation &o1, const Orientation &o2)
> +{
> +	Transform t1 = transformFromOrientation(o1);
> +	Transform t2 = transformFromOrientation(o2);
> +
> +	return -t2 * t1;
> +}
> +
> +/**
> + * \brief Apply the Transform \a t on the base orientation \a o

s/base //

> + * \param o The base orientation

s/base //

> + * \param t The transform to apply on \a o
> + * \return The Orientation resulting from applying \a t on \a o
> + */
> +Orientation operator*(const Orientation &o, const Transform &t)
> +{
> +	/*
> +	 * Apply a Transform corresponding to the base orientation first and
> +	 * then apply \a t to it.
> +	 */
> +	return transformToOrientation(transformFromOrientation(o) * t);
> +}
> +
>  /**
>   * \brief Return a character string describing the transform
>   * \param[in] t The transform to be described.
> 

-- 
Regards,

Laurent Pinchart