Skip to content

fix(layout): fit the screen box to the crop, and honour it without a camera - #149

Closed
EtienneLescot wants to merge 0 commit into
feat/ai-editionfrom
claude/crop-aspect-regression
Closed

fix(layout): fit the screen box to the crop, and honour it without a camera#149
EtienneLescot wants to merge 0 commit into
feat/ai-editionfrom
claude/crop-aspect-regression

Conversation

@EtienneLescot

@EtienneLescot EtienneLescot commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Le symptôme

Un clip recadré s'affiche étiré. Crop 30% × 89% d'une source 16:9 → l'image est écrasée horizontalement d'environ 2,8×.

Diagnostic

Ni un ancien patch resté en place, ni un défaut du compositeur, ni un comportement Electron. Deux défauts indépendants, un de chaque côté du contrat de scène.

1. L'app envoie un rect écran dont le ratio ignore le crop

buildSceneDescription résolvait le layout avec screenSize: { width: 1920, height: 1080 }, en dur.

C'était sans conséquence tant que seul webcamRect était consommé de ce layout : la boîte écran ne servait qu'à borner le slot caméra, sa forme exacte n'avait aucune importance. Elle est devenue porteuse quand ce3acf95 a commencé à envoyer screenRect au natif comme un rect consommé tel quel, documenté « already at the crop's aspect ratio ». Il ne l'était pas.

Les deux autres consommateurs de computeCompositeLayout passaient déjà la taille recadrée :

Consommateur screenSize
PreviewCanvas croppedScreenSize — avec un commentaire décrivant précisément ce mode de panne
frameRenderer (export WebCodecs) croppedVideoWidth/Height
sceneDescription (natif) {1920, 1080} en dur

Le builder natif était le seul à diverger. Il dérive désormais la même valeur des dimensions réelles du premier clip visible — la convention « unités sources du premier asset visible » que son propre commentaire revendiquait déjà.

2. Le natif n'honorait le rect écran que si un rect caméra l'accompagnait

Le bras de match qui appliquait app_screen_rect était indexé sur app_webcam_rect == Some. Sans caméra, l'écran gardait le rect plein cadre du preset — pendant que fit_screen sautait quand même son propre fit au ratio du crop, puisqu'un app_screen_rect était bien présent.

Un clip recadré sans caméra était donc étiré sans qu'aucune des deux voies ne le rattrape. Écran et caméra sont deux calques indépendants ; chaque rect résolu par l'app remplace maintenant sa contrepartie du preset de son côté.

Trouvé en rendant le cas, pas en lisant le code — le golden écrit pour vérifier le défaut n°1 a fait tomber le n°2.

Alignement avec la refonte ratio

Même principe que cover_crop_uv : l'invariant ne doit pas vivre chez l'appelant. Ici il y était deux fois — une hypothèse sur le ratio du rect côté TS, un couplage à la présence de la caméra côté Rust. Aucun cas particulier ajouté ; le n°2 retire une condition au lieu d'en ajouter une.

Non-régression

  • 6/6 hashes du golden identiques — aucun cas qui marchait n'a bougé d'un pixel
  • 3 tests de scène épinglent le ratio du rect contre le crop : bande verticale, letterbox large, sans crop
  • 1 golden rend le clip recadré pour inspection visuelle
  • 17 tests Rust + 1158 tests TS verts

Non couvert

Vérifié par rendu golden hors-app (même chemin que l'export). Le rendu dans l'éditeur reste à confirmer visuellement — je peux redéployer le .node et relancer l'app si tu veux boucler dessus.

@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 1144bc7f-2304-478b-8525-2d7f670e7e5c

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/crop-aspect-regression

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@EtienneLescot
EtienneLescot force-pushed the claude/crop-aspect-regression branch from 04073bf to 5c741e7 Compare July 24, 2026 13:36
@EtienneLescot
EtienneLescot force-pushed the claude/crop-aspect-regression branch from 5c741e7 to ed97fce Compare July 24, 2026 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant