Add Impeller migration step for camera plugin authors #11008
Conversation
|
Visit the preview URL for this PR (updated for commit c9bde67): https://flutter-docs-prod--pr11008-impeller-packages-xp5p23dn.web.app |
camsim99
requested review from
sfshaza2,
parlough,
domesticmouse and
MaryaBelanger
as code owners
src/content/release/breaking-changes/android-surface-plugins.md
Outdated
Show resolved
Hide resolved
| to the equation: | ||
|
|
||
| ```plaintext | ||
| rotation = (sensorOrientationDegrees - deviceOrientationDegrees * sign + 360) % 360 |
There was a problem hiding this comment.
Should we provide more context on how to retrieve sensorOrientationDegrees and deviceOrientationDegrees
directly in this document? (instead of pointing people to a GitHub PR)
We provide code samples for other things in this migration guide (see line 59 in this file), so can we do the same for this addition as well? I don't feel strongly about this, but maybe not everyone writing plugins / reading this doc is okay with going through a potentially long Github PR to find what they need, to migrate.
I would definitely keep the PR link, though, so people have a completed migration to look at as well.
There was a problem hiding this comment.
I added some links for further clarification because I agree that the PR isn't enough. We also had to make some Flutter camera plugin-specific fixes in it, so I don't want the focus to be there.
I think I'll leave it here unless others feel strongly because I think camera plugin authors are familiar with the APIs and could make the migration with this information.
|
@parlough Does this need another review from the devrel team or can this be merged? |
There was a problem hiding this comment.
Hi, @camsim99! There are a few tech writery things I'd like to see fixed. (Writing in present tense, not using "may" which is specified in our writer's guide, linking to the main API site now that the API is live.
Also, there appears to be a link definition that isn't used in the text. Once this little things are cleared up, we can land!
src/content/release/breaking-changes/android-surface-plugins.md
Outdated
Show resolved
Hide resolved
src/content/release/breaking-changes/android-surface-plugins.md
Outdated
Show resolved
Hide resolved
src/content/release/breaking-changes/android-surface-plugins.md
Outdated
Show resolved
Hide resolved
src/content/release/breaking-changes/android-surface-plugins.md
Outdated
Show resolved
Hide resolved
src/content/release/breaking-changes/android-surface-plugins.md
Outdated
Show resolved
Hide resolved
src/content/release/breaking-changes/android-surface-plugins.md
Outdated
Show resolved
Hide resolved
src/content/release/breaking-changes/android-surface-plugins.md
Outdated
Show resolved
Hide resolved
src/content/release/breaking-changes/android-surface-plugins.md
Outdated
Show resolved
Hide resolved
src/content/release/breaking-changes/android-surface-plugins.md
Outdated
Show resolved
Hide resolved
| @@ -117,6 +147,11 @@ Relevant PRs: | |||
| [`createSurfaceTexture`]: {{site.api}}/javadoc/io/flutter/view/TextureRegistry.html#createSurfaceTexture() | |||
| [`getSurface()`]: {{site.api}}/javadoc/io/flutter/view/TextureRegistry.SurfaceProducer.html#getSurface() | |||
| [`setCallback`]: https://main-api.flutter.dev/javadoc/io/flutter/view/TextureRegistry.SurfaceProducer.html#setCallback(io.flutter.view.TextureRegistry.SurfaceProducer.Callback) | |||
| [`CameraCharacteristics.SENSOR_ORIENTATION`]: https://developer.android.com/reference/android/hardware/camera2/CameraCharacteristics#SENSOR_ORIENTATION | |||
| [this logic in our camera plugin]: https://github.com/flutter/packages/blob/d9a6de802e1fa74b377721929bfa6de5716ce6d9/packages/camera/camera_android_camerax/android/src/main/java/io/flutter/plugins/camerax/DeviceOrientationManager.java#L127 | |||
There was a problem hiding this comment.
@camsim99, "this logic in our camera plugin" text doesn't seem to appear in the PR. At least I couldn't find it. If not, can we delete this definition? Or make this a valid link. thx
There was a problem hiding this comment.
Oops! Let me remove this.
Co-authored-by: Shams Zakhour (ignore Sfshaza) <[email protected]>
Co-authored-by: Shams Zakhour (ignore Sfshaza) <[email protected]>
Co-authored-by: Shams Zakhour (ignore Sfshaza) <[email protected]>
Co-authored-by: Shams Zakhour (ignore Sfshaza) <[email protected]>
Co-authored-by: Shams Zakhour (ignore Sfshaza) <[email protected]>
Co-authored-by: Shams Zakhour (ignore Sfshaza) <[email protected]>
Co-authored-by: Shams Zakhour (ignore Sfshaza) <[email protected]>
Co-authored-by: Shams Zakhour (ignore Sfshaza) <[email protected]>
Co-authored-by: Shams Zakhour (ignore Sfshaza) <[email protected]>
Co-authored-by: Shams Zakhour (ignore Sfshaza) <[email protected]>
Co-authored-by: Shams Zakhour (ignore Sfshaza) <[email protected]>
…into impeller_packages
Description of what this PR is changing or adding, and why:
Adds a note to the Impeller Android plugins migration to clarify that plugin authors that create a camera preview may additionally need to correct the rotation of the preview due to rotation information of produced
Surfaces being lost.Issues fixed by this PR (if any):
N/A
PRs or commits this PR depends on (if any):
Should land after flutter/packages#6856 as this is mentioned as an example.
cc @navaronbracke
Presubmit checklist