Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Fix CkBrowserImageDecoder conversion of images to ImageByteFormat.rawRgba and rawStraightRgba #52089

Merged
merged 3 commits into from
Apr 17, 2024

Conversation

jason-simmons
Copy link
Member

VideoFrames of decoded images typically contain BGRA format. The CkBrowserImageDecoder implementation of toByteData needs to convert that into RGBA format. If the output format is rawRgba then it also needs to apply premultipled alpha.

Also fixes an issue where _bgrToRgba was not converting all pixels in the image.

Fixes flutter/flutter#135409
Fixes flutter/flutter#144770

…Rgba and rawStraightRgba

VideoFrames of decoded images typically contain BGRA format.  The CkBrowserImageDecoder implementation of toByteData needs to convert that into RGBA format.  If the output format is rawRgba then it also needs to apply premultipled alpha.

Also fixes an issue where _bgrToRgba was not converting all pixels in the image.

Fixes flutter/flutter#135409
Fixes flutter/flutter#144770
@jason-simmons jason-simmons requested a review from yjbanov April 12, 2024 22:09
@github-actions github-actions bot added the platform-web Code specifically for the web engine label Apr 12, 2024
Copy link
Contributor

@yjbanov yjbanov left a comment

Choose a reason for hiding this comment

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

Thank you for fixing my bugs!

}

/// Based on Chromium's SetRGBAPremultiply.
int _premultiply(int value, int alpha) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It might make sense to inline this function using @pragma('dart2js:tryInline') as images can be pretty big and could take advantage of faster pixel processing logic.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added the inlining annotation

}

// Last resort, just return the original pixels.
return pixels.asByteData();
}

/// Mutates the [pixels], converting them from BGRX/BGRA to RGBA.
void _bgrToRgba(ByteBuffer pixels) {
final int pixelCount = pixels.lengthInBytes ~/ 4;
void _bgrToStraightRgba(ByteBuffer pixels, bool isBgrx) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This could be inlined too so that the compiler specializes the function for possible values of isBgrx, true and false.

@jason-simmons jason-simmons added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 17, 2024
@auto-submit auto-submit bot merged commit 7072872 into flutter:main Apr 17, 2024
29 checks passed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 17, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Apr 18, 2024
…146959)

flutter/engine@624730f...725ebd7

2024-04-17 [email protected] Roll Skia from 5958cd52db48 to a3a016537a8c (3 revisions) (flutter/engine#52208)
2024-04-17 [email protected] Roll Dart SDK from bd7627bca67b to 45dd87c2329c (1 revision) (flutter/engine#52207)
2024-04-17 [email protected] Fix CkBrowserImageDecoder conversion of images to ImageByteFormat.rawRgba and rawStraightRgba (flutter/engine#52089)
2024-04-17 [email protected] Roll Skia from a78dec0c0538 to 5958cd52db48 (2 revisions) (flutter/engine#52206)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
gilnobrega pushed a commit to gilnobrega/flutter that referenced this pull request Apr 22, 2024
…lutter#146959)

flutter/engine@624730f...725ebd7

2024-04-17 [email protected] Roll Skia from 5958cd52db48 to a3a016537a8c (3 revisions) (flutter/engine#52208)
2024-04-17 [email protected] Roll Dart SDK from bd7627bca67b to 45dd87c2329c (1 revision) (flutter/engine#52207)
2024-04-17 [email protected] Fix CkBrowserImageDecoder conversion of images to ImageByteFormat.rawRgba and rawStraightRgba (flutter/engine#52089)
2024-04-17 [email protected] Roll Skia from a78dec0c0538 to 5958cd52db48 (2 revisions) (flutter/engine#52206)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
autosubmit Merge PR when tree becomes green via auto submit App platform-web Code specifically for the web engine
Projects
None yet
2 participants