Skip to content
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

scrolling a grid causes items to jump/re-shuffle #16

Closed
alexda12 opened this issue Nov 1, 2020 · 11 comments
Closed

scrolling a grid causes items to jump/re-shuffle #16

alexda12 opened this issue Nov 1, 2020 · 11 comments
Assignees
Labels
wontfix This will not be worked on

Comments

@alexda12
Copy link

alexda12 commented Nov 1, 2020

Hi and thanks for this useful plugin.

I am using the latest version as of 1st Nov 2020.,

I notice that when I show a mixture of TEXT and images approx 60 cells (30 text cells and 30 image cells of different sizes) and scroll through - the cells appear to jump from 1 side to the next - I am testing this on a live device and get this issue on ANDROID and iOS. Its as if each scroll gesture forces the rebuild of the grid and then the cells get re-shuffled.

I have tested using a normal gridView and do not get this behaviour.

When the grid cells are the same size - the issue does not appear . When we have different sized IMAGES - we get the issue.

The code I use is :


WaterfallFlow.custom(
        padding: EdgeInsets.all(10),
        gridDelegate: SliverWaterfallFlowDelegateWithFixedCrossAxisCount(
            crossAxisCount: crossAxis, crossAxisSpacing: 5, mainAxisSpacing: 5),
        childrenDelegate: SliverChildBuilderDelegate((context, index) {
          var model = items[index].item2;
          var comp = items[index].item1;

          if (model.pictureUrl == null) {
           /// Display a TEXT widget
            return Container(height: 100, width: 100, child: comp);
          } else {
          /// Display an Image widget
            return comp;
          }
        }, childCount: items.length),
      );
@zmtzawqlp
Copy link
Member

please provide a runnable demo

@alexda12
Copy link
Author

alexda12 commented Nov 2, 2020

The issue is because of the size of the image. If you have (say) 50 images approx 1.5mb you will see the image jumping ... If you have exactly the same number of images but lower quality , say 4k - there is no issue.

I took your code where you buildWaterfallFlowItem ... amended my code to use an ExtendedImage that gives ImageSize (Since you use AspectRatio with width/height) and you have the issue.

Additionally - your web demo has the same issue if you change your image repository to use higher res images from any place other than tuchong ... i.e >=1.5mb and you will see the same issue in all of your web demos...

@zmtzawqlp
Copy link
Member

zmtzawqlp commented Nov 3, 2020

I think they are reasonable. as what i said in flutter/flutter#59582 (comment)
more, i don't think big image in list is a good way, thumbnails are better.

@alexda12
Copy link
Author

alexda12 commented Nov 3, 2020

Hi I have read your comment in flutter/flutter#59582 (comment).

The approach you are using appears to rebuild the sliver items when an item is visible within the ViewPort; this approach has the negative impact whereby it rebuilds the list when items come into view (hence the issue that I am seeing with items moving from one cell to the other) - Rebuilding the list isnt an issue if you have very small images... but when your images are >=1.5mb Waterfall_flow exhibits jumping of cell items.

Additionally, since your approach requires knowing the image size - I see you use ExtendedImage for this. ExtendedImage consequently rebuilds unnecessarily which for certain scenarios will be a performance issue. See the image below which shows the debug console and Flutter's 'Loading State' message when we make use of ExtendedImage.

As mentioned previously - there is no issue whatsoever with waterfall_flow if it renders very small images. As you have mentioned here :

i don't think big image in list is a good way, thumbnails are better.

I didnt realise that your plugin is not performant to manage images greater than say (400k) without exhibiting the issues that I have discussed so far - so thanks for pointing this out.

Note - It is possible to achieve a very basic Staggered View without the rebuild issue and without resorting to using ExtendedImage or WaterFall_flow and without having to know the Raw Image Size by using a GridView.custom and using a FittedBox that contains an image. The use of the FittedBox will scale the widget as you know to its parents boundaries. With a combination of combining a SliverGridDelegateWithFixedCrossAxisCount & a SliverChildBuilderDelegate a basic staggered GridView showing images can be achieved.

Basic Staggered View - images approx 3mb each
good

Same Staggered View using Waterfall_Flow & ExtendedImage - Images 3mb each
bad

Here you can easily see that Images G, F,S, P, L etc jump from right to left - which is a negative side-effect of using waterfall_flow

Flutter reports rebuilds with ExtendedImage
image

@zmtzawqlp
Copy link
Member

no matter what image you use, state always rebuild when scroll the list, you can try with official image。

GridView items are layout with horizontal axis( vertical is the same as cross), it won't be changed, so you don't see any layout changed. but waterfallflow vertical size is base on real widget size, it will change after image is loaded complete,and image in cache extent will affect whole layout.

@ssbaval
Copy link

ssbaval commented Nov 5, 2020

I'm getting same issue here and can confirm that with extendedImage I also see flutter rebuild state message.

I notice with Waterfall_flow that it rebuilds everytime I scroll and attempts to use extendedImage.network again even though I have the images downloaded already. Why is it rebuilding or attempting to call again my extendedImage.network call ? That doesnt seem right to me and makes my images jump all over the place.

@zmtzawqlp
Copy link
Member

It needs time to decode image even though the images are download.
run following demo and scroll up and down ,you can get what i said.

import 'package:flutter/material.dart';

void main() {
  runApp(MyApp());
}

class MyApp extends StatelessWidget {
  // This widget is the root of your application.
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      title: 'Flutter Demo',
      theme: ThemeData(
        // This is the theme of your application.
        //
        // Try running your application with "flutter run". You'll see the
        // application has a blue toolbar. Then, without quitting the app, try
        // changing the primarySwatch below to Colors.green and then invoke
        // "hot reload" (press "r" in the console where you ran "flutter run",
        // or simply save your changes to "hot reload" in a Flutter IDE).
        // Notice that the counter didn't reset back to zero; the application
        // is not restarted.
        primarySwatch: Colors.blue,
      ),
      home: MyHomePage(title: 'Flutter Demo Home Page'),
    );
  }
}

