Skip to content

[camera_platform_interface] Add rgba8888 format to ImageFormatGroup#11765

Open
Mairramer wants to merge 7 commits into
flutter:mainfrom
Mairramer:feat/rgba8888-platform-interface
Open

[camera_platform_interface] Add rgba8888 format to ImageFormatGroup#11765
Mairramer wants to merge 7 commits into
flutter:mainfrom
Mairramer:feat/rgba8888-platform-interface

Conversation

@Mairramer
Copy link
Copy Markdown
Contributor

Part of #11632

Pre-Review Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the gemini-code-assist bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.

Footnotes

  1. Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. 2

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds the rgba8888 format to the ImageFormatGroup enum and implements platform-specific mappings for Android and iOS. Review feedback recommends adding unit tests to verify the conversion of platform-specific integer values and updating documentation links for consistency with existing code.

Comment thread packages/camera/camera_platform_interface/lib/src/types/image_format_group.dart Outdated
…format_group.dart

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
@Mairramer Mairramer changed the title Add rgba8888 format to ImageFormatGroup [camera_platform_interface] Add rgba8888 format to ImageFormatGroup May 22, 2026
@stuartmorgan-g stuartmorgan-g added the federated: partial_changes PR that contains changes for only a single package of a federated plugin change label May 26, 2026
@bparrishMines bparrishMines added the CICD Run CI/CD label May 27, 2026
Copy link
Copy Markdown
Contributor

@bparrishMines bparrishMines left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@stuartmorgan-g or @tarrinneal for secondary review

@stuartmorgan-g
Copy link
Copy Markdown
Collaborator

The addition looks good, but the two switch analyzer errors will need temporary // ignores, which the follow-up PR should clean up.

@Mairramer
Copy link
Copy Markdown
Contributor Author

The addition looks good, but the two switch analyzer errors will need temporary // ignores, which the follow-up PR should clean up.

Done.

@stuartmorgan-g
Copy link
Copy Markdown
Collaborator

You added case handling, not ignores. That will fail our safety check against modifying code across dependency layers in a federated plugin.

You need to ignore the warning in this PR, and add the cases in the next PR.

@Mairramer
Copy link
Copy Markdown
Contributor Author

You added case handling, not ignores. That will fail our safety check against modifying code across dependency layers in a federated plugin.

You need to ignore the warning in this PR, and add the cases in the next PR.

Yes, I messed up. I'll fix it right away.

@Mairramer
Copy link
Copy Markdown
Contributor Author

@stuartmorgan-g I believe it should pass now, hopefully.

@stuartmorgan-g stuartmorgan-g added the CICD Run CI/CD label May 29, 2026
@github-actions github-actions Bot removed the CICD Run CI/CD label May 29, 2026
@stuartmorgan-g stuartmorgan-g added the CICD Run CI/CD label May 29, 2026
@stuartmorgan-g
Copy link
Copy Markdown
Collaborator

I'm very confused by the compile error here; I thought this was only an analyzer warning as long as it wasn't a switch expression (e.g., return switch ...). Did the behavior change?

@stuartmorgan-g
Copy link
Copy Markdown
Collaborator

Wow, apparently it changed several years ago for enums and we hadn't hit it until now.

So there will need to be a prequel PR to change all of those constructions to use default instead of the current return-outside-switch approach. Which unfortunately means that we won't get warnings about places we need to update any more, and it'll just have to be feature-test driven.

@Mairramer
Copy link
Copy Markdown
Contributor Author

Wow, apparently it changed several years ago for enums and we hadn't hit it until now.

So there will need to be a prequel PR to change all of those constructions to use default instead of the current return-outside-switch approach. Which unfortunately means that we won't get warnings about places we need to update any more, and it'll just have to be feature-test driven.

How to proceed in this case?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CICD Run CI/CD federated: partial_changes PR that contains changes for only a single package of a federated plugin change p: camera platform-android platform-ios platform-macos

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants