gaplessPlayback broken: Bad state: Cannot clone a disposed image. about octo_image HOT 13 CLOSED

@renefloor Can confirm that issue completely fixed

from octo_image.

We just can't use the result from the imageBuilder (from the Flutter Image widget) as the Image Widget itself might be disposed. So I first thought of just keeping the old image widget or rebuilding that with the OctoImage imagebuilder, but that means the caller has to check whether it tries to build the new or old url. These are both not ideal.

I think we need to manage the loading of the imageprovider ourselves, practically forking the Flutter Image widget. I have to dive into this a bit better, but I've had a really busy week. I'll hope to find time for this soon, but it's no easy fix.

from octo_image.

Thanks.
Something is going wrong by the gaplessPlayback, setting that to false or removing the parameter results in the error disappearing. So it has something to do with the widget trying to show the old image again while loading the new one.

from octo_image.

@dnfield it looks like it broke because of this commit flutter/flutter@2cdec25

The exception is this:

======== Exception caught by widgets library =======================================================
The following StateError was thrown building FadeWidget(dependencies: [_EffectiveTickerMode], state: _FadeWidgetState#40d18(ticker active)):
Bad state: Cannot clone a disposed image.
The clone() method of a previously-disposed Image was called. Once an Image object has been disposed, it can no longer be used to create handles, as the underlying data may have been released.

The relevant error-causing widget was: 
  OctoImage file:///Users/renefloor/Documents/Repos/textfield/lib/main.dart:44:13
When the exception was thrown, this was the stack: 
#0      Image.clone (dart:ui/painting.dart:1758:7)
#1      RawImage.createRenderObject (package:flutter/src/widgets/basic.dart:5760:21)
#2      RenderObjectElement.mount (package:flutter/src/widgets/framework.dart:5439:28)
...     Normal element mounting (15 frames)
#17     Element.inflateWidget (package:flutter/src/widgets/framework.dart:3623:14)
...
====================================================================================================

The OctoImage widget calls the Flutter Image widget with an ImageProvider. When the image widget indicates it is done loading it allows the OctoImage user to modify that widget using the ImageBuilder and that result is shown.

Using gaplessPlayback that first widget is shown while a new image is being loaded. However, that widget reuses the image which is not possible anymore. If I just store the first ImageProvider instead of the first Widget everything works just fine, however I lose the information of possible transformations that were applied on the Image widget as I build a new widget. This is the potential fix: 7f815b8

However, that fix introduces a new problem.

@dnfield do you have any idea how I can reuse the widget here? Maybe I can store both the ImageProvider as the ImageBuilder function so the widget can be recreated as well, but I'm not sure if that is a good way as we might call a function on a widget that is already disposed.

from octo_image.

As I already feared, it is broken on Flutter 2.0 stable :(

from octo_image.

If you want to refer to an image, you have to make sure you've cloned it and then disposed of it when you're done. Otherwise, you might be using a disposed reference and find that it is no longer valid.

Using image providers directly is probably the right way to go, but if you need the reference to the image to stay alive you'll have to manage your own listening on that provider and make sure you clone/dispose the image appropriately.

from octo_image.

Thanks for the response @dnfield. I think I mis-worded maybe some sentences. I prefer to not keep track of the image itself, but the widget that contains the image. I put the old ImageWidget on a stack and fade it out, I put the new ImageWidget on that stack and fade in.
It is practically the widget returned by the FrameBuilder:

 Image(
        image: widget.image,
        frameBuilder: frameBuilder,
      ),

I use that in a stack like this:

  Widget _stack(Widget revealing, Widget disappearing) {
    return Stack(
      fit: StackFit.passthrough,
      alignment: Alignment.center,
      children: [
        FadeWidget(
          child: revealing,
          duration: widget.fadeInDuration,
          curve: widget.fadeInCurve,
        ),
        FadeWidget(
          child: disappearing,
          duration: widget.fadeOutDuration,
          curve: widget.fadeOutCurve,
          direction: AnimationDirection.reverse,
        )
      ],
    );
  }

The Image widget is first added as 'revealing', but when you load a different image the widget we had gotten from framebuilder is now used as 'disapearing'. I guess I need to use the main Image widget instead of the widget I get in 'frameBuilder'. Would that help?

Edit: nvm, that last suggestion breaks the whole flutter tree of course.. :(

Build functions must never return their BuildContext parameter's widget or a child that contains
"context.widget". Doing so introduces a loop in the widget tree that can cause the app to crash.

Edit2: using that Image widget directly not in the builder, but as the placeholder does work and not break, so maybe that is promising.

Edit3: Doing this indeed results in applying the new imageBuilder to the old image again.
This example shows the index on top of the image:

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      body: OctoImage(
        image:
        NetworkImage('https://via.placeholder.com/30$index.png/09f/fff'),
        imageBuilder: (url, image) {
          return Stack(
            alignment: Alignment.bottomCenter,
            children: [
              image,
              Text(index.toString()),
            ],
          );
        },
        placeholderBuilder: (_) => Container(color: Colors.red),
        gaplessPlayback: true,
      ),
    );
  }

This result in:

At least this is better than being broken, but still not optimal.

This is the proposed change: http://github.com/baseflow/octo_image/commit/389b6b46d16a4df207dbe5ec56d8c38912e6954d

from octo_image.

Pulling the image provider out of the widget isn't really safe.

from octo_image.

What do you mean with pulling it out of the widget? The ImageProvider is given to OctoImage by the user of the library. OctoImage then creates an image widget using that ImageProvider. So we already have the ImageProvider.

from octo_image.

Sorry, I think I was replying to an earlier comment. Also, sorry, but I'm dealing with a newborn and not sleeping much so I'm not sure I'm fully coherent here haha.

The basic change is that if you want to paint an image (or give it to something else that will paint it), you need to make sure that you're holding on to a clone of it. Otherwise, someone else can dispose the native data out from under you. Fishing an image provider out of an Image widget is probably not safe, since you don't know if that widget's state has been disposed and thus it disposed the underlying image.

If you're giving it the image provider, you shoudn't need to use widget.image in your code.

If OctoImage needs to keep the image provider's image around, it should be a listener on the provider and making sure that it creates and disposes of clones appropriately.

from octo_image.

@renefloor do you have any ideas on how to fix it?

from octo_image.

Add @fryette I think I fixed it in PR #18. Can you verify this PR? I'm going to write tests for it now.

from octo_image.

@renefloor Will do it and let you know.
Today/tommorow

Thanks!

from octo_image.