Image.toByteData and Picture.toImage implementations (#3)

[deakjahn]

Description

• Image.toByteData() was not implemented in either DomCanvas or CanvasKit. This PR covers both.
• Picture.toImage() was not implemented in either DomCanvas or CanvasKit. This PR covers CanvasKit (I also have a solution for DomCanvas but that would need some discussion, it might clash with functionality somewhere else; details in a comment below).

Related Issues

• [web] Support Image.toByteData on web in HTML renderer flutter#59072
• Implement Picture.toImage, Scene.toImage in CanvasKit flutter#42767
• Craft better error message for OffsetLayer.toImage in the HTML renderer flutter#49857

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

• I read the contributor guide and followed the process outlined there for submitting PRs.
• I signed the CLA.
• I read and followed the C++, Objective-C, Java style guides for the engine.
• I read the tree hygiene wiki page, which explains my responsibilities.
• I updated/added relevant documentation.
• All existing and new tests are passing.
• I am willing to follow-up on review comments in a timely manner.

Breaking Change

Did any tests fail when you ran them? Please read handling breaking changes.

• No, no existing tests failed, so this is not a breaking change.
• Yes, this is a breaking change. If not, delete the remainder of this section.

[dnfield]

I think this should probably throw instead of returning null. This indicates we don't support the format. WDYT?

[deakjahn]

I really don't know what you normally do in a situation like that. If throw, than throw it is.

[dnfield]

This ignores the format parameter. It's not entirely clear to me what format the data: URI is in, I'm trying to test this and I'm only seeing regular URIs or blob URIs.

[deakjahn]

Oops, this was some time ago already, we spent some with finalizing the PR, so the code itself is even older. Let me think. I seem to remember PNG was the only format available in the data URL, so it simply couldn't support anything else. The possibilities are:

• Only serve PNG, throw for the others,
• Always serve PNG, no matter what is asked for (current),
• Convert if something else than PNG is asked for (unlikely, I guess).

[monsieurtanuki]

For what it's worth, I've just run a simple Picture.toImage test on both iOS and web, and it crashed on web with an Unexpected null value. (in that case the Picture being instantiated as an EnginePicture)
I don't know what is supposed to be currently merged on what flutter channel; sorry if I've tested much too early.
I hope this will be fixed soon: thanks in advance :)

flutter doctor

[✓] Flutter (Channel beta, 1.23.0-18.1.pre, on Mac OS X 10.15.5 19F101 x86_64,
    locale fr-FR)
 
[✓] Android toolchain - develop for Android devices (Android SDK version 30.0.2)
[✓] Xcode - develop for iOS and macOS (Xcode 12.2)
[✓] Chrome - develop for the web
[✓] Android Studio (version 4.1)
 
[✓] Connected device (3 available)            

• No issues found!

simple Picture.toImage test

ui.PictureRecorder recorder = new ui.PictureRecorder();
new Canvas(recorder, Rect.fromLTRB(0, 0, 100, 200));
ui.Picture picture = recorder.endRecording();
try {
  var img = await picture.toImage(100, 200);
} catch (e) {
  print(e);
}

[deakjahn]

The very first question is whether CanvasKit or DomCanvas. As you can read in the OP, the Picture variant was only done for the first (I described in one of the related issues what could be the solution to the DomCanvas case but that was beyond the scope of a relatively simple PR).

[monsieurtanuki]

@deakjahn Köszönöm! I've just run my test on debug with CanvasKit on my Mac (flutter run -d chrome --dart-define=FLUTTER_WEB_USE_SKIA=true) and it was successful. I mean, the Picture.toImage part was successful. The rest of the interface was a bit bumpy, though.
I'm still curious to see how this will work on release mode, and on a mobile.

Regarding DomCanvas, you say that this PR fixes Image.toByteData(), but not Picture.toImage().
And in a comment you say

If at least one of the two worked (Picture.toImage() or Image.toByteData()), the first could provide a workaround for the second. Not pretty but a workaround, nonetheless.

