Added float32 support to decodeImageFromPixels #40068
Conversation
gaaclarke
force-pushed
the
decodeImageFromPixels-float32
branch
from
2c50f7b to
b43c685
Compare
|
I'm going to move this back to a draft until #40031 lands. I suspect that as this is written it will create a texture that is in the f32 format. That can cause things like getByteData to fail since it is looking for f32 or bgr10_xr. We can add that test after that lands. |
gaaclarke
force-pushed
the
decodeImageFromPixels-float32
branch
2 times, most recently
from
62453fb to
a54af45
Compare
gaaclarke
force-pushed
the
decodeImageFromPixels-float32
branch
from
a54af45 to
0fb933c
Compare
|
@dnfield the dependent PR has landed so this is ready for review now. |
| case kRGBA_F32_SkColorType: | ||
| return kRGBA_F16_SkColorType; |
There was a problem hiding this comment.
Why not
| case kRGBA_F32_SkColorType: | |
| return kRGBA_F16_SkColorType; | |
| case kRGBA_F32_SkColorType: | |
| case kRGBA_F16_SkColorType: | |
| return kRGBA_F16_SkColorType; |
There was a problem hiding this comment.
There isn't a way to send in kRGBA_F16_SkColorType directly. If we decide to implement that it will probably be better to do that in its own pr with its own tests.
| switch (pixel_format) { | ||
| case PixelFormat::kRGBA8888: | ||
| color_type = kRGBA_8888_SkColorType; | ||
| break; | ||
| case PixelFormat::kBGRA8888: | ||
| color_type = kBGRA_8888_SkColorType; | ||
| break; | ||
| case PixelFormat::kRGBAFloat32: | ||
| color_type = kRGBA_F32_SkColorType; | ||
| alpha_type = kUnpremul_SkAlphaType; |
There was a problem hiding this comment.
Why are these unpremul but the other ones are premul?
There was a problem hiding this comment.
From the Dart API perspective it's not the only one that uses unpremultiplied, rawStraightRgba also returns unpremultiplied alpha. I'm not sure what flow of the code that goes through to achieve it, but I'm maintaining the old behavior here for old flows. I chose to make this unpremultiplied since that is the most expected result and the benefits of using premultiplied alpha don't exist here since we have to do a conversion anyway because we can't use f32 directly, we have to convert to f16.
There was a problem hiding this comment.
Consider adding a comment about that.
There was a problem hiding this comment.
I got a bit confused between toByteData and decodeImageFromPixels. The input to toByteData was documented as non-premultiplied, but the input to decodeImageFromPixels wasn't. I've updated all the documentation for ui.PixelFormat to be explicit about pixel format. I also added a comment in this code. The important thing is I wanted ImageByteFormat.rawExtendedRgba128 and PixelFormat.rgbaFloat32 to match so people can easily download, tweak, then reupload.
gaaclarke
added
the
autosubmit
fixes flutter/flutter#121730
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.