[libcamera-devel] [PATCH v2 6/9] libcamera: transform: Add operations with Orientation

[Jacopo Mondi]

Hi David

On Mon, Jul 17, 2023 at 03:01:52PM +0100, David Plowman via libcamera-devel wrote:
> Hi

[snip]

> > > On a semi-related note, I find myself second-guessing the order of
> > > transform composition. The documentation is pretty clear that we're
> > > using traditional mathematical composition rules, but does that still
> > > make sense?
> > >
> > > We defined operator*(orientation, transform) so that we have:
> > >
> > > o1 * t = o2
> > >
> > > but if t = t1 * t2 then (according to the current rules)
> > >
> > > o2 * (t1 * t2) = o2 and therefore (o2 * t2) * t1 = o2 so in fact o2 *
> > > t2 * t1 = o2. Or in other words, these two flavours of * aren't
> > > associative. It all feels wrong.
> >
> > I got the same doubt to be honest. I just found more natural to have
> >
> > Orientation = Orientation * transform
> >
> > which reads "The Transform you have to apply to an Orientation o1 to
> > get Orientation o2" as an API, but I agree the fact I then need to
> > swap operands in the implementation to apply the orientation first
> > then the transform is misleading, even more so with multiple
> > compositions as per your example
> >
> > > Having a prefix version, namely operator*(transform, orientation)
> > > would fix this nastiness, but I do wonder whether it would be more
> > > natural to change the transform order when we compose them. Opinions
> > > sought!!
> >
> > I honestly find the composition notion implemented by
> > operator*(Transform t1, Transform t2) a little bit confusing, but it
> > is indeed more formally correct.
> >
> > I guess we have to chose between
> >
> > 1)
> >
> > operator*(Transform t1, Transform t2) = Apply t2 then t1
> > operator*(Transform t, Orientation o) = Apply o then t
> >
> > 2)
> >
> > operator*(Transform t1, Transform t2) = Apply t1 then t2
> > operator*(Orientation o, Transform t) = Apply o then t
> >
> > The only argument I have is the one I expressed above:
> >
> > "Orientation * transform" as "the Transform t applied to Orientation o"
> >
> > reads more naturally than
> >
> > "Transform * Orientation" as "the Transform t applied to Orientation o"
> >
> > But it's not a strong opinion. What is your preference ?
>
> Yes, this is exactly it, we need to choose either 1 or 2 to be
> consistent. The mere fact that, the other week, I automatically went
> for operator*(Orientation, Transform) probably underlines that 2 is
> more natural, and I would therefore prefer to change to this view of
> the world. But maybe we need to see if others are OK for us to do
> this!

Assuming nobody else has different ideas, would you like me to change
the operation semantic as part of this series or do you prefer to do
it yourself and update your consumers (libcamera-apps and picamera2)
at the same time ?

>
> David
>
> >
> > >
> > > Thanks
> > > David
> > >
> > > > +}
> > > > +
> > > >  /**
> > > >   * \brief Return a character string describing the transform
> > > >   * \param[in] t The transform to be described.
> > > > --
> > > > 2.40.1
> > > >