fix(compositor): never stretch the webcam, whatever box the layout gives it - #147
Merged
Merged
Conversation
…ves it The side-by-side preset renders the camera visibly squashed. `computeCompositeLayout`'s dual-frame branch hands the webcam a column slot — `webcamRect = webcamSlot`, a fixed fraction of the width at full content height, with no aspect adjustment at all — and the compositor mapped the whole camera frame onto it. A 16:9 camera in a ~0.55 ratio box is squeezed to about half its width. The source crop was chosen from the webcam SHAPE, on the assumption stated in its own comment: "le dst matche le ratio de la source". That holds for the default placement, where `fit_cam_aspect` shapes the box from the camera's aspect, and for square/circle, which got a hand-written centre crop. It does not hold the moment the app supplies the rect — side-by-side, or a webcam dragged anywhere by the user. The invariant lived in the callers, so any preset could break it silently, and did. Fixed at the primitive instead: `cover_crop_uv` derives the source sub-rect from the box's ACTUAL aspect — the largest centred crop with that shape, i.e. `object-fit: cover`, which is also what the web preview already shows. The layer cannot be distorted no matter who produced the rect. The square/circle centre crop falls out as the 1:1 case and stops being special. This is not the anisotropy that the render-target refactor removed: that one came from the output ratio (canvas -> output), this one from source -> box fit. Different axis, which is why it survived. It predates the refactor — the old `inverse_undistort` + `apply_undistort` pair around this path composed to the identity. Safety property, and the reason this cannot regress the good cases: when the box already matches the camera aspect, the crop is the whole frame. All six golden formats hash bit-for-bit identical. Three unit tests pin the invariant (crop aspect always equals box aspect, crop stays inside the visible frame and centred, whole-frame when already matching), plus a golden that renders the column-slot case for eyeballing.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Le symptôme
Le preset side-by-side affiche la caméra visiblement écrasée.
La cause
La branche
dual-framedecomputeCompositeLayoutdonne à la caméra un slot de colonne —webcamRect = webcamSlot, une fraction fixe de la largeur sur toute la hauteur du contenu, sans le moindre ajustement d'aspect (compositeLayout.ts:439). Le compositeur y plaquait la frame caméra entière. Une caméra 16:9 dans une boîte de ratio ~0.55 perd la moitié de sa largeur.Le choix de la coupe source se faisait sur la forme de la webcam, sur une hypothèse que son propre commentaire énonçait : « le dst matche le ratio de la source ». Vrai pour le placement par défaut (
fit_cam_aspectfaçonne la boîte depuis l'aspect caméra) et pour square/circle (center-crop écrit à la main). Faux dès que l'app fournit le rect — side-by-side, ou une webcam déplacée par l'utilisateur. L'invariant vivait chez les appelants, donc n'importe quel preset pouvait le violer en silence. C'est arrivé.Le correctif — au niveau de la primitive, pas du cas
cover_crop_uvdérive le sous-rect source du ratio réel de la boîte : le plus grand rect centré ayant cette forme, soit exactementobject-fit: cover— ce que la preview web affiche déjà. La couche ne peut plus être déformée, quel que soit le producteur du rect. Le center-crop carré de square/circle en devient le cas 1:1 et cesse d'être traité à part.Avant / après
Slot en colonne, même frame source, ratio de boîte ~0.55 :
Pourquoi ça ne peut pas casser les cas déjà bons
Propriété de sûreté : quand la boîte a déjà le ratio de la caméra, la coupe est la frame entière. Les placements corrects (PiP par défaut, vertical-stack, square/circle) sont donc inchangés à l'octet près.
feat/ai-editioncargo test: 17 unitaires + 2 golden, verts.Non couvert
Pas encore validé dans l'app — vérifié par rendu golden hors-app (même chemin que l'export :
Compositor+Player). Le rendu réel du side-by-side dans l'éditeur reste à confirmer visuellement.