Baseflow / octo_image

A multifunctional Flutter image widget
https://baseflow.com/
MIT License
156 stars 23 forks source link

[web] support for new CanvasKit #5

Closed deakjahn closed 4 years ago

deakjahn commented 4 years ago

🐛 Bug Report

Using OctoImage with the new CanvasKit Skia support seems to result in

Another exception was thrown: Expected a value of type 'SkImage', but got one of type 'HtmlImage'

exceptions. I'm still investigating, just asking if you already tried it.

renefloor commented 4 years ago

I haven't tried. OctoImage mostly uses the standard image widget, so that should also go wrong with that one. What ImageProvider did you use?

deakjahn commented 4 years ago

CachedNetworkImageProvider. But you're right, it's a deeper issue: https://github.com/flutter/flutter/issues/57928

renefloor commented 4 years ago

I had a look at how networkimage sets is headers on web, but it doesn't. That's when I found the image was set here: https://github.com/flutter/engine/blob/a0983d36f7bf2b4f45a5762ce40bb81d8a3f8f9f/lib/web_ui/lib/src/engine/html_image_codec.dart#L39

I guess the bug is somewhere in this code.

deakjahn commented 4 years ago

I wouldn't put past it, I already reported at least three bugs against that, and not even with CanvasKit, just plain simple Flutter Web... Actually, that Blurhash problem was in here, too.

deakjahn commented 4 years ago

This very simple (no cache, plain download) provider makes it work, though:

class NetworkImageProvider extends ImageProvider<NetworkImageProvider> {
  final String url;
  final double scale;

  const NetworkImageProvider(this.url, {this.scale = 1.0})
      : assert(url != null),
        assert(scale != null);

  @override
  Future<NetworkImageProvider> obtainKey(ImageConfiguration configuration) => SynchronousFuture<NetworkImageProvider>(this);

  @override
  ImageStreamCompleter load(NetworkImageProvider key, DecoderCallback decode) => OneFrameImageStreamCompleter(_loadAsync(key));

  Future<ImageInfo> _loadAsync(NetworkImageProvider key) async {
    assert(key == this);
    final bytes = (await http.get(key.url)).bodyBytes;
    final codec = await skia.instantiateImageCodec(bytes);
    final frame = await codec.getNextFrame();
    return ImageInfo(image: frame.image, scale: key.scale);
  }

  @override
  bool operator ==(dynamic other) {
    if (other.runtimeType != runtimeType) return false;
    final NetworkImageProvider typedOther = other;
    return url == typedOther.url && scale == typedOther.scale;
  }

  @override
  int get hashCode => hashValues(url.hashCode, scale);

  @override
  String toString() => '$runtimeType(${describeIdentity(url)}, scale: $scale)';
}

OctoImage with this provider, Flutter Web, CanvasKit.

renefloor commented 4 years ago

But skia.instantiateImageCodec only works when you enable skia I assume? Or does this always work on the web?

deakjahn commented 4 years ago

CanvasKit, yes, switching to the new engine with the k key in the console. In this mode, everything that works in dart:ui on the mobile platform should be equally available on the web (well, should be, hopefully will be, it's far from being finished yet). Still, I'm experimenting with moving to it completely because there are quite a few very important things missing from the original mode. Things that won't be necessarily important for many of the Flutter developers only using stock widgets but I need rather sophisticated canvas and image manipulation. instantiateImageCodec is actually one of those.

So, as it matures, more and more people would probably move to it, because if it runs, the performance advantage is rather significant. The question is whether you should already start experimenting with it, too, or you can still wait. :-)

deakjahn commented 4 years ago

Anyway, if you want to look into it, this is how I currently use it. I made my own widget using OctoImage because I use it in many places, plus I need to handle reading the blurhash value as well. The important part is:

return OctoImage(
  image: kIsWeb ? NetworkImageProvider(imageUrl) : CachedNetworkImageProvider(imageUrl), //FIXME
  placeholderBuilder: isNotEmpty(blurhash) ? OctoPlaceholder.blurHash(blurhash) : _placeholder,
  errorBuilder: (context, error, stackTrace) {
    return (errorIcon != null) ? Icon(errorIcon) : Container(color: errorColor);
  },
  fit: fit,
);

OctoPlaceholderBuilder get _placeholder {
  return (context) => Container(color: Colors.grey[200]);
}