I cannot picture (ahah) how I could rely only on Image.toByteData() and not on Picture.toImage(). In my case I create a ui.PictureRecorder to paint on it, and then get a Picture with recorder.endRecording().

Questions:

1. what was your idea on a workaround to emulate Picture.toImage() from Image.toByteData(), even in an ugly way?
2. do you know if there is a pending issue or PR about Picture.toImage() on DomCanvas?

[deakjahn]

That comment was written before I started to work on the PR. I've been complaining about the lack of support for some time then, finally, I started to see how I could fix it myself instead of waiting for the Flutter team to do it. :-))

Basically, I wasn't particulary interested in DomCanvas because, for my work, I switched to CanvasKit completely (both for performance and because I needed many features that were only available in the new engine, anyway). But when I set out to create the PR, I was obviously trying to cover both engines, if possible.

Now, I don't think there is a PR, or even any work done on that case for that matter. There is a post "DomCanvas case" early in this thread, check that out. The change would be straightforward but while it would fix this case, it would also probably break some others. To avoid the regression, some refactoring would be needed.

[deakjahn]

Wait a minute, Picture null and turning the canvas into an image, that's it, right? Then I must have thought of this, I also answered about this on SO and was at same point, while still in DomCanvas, using this solution: https://stackoverflow.com/questions/56052004/capturing-a-canvas-as-an-image-in-flutter-web/61960761#61960761

[monsieurtanuki]

@deakjahn Thank you for your help and all your remarks.

Regarding HTML5's canvas.context2D it's not that simple: I have tons of drawing operations. And I would need to maintain both canvas and context2D - or rather create some kind of meta canvas - hoping that all the drawing methods I need are available in both, and that the resulting picture looks more or less the same. Looks like an interesting new pub lib to develop ;)

[deakjahn]

And are you sure you wouldn't be better off using CanvasKit?

[monsieurtanuki]

@deakjahn I'm not sure of anything; I just want to know all the possibilities.

Using CanvasKit is a possibility, but it is branded as "highly experimental" and I've already experienced glitches in the UI.
I don't feel like devoting time into fixing CanvasKit. After all the good job you did on DomCanvas, your involvement and your understanding of the case, I guess it's not that straightforward to fix it for CanvasKit too (with the refactoring you suggested).

I know it will take me about a day to run my tests about my "meta canvas" solution, and that's a visibility I'm ok with.

[deakjahn]

That's the other way around. I primarily did it for CanvasKit, not DomCanvas. I wouldn't call it experimental any more (yes, Google has to be careful in the documentation) but more like work in progress. In my actual work where I needed to have the same Skia-based graphics support that I already use on Android and iOS, it was indispensable for me, that's why I moved to CanvasKit definitely. I don't find any serious issues with CanvasKit nowadays, so, even if it's not yet officially done, I'm quite comfortable with using it.

[monsieurtanuki]

@deakjahn Correct: my problem is with DomCanvas.
I mean, "was with DomCanvas", as I managed to code an elegant meta layer for the canvas and the SVG path operations, that now work on both the standard flutter Canvas and html.CanvasRenderingContext2D.
As you provided some javascript code in order to download an html canvas into a picture file, I'm finished! Thanks again.

[lukemadera]

@deakjahn Thanks for all your work on this! Is there a fix for picture.toImage for DomCanvas or is it still broken? I'm having this issue when I use the html renderer (which we use for deploys) but not when I use the canvaskit renderer (local development).

[deakjahn]

I really don't know, I left DomCanvas quite some time ago and never looked back. The last time I checked the problem was as described in #20750 (comment).

Come to think of it, this is the same problem I mentioned as a "Known problem" with my https://github.com/deakjahn/crop_image package... I'll link to this issue then.

[lukemadera]

Ok, thanks for the prompt reply; seems it still doesn't work then :(

[tolotrasamuel]

Steps to reproduce:

1. On Flutter Web, Wrap Flutter Google Maps in a RepaintBoundary

Like so:

  @override
  Widget build(BuildContext context) {
    return RepaintBoundary(
      key: ScreenShotable.previewContainer,
      child: widget.child,
    );
  }

Usage:

