[{"id":27582,"web_url":"https://patchwork.libcamera.org/comment/27582/","msgid":"<CAHW6GY+VzeWoLYhdOfKoNaKvSyisudrxJ+SCn6AUKuOg3K2Bjg@mail.gmail.com>","date":"2023-07-18T11:49:55","subject":"Re: [libcamera-devel] [PATCH v3 06/10] libcamera: transform: Invert\n\toperator*() operands","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Jacopo\n\nThanks for doing this!\n\nOn Tue, 18 Jul 2023 at 11:52, Jacopo Mondi\n<jacopo.mondi@ideasonboard.com> wrote:\n>\n> The current definition of operator*(Transform t1, Transform t0) follows\n> the function composition notion, where t0 is applied first then t1 is\n> applied last.\n>\n> In order to introduce operator*(Orientation, Transform) where a\n> Transform is applied on top of an Orientation, invert the operand order\n> of operator*(Transform, Transform) so that usage of operator* with both\n> Orientation and Transform can be made associative.\n>\n> For example:\n>\n> Orientation o;\n> Transform t = t1 * t2\n> Transform t3 = o * t\n\nStrictly, o * t gives us an orientation and not a transform. But\notherwise I agree!\n\nReviewed-by: David Plowman <david.plowman@raspberrypi.com>\n\nThanks!\nDavid\n\n>              = o * (t1 * t2) = (o * t1) * t2 = o * t1 * t2\n>\n> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> ---\n>  src/libcamera/camera_sensor.cpp |  4 ++--\n>  src/libcamera/transform.cpp     | 19 +++++++++++--------\n>  2 files changed, 13 insertions(+), 10 deletions(-)\n>\n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index 3ba364c44a40..038d8b959072 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -1051,7 +1051,7 @@ Transform CameraSensor::validateTransform(Transform *transform) const\n>          * Combine the requested transform to compensate the sensor mounting\n>          * rotation.\n>          */\n> -       Transform combined = *transform * rotationTransform_;\n> +       Transform combined = rotationTransform_ * *transform;\n>\n>         /*\n>          * We combine the platform and user transform, but must \"adjust away\"\n> @@ -1080,7 +1080,7 @@ Transform CameraSensor::validateTransform(Transform *transform) const\n>                  * If the sensor can do no transforms, then combined must be\n>                  * changed to the identity. The only user transform that gives\n>                  * rise to this is the inverse of the rotation. (Recall that\n> -                * combined = transform * rotationTransform.)\n> +                * combined = rotationTransform * transform.)\n>                  */\n>                 *transform = -rotationTransform_;\n>                 combined = Transform::Identity;\n> diff --git a/src/libcamera/transform.cpp b/src/libcamera/transform.cpp\n> index c70cac3f14ee..d192d63d9290 100644\n> --- a/src/libcamera/transform.cpp\n> +++ b/src/libcamera/transform.cpp\n> @@ -189,14 +189,17 @@ Input image   | |   goes to output image   | |\n>   */\n>\n>  /**\n> - * \\brief Compose two transforms together\n> - * \\param[in] t1 The second transform\n> - * \\param[in] t0 The first transform\n> + * \\brief Compose two transforms by applying \\a t0 first then \\a t1\n> + * \\param[in] t0 The first transform to apply\n> + * \\param[in] t1 The second transform to apply\n> + *\n> + * Compose two transforms by applying \\a t1 after \\a t0. The operation\n> + * is conceptually equivalent to the canonical notion of function composition,\n> + * with inverse order of operands. If in the canonical function composition\n> + * notation \"f * g\" equals to \"f(g())\", the notation for Transforms composition\n> + * \"t0 * t1\" equals to \"t1(t0()))\" where \\a t0 is applied first, then \\a t1.\n>   *\n> - * Composing transforms follows the usual mathematical convention for\n> - * composing functions. That is, when performing `t1 * t0`, \\a t0 is applied\n> - * first, and then \\a t1.\n> - * For example, `Transpose * HFlip` performs `HFlip` first and then the\n> + * For example, `HFlip * Transpose` performs `HFlip` first and then the\n>   * `Transpose` yielding `Rot270`, as shown below.\n>  ~~~\n>               A-B                 B-A                     B-D\n> @@ -206,7 +209,7 @@ Input image  | |   -> HFLip ->   | |   -> Transpose ->   | |   = Rot270\n>   * Note that composition is generally non-commutative for Transforms,\n>   * and not the same as XOR-ing the underlying bit representations.\n>   */\n> -Transform operator*(Transform t1, Transform t0)\n> +Transform operator*(Transform t0, Transform t1)\n>  {\n>         /*\n>          * Reorder the operations so that we imagine doing t0's transpose\n> --\n> 2.40.1\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 75E25BDB1C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 18 Jul 2023 11:50:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D0A3C628C0;\n\tTue, 18 Jul 2023 13:50:08 +0200 (CEST)","from mail-oi1-x22f.google.com (mail-oi1-x22f.google.com\n\t[IPv6:2607:f8b0:4864:20::22f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EC42A61E2B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 18 Jul 2023 13:50:07 +0200 (CEST)","by mail-oi1-x22f.google.com with SMTP id\n\t5614622812f47-3a46da5cd6dso738951b6e.0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 18 Jul 2023 04:50:07 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1689681008;\n\tbh=70HQj5faqW0DLLW/QMFcxDr7p1WSPhmbgyCM0MTCMzk=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=QAgeQUkSH4QlijqRBanZcmFboMtGNl3p6hzYzMCzecr4MHY/kRLpV9isRqIIBXy41\n\tLAz+Q74zOlvwlMG84JpRCd6vdq4RHRUopvUxTLrciWAGeeeI43A2ReacX2k3UFLrYa\n\tMBeAAMCyWVpcK+X6GVtfTamcmooFAmxK7kGIh64DHBfPuObCSanVetERCjY6LYapgT\n\tHWzHx7QV6An4k59dlKhtlS3rDUdtTK114B7EPfueX74XdAQlmV6q21vr3++7t/5M0n\n\tx8wTrft8OLzWO9dHGycvVlLrw0NeTzi/QSMgtCrw29QK7263FcqCiGZTewelGdbp1Y\n\t1FR748yQ156sg==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1689681006; x=1692273006;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=LRJHw58/xns3iI3YyFUgyycTT0k7lJ/NCuwZLmp8dlA=;\n\tb=mXGUG/4kH7+WKrkRMfJ7GNiZkn+lwKmkLJBiTt2vm3fK21rGC5JnbMNwXTqbAN9O+V\n\tgtUrEKm6KfVbQEMtJmt3Totv0dnnjELK4kQcLHTXMI/a382BhetbyYdTEmkjnWHTWGYY\n\tJ98huqzLTbGqhb+XGeFLVCnpc0O3C1DHKBULm79QBUBY94Q+Q/DJsOhoi7V4NXG2wFUc\n\tiQS+Y8Lf4BAuFARmByfuWcb3SDbsJYyb0G4AZO+5Za1YUCGH9LMbsYqnfbd+z7Bt9G6l\n\tejQNEel/jBfNl4BXVCFiZsIubiMAJ+U8bVNh+ClYSjmn7YxdUFNYvnY7+pCd58/uAR3I\n\t6Ojw=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"mXGUG/4k\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20221208; t=1689681006; x=1692273006;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=LRJHw58/xns3iI3YyFUgyycTT0k7lJ/NCuwZLmp8dlA=;\n\tb=XnCYWqWdJXTiSKtXWaw2MEznm3FMS/Qka3SD4Mfm3vWhHYADcE8tUK0Jz8LgKpbr6x\n\tovr4qIdcwg4iViRLGNp+c1ieuI0ImJNq0BHxCWh/F5zkzVGooYlnoitcYXoicwp0j/zQ\n\tiQcuN0OnkvGK7RXwSYAgrXG09svu+t39bc1YXIYZ7q7XGMvijP2GFI23vBY6REQ65Gky\n\tlOBWOArAAdQm1d+v11g9gai9Ax/r4tI69SlJMERSKtiBoC4LLoFl+YTkm9YKH9HktN5l\n\t/VJoVNFmjoDnLJBhdGbtyp2aWiBi4jqc6nb1P8lmvZ1A8RLSKoKw/PIxmsUJcv2BL2vs\n\tGVZg==","X-Gm-Message-State":"ABy/qLaISULyzg4u7j8fqZ/BlonJrZOdQ4igS9r2aHk/xGW5h9CVElg5\n\twud59NAF+XkrUrGEwcac/6HjMUuocFjSYMWS6YWAeMEr+V53dHB9","X-Google-Smtp-Source":"APBJJlERj1XqA/a++DXRXyTS/0TjHeYTwdOjyeb7Ys+NiIkpdofmA7k2++cZjtTDI4LofwxKK8caUrrVys6ZEHskU+c=","X-Received":"by 2002:aca:f246:0:b0:396:169f:3660 with SMTP id\n\tq67-20020acaf246000000b00396169f3660mr11687843oih.58.1689681006601;\n\tTue, 18 Jul 2023 04:50:06 -0700 (PDT)","MIME-Version":"1.0","References":"<20230718105210.83558-1-jacopo.mondi@ideasonboard.com>\n\t<20230718105210.83558-7-jacopo.mondi@ideasonboard.com>","In-Reply-To":"<20230718105210.83558-7-jacopo.mondi@ideasonboard.com>","Date":"Tue, 18 Jul 2023 12:49:55 +0100","Message-ID":"<CAHW6GY+VzeWoLYhdOfKoNaKvSyisudrxJ+SCn6AUKuOg3K2Bjg@mail.gmail.com>","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v3 06/10] libcamera: transform: Invert\n\toperator*() operands","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"David Plowman via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"David Plowman <david.plowman@raspberrypi.com>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]