So, if I change to this provider above, it works on the web, both modes, CanvasKit and default. CachedNetworkImageProvider only works in default mode. What I really don't know is, CanvasKit being a moving target itself, whether you should try to patch things before they are ready...

renefloor commented 4 years ago

To me it looks like the only real problem is the cachemanager itself.

This works perfectly fine, with and without skia enabled:

      Uint8List imageBytes;
      if(kIsWeb) {
        imageBytes = (await http.get(url)).bodyBytes;
      }else {
        var file = await DefaultCacheManager()
            .getSingleFile(url);
        imageBytes = await file.readAsBytes();
      }
      decodedImage = await decode(imageBytes);
      yield decodedImage;

However, I close this issue because it doesn't really have anything to do with octo_image. My plan is to try to make the CacheManager web compatible that it at least returns a file, even without any caching, than the imageprovider doesn't have to differentiate between web and mobile anymore.

renefloor commented 4 years ago

@deakjahn in CachedNetworkImage 2.3.0-beta.1 I removed the web version of the ImageProvider and just have 1 version again. The CacheManager now also just downloads the image file. Could you verify that this indeed works with Skia? When I run this in the example it works perfectly fine: flutter run -d chrome --release --dart-define=FLUTTER_WEB_USE_SKIA=true

deakjahn commented 4 years ago

With some images it works, with others (same widget, just another page) CacheManager gives errors like this:

errors.dart:167 Uncaught (in promise) Error: XMLHttpRequest error.
    C:/b/s/w/ir/cache/builder/src/out/host_debug/dart-sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/errors.dart 268:20      get current
packages/http/src/browser_client.dart 84:22                                                                                    <fn>
C:/b/s/w/ir/cache/builder/src/out/host_debug/dart-sdk/lib/async/zone.dart 1439:54                                              runUnary
C:/b/s/w/ir/cache/builder/src/out/host_debug/dart-sdk/lib/async/future_impl.dart 141:18                                        handleValue
C:/b/s/w/ir/cache/builder/src/out/host_debug/dart-sdk/lib/async/future_impl.dart 686:44                                        handleValueCallback
C:/b/s/w/ir/cache/builder/src/out/host_debug/dart-sdk/lib/async/future_impl.dart 715:32                                        _propagateToListeners
C:/b/s/w/ir/cache/builder/src/out/host_debug/dart-sdk/lib/async/future_impl.dart 516:7                                         [_complete]
C:/b/s/w/ir/cache/builder/src/out/host_debug/dart-sdk/lib/async/stream_pipe.dart 65:11                                         _cancelAndValue
C:/b/s/w/ir/cache/builder/src/out/host_debug/dart-sdk/lib/async/stream.dart 1256:11                                            <fn>
C:/b/s/w/ir/cache/builder/src/out/host_debug/dart-sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/operations.dart 326:14  _checkAndCall
C:/b/s/w/ir/cache/builder/src/out/host_debug/dart-sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/operations.dart 331:39  dcall
C:/b/s/w/ir/cache/builder/src/out/host_debug/dart-sdk/lib/html/dart2js/html_dart2js.dart 37108:58                              <fn>

I also try in debug mode, not just release.

deakjahn commented 4 years ago

Wait, this might come from other changes in my app in the meantime. I'll report back.

renefloor commented 4 years ago

@deakjahn do these work with your own ImageProvider? I also had this error, but when you Google it you only get that it has to do with CORS, so I don't think I can fix that.

deakjahn commented 4 years ago

Bummer, the timeout is on our server. I notified our admin. Just at the exact same time. I'll only be able to go on testing if I get a word back about what is wrong up there.

No, we don't normally have CORS issues, the servers are set up to send the appropriate headers. This is a genuine timeout, even when I try the same image URL from the browser.

renefloor commented 4 years ago

Weird that the error isn't more clear about the timeout

deakjahn commented 4 years ago

I also get that now. CacheManager only reports:

CacheManager: Failed to download file from https://....jpg with error: XMLHttpRequest error.

but the browser itself says:

browser_client.dart:87 GET https://....jpg net::ERR_CONNECTION_TIMED_OUT

This is why I didn't see the error in the IDE output window. The browser line only appears in the browser console, it doesn't get back into the Flutter console somehow.

I just learned it's not our servers, it's the hosting (the one I use for testing now isn't in the cloud, actually). I'll go on when it comes back. :-)

deakjahn commented 4 years ago

OK, we're back in business.