Add Rect support for ObjectFit::affine.

[xStrom]

ObjectFit docs state that it provides strategies for inscribing a rectangle inside another rectangle. However, the ObjectFit::affine method only operates on a pair of Size structs.

This PR changes that to a pair of Rect structs so that the fitting strategies are also available for scenarios where the source/destination doesn't have a (0,0) origin.

[PoignardAzur]

So this new function adds a translation component to account for differences in Rect position? This need to be documented.

[xStrom]

Yes, it now translates arbitrary origins instead of just assuming (0,0). I added a comment to make it explicit.

[adamjgmiller]

Going to run a code review on this - hope i'm not misunderstanding the "needs review" label - been looking for a good public PR to test a new code review tool on.

[xStrom]

Sure, we welcome review by anyone. When doing it with AI, it's worth manually filtering the noise and only posting reasonable results. We don't want just unfiltered LLM output posted for every PR.

[adamjgmiller]

Code review

Branch: object-fit-rect → main
PR: #1791 (open)
Review ID: rev_01KRBXJHSRRGK155N7XTRXSS9X
Fix threshold: not yet set (run /adamsreview:fix [threshold] to apply fixes)
Sub-agent tokens: 494,030 across 16 invocations
Orchestrator tokens: 235,860 output / 22,547 input across 182 turns

Found 9 findings across all lanes:

• Deep lane (correctness/security): 1 uncertain
• Light lane (ux/policy/architecture): 1 uncertain

---

Deep lane — correctness & security

ℹ Uncertain (1)

#	Score	Impact	File	Issue
F014	45	correctness	masonry/src/tests/properties.rs:36-62	New tests only exercise Stretch and Contain; the translation logic that's shared with None, Cover, FitHeight, FitWidth, and ScaleDown (especially None, whose origin-translation semantics are non-obvious) is uncovered.

Phase 4 couldn't confirm decisively. Re-run /adamsreview:review if you suspect this deserves
further investigation with fresh context.

Light lane — ux, policy, architecture

#	Score	Impact	File	Finding	Disposition
F016	52	policy	masonry/src/properties/object_fit.rs:82	Breaking change to a public method signature (affine(Size, Size) -> affine(Rect, Rect)) without a deprecation shim or CHANGELOG note; downstream consumers of masonry will hit compile errors with no migration guidance.	uncertain

---

🤖 Generated with Adam's Claude Code Review Command

[adamjgmiller]

Sure, we welcome review by anyone. When doing it with AI, it's worth manually filtering the noise and only posting reasonable results. We don't want just unfiltered LLM output posted for every PR.

That's fair. I must admit, I am an AI-only developer with no knowledge of this repo. I hope the review isn't annoying despite that. Been developing this review tool that I like better than other AI reviewers I've tried: https://github.com/adamjgmiller/adamsreview

[xStrom]

Yeah that's the risk. The breaking change note is a good example of low quality feedback. There is no changelog at all right now for Masonry, also the compilation error of going from Size to Rect doesn't really need migration guidance even if we had a changelog, because it's too obvious.

The note about new tests only exercising Stretch and Contain is of more use. However, this remark misses the fact that the Image widget has existing screenshot tests that use all the ObjectFit variants. Thus, it's already tested that they all continue to work as expected.

The post also has too much low value info as extra noise. It could have been just the two paragraphs from the "Finding" columns. As it stands, more than 50% of the text is just noise.

[adamjgmiller]

Thank you for the feedback, this was helpful for me. And I apologize if I wasted your time.

[PoignardAzur]

"Correctly translated" doesn't give me a strong intuition for how this function will behave given rects with non-zero origin.

My guess would have been "it just adds a applies of container.origin - content.origin to the affine", but the code seems to do something more complex than that?

[xStrom]

Well it does do that, but also accounts for scale changes. In every case the content will end up being inside of container, but depending on exact fit strategy, some of content might overflow.

I'll give a brief example and its journey through this method.

10x10 Container, origin (10,10), end (20,20)
20x10 Content, origin (30,30), end(50,40)

ObjectFit::Contain

// Container vs Content factor
raw_scalex = 0.5
raw_scaley = 1.0

// For Contain we end up with this scale target
(scalex, scaley) = (0.5, 0.5)

// The following is the target origin for content
origin_x = 10 + (10 - (20 * 0.5)) * 0.5 = 10
origin_y = 10 + (10 - (10 * 0.5)) * 0.5 = 12.5

// To achieve the target size and origin, we calculate the affine
Affine::scale_x = 0.5
Affine::scale_y = 0.5
Affine::translate_x = 10 - 30 * 0.5 = -5
Affine::translate_y = 12.5 - 30 * 0.5 = -2.5

// Applying the affine later ends up giving us the correct origin and end for the content
(30,30) * affine = (30 * 0.5 + -5, 30 * 0.5 + -2.5) = (10,12.5)
(50,40) * affine = (50 * 0.5 + -5, 40 * 0.5 + -2.5) = (20,17.5)