-
Notifications
You must be signed in to change notification settings - Fork 28.7k
ImageInfo adds a new getter named sizeBytes to decouple ImageCache and ui.Image #86555
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat. If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
19f45ef
to
ca0ca43
Compare
👏🏿 This small change can bring a lot of scalability benefits, awesome |
Hi,@dnfield |
Can you please add a test to this PR? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs a test.
It's also not entirely clear to me what the use case is here. ImageInfo
must have an image
, which must have a width
and height
.
It would probably be better to expose sizeBytes
on dart:ui.Image
instead.
Hi, @dnfield BackgroundWe used external texture to display the images in our flutter app. On the Android platform, we draw the Bitmap onto the SurfaceTexture, and on the iOS platform, we convert the UIImage to CVPixelBufferRef, and finally use the Texture Widget to display the image. We do this mainly based on the following considerations:
DetailUse ui.Image _dummy;
FutureOr<TextureImageInfo> createTextureImageInfo(
{int textureId, int width, int height}) async {
if (_dummy != null) {
return TextureImageInfo(
textureId: textureId, width: width, height: height, image: _dummy);
}
_dummy = await _createImage(1, 1);
return TextureImageInfo(
textureId: textureId, width: width, height: height, image: _dummy);
}
Future<ui.Image> _createImage(int width, int height) async {
final Completer<ui.Image> completer = Completer<ui.Image>();
ui.decodeImageFromPixels(
Uint8List.fromList(
List<int>.filled(width * height * 4, 0, growable: false)),
width,
height,
ui.PixelFormat.rgba8888,
(ui.Image image) {
completer.complete(image);
},
);
return completer.future;
}
// ignore: must_be_immutable
class TextureImageInfo extends ImageInfo {
TextureImageInfo({
this.textureId,
this.width,
this.height,
ui.Image image,
double scale = 1.0,
String debugLabel,
}) : super(
image: image,
scale: scale,
debugLabel: debugLabel,
);
int textureId;
int width;
int height;
int get sizeBytes => height * width * 4;
......
}
Now, we can create
Then we customized an ImageProvider to provide TextureImageInfo,and copy the code of ImageCache and Image Widget and modify their dependence on ui.Image. It works well. ExpectationAt present, our solution is create a QuestionsBy the way,would you mind answering the following two questions:
Thank you |
f6b3159
to
7bd1a5e
Compare
Supporting texture based images is an interesting idea. There's also a bug open for supporting other graphics related things in the cache - #83801 Let me think about this one a little bit. |
Can you explain a bit more about how you're getting the size of the texture? |
Hi, @dnfield Usually we use the size of Bitmap/CGImage as the size of the texture, but sometimes we can also specify the size of the texture Android: Bitmap bitmap = getBitmapFromSomeWhere();
int textureWidth = bitmap.width();
int textureHeight = bitmap.height();
// we can also specify the size of the texture like this:
// int textureWidth = expectTextureWidthFromSomeWhere;
// int textureHeight = expectTextureHeightFromSomeWhere;
TextureRegistry.SurfaceTextureEntry entry = textureRegistry.createSurfaceTexture();
Surface surface = new Surface(textureEntry.surfaceTexture());
textureEntry.surfaceTexture().setDefaultBufferSize(textureWidth, textureHeight);
if (surface != null && surface.isValid()) {
try {
Canvas canvas = surface.lockCanvas(null);
Rect mSrcRect = new Rect(0, 0, bitmap.getWidth(), bitmap.getHeight());
Rect mDestRect = new Rect(0, 0, textureWidth, textureHeight);
canvas.drawBitmap(imageBitmap, mSrcRect, mDestRect, null);
surface.unlockCanvasAndPost(canvas);
} catch (Exception e) {
e.printStackTrace();
onLoadFailed(imageBitmap, needRecycle);
}
}
Map<String,Object> result = new HashMap();
result.put("textureId", textureEntry.id());
result.put("width", textureWidth);
result.put("height", textureHeight);
// then send result to flutter The logic of iOS is basically the same as Android,we convert - (CVPixelBufferRef)createPixelBufferFromImage:(CGImageRef)image andSize:(CGSize)size {
NSDictionary *options = @{
(__bridge NSString *)kCVPixelBufferIOSurfacePropertiesKey : @{},
(__bridge NSString *)kCVPixelBufferCGImageCompatibilityKey: @NO,
(__bridge NSString *)kCVPixelBufferCGBitmapContextCompatibilityKey: @NO
};
CVPixelBufferRef pxbuffer = NULL;
// imageSize is the size of the texture
CGSize imageSize = size;
if (CGSizeEqualToSize(size, CGSizeZero)) {
imageSize = CGSizeMake(CGImageGetWidth(image), CGImageGetHeight(image));
}
CVReturn status = CVPixelBufferCreate(kCFAllocatorDefault,
imageSize.width,
imageSize.height,
kCVPixelFormatType_32BGRA,
(__bridge CFDictionaryRef) options,
&pxbuffer);
if (status!=kCVReturnSuccess) {
NSLog(@"Operation failed");
}
NSParameterAssert(status == kCVReturnSuccess && pxbuffer != NULL);
CVPixelBufferLockBaseAddress(pxbuffer, 0);
void *pxdata = CVPixelBufferGetBaseAddress(pxbuffer);
CGColorSpaceRef rgbColorSpace = CGColorSpaceCreateDeviceRGB();
CGContextRef context = CGBitmapContextCreate(pxdata, imageSize.width,
imageSize.height, 8, CVPixelBufferGetBytesPerRow(pxbuffer), rgbColorSpace,
kCGBitmapByteOrder32Little | kCGImageAlphaPremultipliedFirst);
NSParameterAssert(context);
CGContextDrawImage(context, CGRectMake(0, 0, imageSize.width,
imageSize.height), image);
CGColorSpaceRelease(rgbColorSpace);
CGContextRelease(context);
CVPixelBufferUnlockBaseAddress(pxbuffer, 0);
return pxbuffer;
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I can't think of a great reason this API is harmful, and subclassing ui.Image
could do bad things.
Gold has detected about 4 new digest(s) on patchset 3. |
@ColdPaleLight Can you rebase this PR to make the checks pass? |
This pull request is not suitable for automatic merging in its current state.
|
c5da366
to
adfd9b8
Compare
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
@googlebot I consent. |
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
adfd9b8
to
1740e5a
Compare
@dnfield @goderbauer All checks have passed. Can you add the label "waiting for tree to go green" again? |
We used external texture to display the images in our flutter app. On the Android platform, we draw the Bitmap onto the SurfaceTexture, and on the iOS platform, we convert the UIImage to CVPixelBufferRef, and finally use the Texture Widget to display the image.
We want to let the external texture can benefit from the ImageCache logic.We hope that ImageCache can no longer dependent on ui.Image, so that we can customize ImageInfo and ImageProvier to cache external texture images in ImageCache.
List which issues are fixed by this PR. You must list at least one issue.
Fixes #86402
If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.
Pre-launch Checklist
///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.