flutter_map icon indicating copy to clipboard operation
flutter_map copied to clipboard

[BUG] Using bounds creates map flicker

Open stx opened this issue 3 years ago • 4 comments

What is the bug?

The bounds fix at https://github.com/fleaflet/flutter_map/commit/678a8abcc9afce68b87f50fba8a16f61afa5e7e5 for https://github.com/fleaflet/flutter_map/issues/1362 allows the bounds to function correctly now, but now there is a frame flicker that looks quite bad on the web (since you're clicking between pages that load instantly) or when using map marker clustering via https://pub.dev/packages/flutter_map_marker_cluster.

Regarding clustering, because the map loads in one state, and then quickly switches to another state (with another location, zoom level, etc.), it causes all the markers to animate and reposition.

The issue is because the bounds are being fit after the map has already loaded:

    WidgetsBinding.instance.addPostFrameCallback((_) {
      // Finally, fit the map to restrictions
      if (options.bounds != null) {
        fitBounds(options.bounds!, options.boundsOptions);
      }
      options.onMapReady?.call();
    });

What is the expected behaviour?

The map bounds are set when the map loads and the map doesn't change or move after the map has loaded.

How can we reproduce this issue?

MapOptions(
  bounds: LatLngBounds([corner1, corner2]),
  boundsOptions: const FitBoundsOptions(
    padding: EdgeInsets.all(50),
    maxZoom: 20,
  ),
),

Do you have a potential solution?

If I recall, this worked as expected in 2.x.

Can you provide any other information?

No response

Platforms Affected

Android, iOS, Web, Windows, MacOS, Linux

Severity

Obtrusive: Prevents normal functioning but causes no errors in the console

Frequency

Consistently: Always occurs at the same time and location

Requirements

  • [X] I agree to follow this project's Code of Conduct
  • [X] My Flutter/Dart installation is unaltered, and flutter doctor finds no relevant issues
  • [X] I am using the latest stable version of this package
  • [X] I have checked the FAQs section on the documentation website
  • [X] I have checked for similar issues which may be duplicates

stx avatar Sep 24 '22 18:09 stx

Darn I suspect this one is going to be a bit of a pain, as it may be a timing issue. The problem is, it doesn't work if its not in the callback, and I don't experience the issue.

Could you paste a minimal example you are using (without a plugin). I don't experience the problem on web, so will find it hard to know if it's definitely fixed.

ibrierley avatar Sep 24 '22 22:09 ibrierley

I suspect this may end up being a problem we've seen before, where an initial draw doesn't know it's size on first frame (for bound calculations), and it can depend upon timing if anyone wants a look (not sure I'll have a lot of time to dig properly just atm). I may be wrong though, as some of that code has probably changed.

ibrierley avatar Sep 24 '22 22:09 ibrierley

I suspect this may end up being a problem we've seen before, where an initial draw doesn't know it's size on first frame (for bound calculations), and it can depend upon timing if anyone wants a look (not sure I'll have a lot of time to dig properly just atm). I may be wrong though, as some of that code has probably changed.

Looking at the old method of doing this, it was doing it on the build method in LayoutBuilder after the constraints weren't 0 and it was working correctly without this interactivity locking bug.

It looks like the widget with the LayoutBuilder is being rebuilt constantly now which is what caused the locking bug because the camera kept repositioning.

One strategy could be to try it in the LayoutBuilder again but set _hasInitiallyFitBounds = true:

_hasFitInitialBounds = false;

// ...

return LayoutBuilder(builder: (BuildContext context, BoxConstraints constraints) {
  // ... existing stuff like setSize(constraints.maxWidth, constraints.maxHeight);

  if (options.bounds != null && !_hasFitInitialBounds && constraints.maxWidth != 0.0) {
    fitBounds(options.bounds!, options.boundsOptions);

    _hasFitInitialBounds = true;
  }
});

stx avatar Sep 24 '22 23:09 stx

I don't think that would quite work, as it would be calling a move (from fitBounds) within a build.

However, what if we git rid of of the initState move/fitBounds stuff, and also _pixelbounds/_bounds/_pixelOrigin outside of the layoutBuilder, moved them inside and also did a bounds check, so something like...

initState method

    _bounds = _calculateBounds();

    WidgetsBinding.instance.addPostFrameCallback((_) async {
      options.onMapReady?.call();
    });
...

build method

/// _pixelBounds = getPixelBounds(zoom);
   /// _bounds = _calculateBounds();
   /// _pixelOrigin = getNewPixelOrigin(_center);

    return LayoutBuilder(
        builder: (BuildContext context, BoxConstraints constraints) {

      //Update on layout change
      setSize(constraints.maxWidth, constraints.maxHeight);

      if (options.bounds != null && !_hasFitInitialBounds) {
        final target = getBoundsCenterZoom(options.bounds!, options.boundsOptions);
        _zoom = target.zoom;
        _center = target.center;
        _hasFitInitialBounds = true;
      }
      
      _pixelBounds = getPixelBounds(zoom);
      _bounds = _calculateBounds();
      _pixelOrigin = getNewPixelOrigin(_center);

      return MapStateInheritedWidget(
        mapState: this,
...

I don't experience the problem, so it's hard for me to test if there are some other fundamental issues with this. So it would be handy if someone who does have the problem could try this.

PR at https://github.com/fleaflet/flutter_map/pull/1376

Please test this one thoroughly, especially for side effects etc.

ibrierley avatar Sep 25 '22 09:09 ibrierley