final img = await screenshotController.captureAsUiImage(
        Container(
          height: 100,
          width: 100,
          color: Colors.brown,
          child: GoogleMap(
            initialCameraPosition: CameraPosition(
              target: FlitLatLng(-20.1020344, 57.5621812999999).toLatLng(),
              zoom: 13.0,
            ),
            rotateGesturesEnabled: false,
            tiltGesturesEnabled: false,
            // markers: Set<Marker>.of(controller.markers.values),
          ),
        ),
        delay: Duration(seconds: 6));

ScreenshotControllers.captureAsUiImage

RenderRepaintBoundary boundary =
    this.view!.context.findRenderObject() as RenderRepaintBoundary;
ui.Image image = await boundary.toImage(pixelRatio: pxRatio); // crash here

Actual:

Error: Unexpected null value.
    at Object.throw_ [as throw] (http://localhost:49196/dart_sdk.js:5037:11)
    at Object.nullCheck (http://localhost:49196/dart_sdk.js:5362:30)
    at _engine.PlatformViewLayer.new.preroll (http://localhost:49196/dart_sdk.js:135222:12)
    at _engine.OffsetEngineLayer.new.prerollChildren (http://localhost:49196/dart_sdk.js:134614:15)
    at _engine.OffsetEngineLayer.new.preroll (http://localhost:49196/dart_sdk.js:134881:35)
    at _engine.OffsetEngineLayer.new.prerollChildren (http://localhost:49196/dart_sdk.js:134614:15)
    at _engine.OffsetEngineLayer.new.preroll (http://localhost:49196/dart_sdk.js:134881:35)
    at _engine.TransformEngineLayer.new.prerollChildren (http://localhost:49196/dart_sdk.js:134614:15)
    at _engine.TransformEngineLayer.new.preroll (http://localhost:49196/dart_sdk.js:134881:35)
    at _engine.RootLayer.new.prerollChildren (http://localhost:49196/dart_sdk.js:134614:15)
    at _engine.RootLayer.new.preroll (http://localhost:49196/dart_sdk.js:134609:31)
    at _engine.LayerTree.new.flatten (http://localhost:49196/dart_sdk.js:135508:22)
    at _engine.LayerScene.new.toImage (http://localhost:49196/dart_sdk.js:135263:36)
    at layer$.OffsetLayer.new.toImage (http://localhost:49196/packages/flutter/src/rendering/layer.dart.js:1327:30)
    at toImage.next (<anonymous>)
    at runBody (http://localhost:49196/dart_sdk.js:37393:34)
    at Object._async [as async] (http://localhost:49196/dart_sdk.js:37424:7)
    at layer$.OffsetLayer.new.toImage (http://localhost:49196/packages/flutter/src/rendering/layer.dart.js:1318:20)
    at proxy_box.RenderRepaintBoundary.new.toImage (http://localhost:49196/packages/flutter/src/rendering/proxy_box.dart.js:3096:26)
    at captureFromWidget (http://localhost:49196/packages/screenshot/screenshot.dart.js:242:44)
    at captureFromWidget.next (<anonymous>)
    at http://localhost:49196/dart_sdk.js:37374:33
    at _RootZone.runUnary (http://localhost:49196/dart_sdk.js:37245:59)
    at _FutureListener.thenAwait.handleValue (http://localhost:49196/dart_sdk.js:32501:29)
    at handleValueCallback (http://localhost:49196/dart_sdk.js:33028:49)
    at Function._propagateToListeners (http://localhost:49196/dart_sdk.js:33066:17)
    at _Future.new.[_complete] (http://localhost:49196/dart_sdk.js:32906:25)
    at http://localhost:49196/dart_sdk.js:32085:30
    at internalCallback (http://localhost:49196/dart_sdk.js:24224:11)

Expected:

No crash. Even better if we get the screenshot of the map.
On Mobile, we get blank screen.

Further debugging:

I tried with my own code above and even with the https://pub.dev/packages/screenshot package. Same result

If you have any clue on what the error above means, I would appreciate, even a small comment. It will help me find a workaround