class MyHomePage extends StatefulWidget {
  MyHomePage({Key key, this.title}) : super(key: key);

  // This widget is the home page of your application. It is stateful, meaning
  // that it has a State object (defined below) that contains fields that affect
  // how it looks.

  // This class is the configuration for the state. It holds the values (in this
  // case the title) provided by the parent (in this case the App widget) and
  // used by the build method of the State. Fields in a Widget subclass are
  // always marked "final".

  final String title;

  @override
  _MyHomePageState createState() => _MyHomePageState();
}

class _MyHomePageState extends State<MyHomePage> {
  List<Item> items = List.generate(100, (index) => Item());

  @override
  Widget build(BuildContext context) {
    // This method is rerun every time setState is called, for instance as done
    // by the _incrementCounter method above.
    //
    // The Flutter framework has been optimized to make rerunning build methods
    // fast, so that you can just rebuild anything that needs updating rather
    // than having to individually change instances of widgets.
    return Scaffold(
      appBar: AppBar(
        // Here we take the value from the MyHomePage object that was created by
        // the App.build method, and use it to set our appbar title.
        title: Text(widget.title),
      ),
      body: ListView.builder(
        itemBuilder: (BuildContext context, int index) {
          return FutureBuilder<double>(
            builder: (c, d) {
              if (!d.hasData) {
                return Container();
              }

              return Container(
                height: d.data,
                alignment: Alignment.center,
                child: Text('$index'),
              );
            },
            future: items[index].getHeight(),
          );
        },
        itemCount: items.length,
      ),
    );
  }
}

class Item {
  double _height;
  double get height => _height;
  Future<double> getHeight() async {
    if (_height == null) {
      await Future.delayed(Duration(milliseconds: 200));
      _height = 200;
    }
    return _height;
  }
}

``

@zmtzawqlp
Copy link
Member

zmtzawqlp commented Nov 6, 2020

Suggestion:

  • compress big image or use cacheheitht/cachewidth
  • perfer to know image width/height before render.
  • store image width/height as i done in demo

@ssbaval
Copy link

ssbaval commented Nov 6, 2020

In my actual app I do store the image size and use it later on exactly how you do in the web demo. Additionally - I have taken your web demo and can generate the exact same issue that I see by adding a reference to loading images that are large in size rather than a few kilobytes.

The waterfall_flow plugin rebuilds like I have mentioned constantly on scrolling up and down and hence you see progress displaying all the time when we scroll and then stops displaying when images come into view. So even though images have already been downloaded, the plugin will just always attempt to download.

Additionally - when I dynamically remove items from a waterfall_flow and then begin to scroll up or down , i see items in a 2 dimensional grid jump from left to right and vice versa - Is there support for dynamic additions/deletions ?

Maybe I have misunderstood your logic for extended image , this is how I am using :

_buildExtendedImage() {
    Widget image = ExtendedImage.network(
      widget.question.imagePath,       
      shape: BoxShape.rectangle,
      border: Border.all(color: Colors.grey.withOpacity(0.4), width: 1.0),
      borderRadius: const BorderRadius.all(
        Radius.circular(10.0),
      ),
      loadStateChanged: (ExtendedImageState value) {
        if (value.extendedImageLoadState == LoadState.loading) {
          Widget loadingWidget = Container(
              alignment: Alignment.center,
              color: Colors.grey.withOpacity(0.2),
              child: CircularProgressIndicator(
                strokeWidth: 2.0,
                valueColor: AlwaysStoppedAnimation<Color>(
                    Theme.of(context).primaryColor),
              ));

          loadingWidget = AspectRatio(
            aspectRatio: 1.0,
            child: loadingWidget,
          );
          return loadingWidget;
        } else if (value.extendedImageLoadState == LoadState.completed) {
          widget.question.imageRawSize = Size(
              value.extendedImageInfo.image.width.toDouble(),
              value.extendedImageInfo.image.height.toDouble());
        } else if (value.extendedImageLoadState == LoadState.failed) {
          return Column(mainAxisAlignment: MainAxisAlignment.center, children: [
            SizedBox(height: 10),
            Icon(
              Icons.error,
              color: Colors.red,
            ),
            SizedBox(height: 10),
            FixedSizedTitle(
              title: 'Image failed to load',
              useTextOverflow: false,
              textAlign: TextAlign.center,
            ),
            SizedBox(height: 10),
          ]);
        }
        return null;
      },
    );

    if (widget.question.imageRawSize != null) {
      image = AspectRatio(
        aspectRatio: widget.question.imageRawSize.width /
            widget.question.imageRawSize.height,
        child: image,
      );
    }

    return image;

@zmtzawqlp
Copy link
Member

in web ExtendedImage do as official Image. No Cache.
https://github.com/fluttercandies/extended_image_library/blob/43e58bfa37878aec8a5937fa51fad499315e9ef6/lib/src/_network_image_web.dart#L87
and Web is not already for product , i can do nothing for it for now.

@zmtzawqlp
Copy link
Member

Additionally - when I dynamically remove items from a waterfall_flow and then begin to scroll up or down , i see items in a 2 dimensional grid jump from left to right and vice versa - Is there support for dynamic additions/deletions ?

Items are changed, so layout will recalculate, the position of item are base on the before Item. That why Flutter list can't scroll into large distance by code. Sliver calculate item one by one

@zmtzawqlp zmtzawqlp self-assigned this Nov 6, 2020
@zmtzawqlp zmtzawqlp added the wontfix This will not be worked on label Nov 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

3 participants