TransformedBbox, Transform.transform_bbox, and Bbox.transformed have subtly different semantics

[anntzer]

TransformedBbox constructs an object that tracks changes in the original bbox and the transform (it is dynamically updated when either parent changes), wheread Transform.transform_bbox and Bbox.transformed (as well as Bbox.inverse_transformed) return a "dumb" Bbox which forgets its parents. It was recently realized (#12031) that we are constructing a lot of transform objects and indeed, looking at the codebase, there are a few places where calls to TransformedBbox could be replaced by dumb transformed bboxes as we just want to get e.g. the transformed width and throw away the result immediately after.

I was going to submit a PR for that, but then I realized that the three transformation methods have subtly different semantics. The implementations (as of 3.0rc2) are copied here for convenience:

class TransformedBbox(BboxBase):
    ...
            p = self._bbox.get_points()
            # Transform all four points, then make a new bounding box
            # from the result, taking care to make the orientation the
            # same.
            points = self._transform.transform(
                [[p[0, 0], p[0, 1]],
                 [p[1, 0], p[0, 1]],
                 [p[0, 0], p[1, 1]],
                 [p[1, 0], p[1, 1]]])
            points = np.ma.filled(points, 0.0)

            xs = min(points[:, 0]), max(points[:, 0])
            if p[0, 0] > p[1, 0]:
                xs = xs[::-1]

            ys = min(points[:, 1]), max(points[:, 1])
            if p[0, 1] > p[1, 1]:
                ys = ys[::-1]

            self._points = np.array([
                [xs[0], ys[0]],
                [xs[1], ys[1]]
            ])

...

    def transform_bbox(self, bbox):
        return Bbox(self.transform(bbox.get_points()))

...

    def transformed(self, transform):
        pts = self.get_points()
        ll, ul, lr = transform.transform(np.array([pts[0],
            [pts[0, 0], pts[1, 1]], [pts[1, 0], pts[0, 1]]]))
        return Bbox([ll, [lr[0], ul[1]]])

transform_bbox is the simplest: it applies the transform to p0 and p1 (the two corners that specify the bbox) and constructs a new Bbox keeping their order.

TransformedBbox transforms the four corners of the bbox (the two explicit ones, p0 and p1, and the two implied ones, (x0, y1) and (x1, y0)) and constructs a new bbox from the min and max of these, keeping a possible inversion between min and max.

transformed transforms three points (p0 and the two "implied" corners) and constructs the new p0 from the original p0, and the new p1 by "mixing" the results for the two "implied" corners (this change came in in #1664 by @dopplershift).

---

Even if there is a good reason to keep all these different semantics (not clear, but possibly for handling e.g. skewed axes as in #1664), I think at least they should be renamed in a clearer way to know exactly what to use where...

[dopplershift]

I doubt the differing semantics are actually a good thing. The changes in #1664 were to make panning/zooming work on a skewed Axes. I think I would change transform_bbox to do:

return bbox.transformed(self)

Not only is that shorter, but it seems to properly delegate the work and bleed less of the details of Bbox into Transform.

It's possible TransformedBbox has a better/more robust implementation of the approach taken in bbox.transformed(). I can't think of any reason they should be different.

[efiring]

@mdboom, your insight would be especially valuable here and in #13110.

[github-actions]

This issue has been marked "inactive" because it has been 365 days since the last comment. If this issue is still present in recent Matplotlib releases, or the feature request is still wanted, please leave a comment and this label will be removed. If there are no updates in another 30 days, this issue will be automatically closed, but you are free to re-open or create a new issue if needed. We value issue reports, and this procedure is meant to help us resurface and prioritize issues that have not been addressed yet, not make them disappear. Thanks for your help!