Skip to content

Fix aspect ratio issues with preview#281

Merged
owahltinez merged 1 commit intoandroid:masterfrom
lnishan:master
Aug 20, 2020
Merged

Fix aspect ratio issues with preview#281
owahltinez merged 1 commit intoandroid:masterfrom
lnishan:master

Conversation

@lnishan
Copy link
Copy Markdown
Contributor

@lnishan lnishan commented Aug 20, 2020

It seems that ConstraintLayout resizes children so that they don't clip.
This creates problems for AutoFitSurfaceView. AutoFitSurfaceView
attempts to set the measured dimension to be "just bigger" than the
viewfinder and expect that the overflowing pixels would be clipped,
creating the desired "center-crop" effect.
This commit changes the parent layout to FrameLayout.

TEST=Deployed on Pixel 4 AVD and verify the aspect ratios look right in
the test scene.

@lnishan
Fix aspect ratio issues with preview
1b776ce 
It seems that ConstraintLayout resizes children so that they don't clip.
This creates problems for AutoFitSurfaceView. AutoFitSurfaceView
attempts to set the measured dimension to be "just bigger" than the
viewfinder and expect that the overflowing pixels would be clipped,
creating the desired "center-crop" effect.
This commit changes the parent layout to FrameLayout.

TEST=Deployed on Pixel 4 AVD and verify the aspect ratios look right in
the test scene.
@lnishan
Copy link
Copy Markdown
Contributor Author

lnishan commented Aug 20, 2020

This should fix #256 I think

@owahltinez
Copy link
Copy Markdown
Contributor

owahltinez commented Aug 20, 2020

Thank you Jasmine, very interesting finding! Did you file a bug internally, or do you think ConstraintLayout's behavior is WAI?

@owahltinez owahltinez merged commit 317364c into android:master Aug 20, 2020
@owahltinez owahltinez mentioned this pull request Aug 20, 2020
Closed
@lnishan
Copy link
Copy Markdown
Contributor Author

lnishan commented Aug 21, 2020 •
edited
Loading

Hi Oscar,

I didn't file a bug internally. I was hoping to find some references that might answer my question but didn't find anything conclusive.
What I did find was a curious line in our documentation

Note: You cannot use match_parent for any view in a ConstraintLayout. Instead use "match constraints" (0dp).

The AutoFitSurfaceView is set to "match_parent", which seems to be disallowed according to that line. So it's at the very least, undefined behavior.
I did try to use match_constraint and constrain all 4 sides. The result of that was the same, and this was WAI I think because I was essentially telling ConstraintLayout to "stretch" the children (viewfinder) to all 4 sides.
Based off of that, I think ConstraintLayout is WAI here.

@guillaume-tgl guillaume-tgl mentioned this pull request Aug 25, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@lnishan @owahltinez