-
-
Notifications
You must be signed in to change notification settings - Fork 868
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
refactor!: migrate tile rendering from widgets to CustomPainter
[replaced by #1908]
#1853
Changes from all commits
63d9052
7ff8726
385f1d2
de0823d
43487ee
c48f92e
c393607
8b542ee
4adef70
8e06d91
78ea7f5
bec92d7
a21e98f
902d2c5
1c47375
fad9cb9
2463dac
b8a889b
b61fb5d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
import 'dart:math'; | ||
import 'package:flutter_map/flutter_map.dart'; | ||
import 'package:flutter_map/src/layer/tile_layer/tile_painter.dart'; | ||
|
||
/// Model for tiles displayed by [TileLayer] and [TilePainter] | ||
class TileModel { | ||
/// [TileImage] is the model class that contains meta data for the Tile image. | ||
final TileImage tileImage; | ||
|
||
/// The [TileBuilder] is a reference to the [TileLayer]'s | ||
/// [TileLayer.tileBuilder]. | ||
final TileBuilder? tileBuilder; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes |
||
|
||
/// The tile size for the given scale of the map. | ||
final double scaledTileSize; | ||
|
||
/// Reference to the offset of the top-left corner of the bounding rectangle | ||
/// of the [MapCamera]. The origin will not equal the offset of the top-left | ||
/// visible pixel when the map is rotated. | ||
final Point<double> currentPixelOrigin; | ||
|
||
/// Creates a new instance of TileModel. | ||
const TileModel({ | ||
required this.scaledTileSize, | ||
required this.currentPixelOrigin, | ||
required this.tileImage, | ||
required this.tileBuilder, | ||
}); | ||
|
||
///Equality operator for TileModel | ||
@override | ||
bool operator ==(Object other) { | ||
if (identical(this, other)) return true; | ||
if (other is! TileModel) return false; | ||
final TileModel otherTile = other; | ||
return tileImage == otherTile.tileImage && | ||
tileBuilder == otherTile.tileBuilder && | ||
scaledTileSize == otherTile.scaledTileSize && | ||
currentPixelOrigin == otherTile.currentPixelOrigin; | ||
} | ||
|
||
///HashCode for TileModel | ||
@override | ||
int get hashCode => | ||
tileImage.hashCode ^ | ||
tileBuilder.hashCode ^ | ||
scaledTileSize.hashCode ^ | ||
currentPixelOrigin.hashCode; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
import 'package:flutter/material.dart'; | ||
import 'package:flutter/rendering.dart'; | ||
import 'package:flutter_map/src/layer/tile_layer/tile_model.dart'; | ||
|
||
/// Draws [TileModel]s onto a canvas at the correct position | ||
class TilePainter extends CustomPainter { | ||
/// List of [TileModel]s to draw to the canvas | ||
List<TileModel> tiles; | ||
|
||
/// Paint to use when drawing each tile | ||
/// | ||
/// Defaults to [defaultTilePaint]. | ||
final Paint? tilePaint; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure what would be missing comparing with TileBuilder. User could get image from tile and modify it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Modifying an incoming tile's bytes can be done with a I understand that positioning widgets over this kind of defeats the point of a canvas, and is one of the reasons we previously didn't use canvas, to allow this kind of easy tweaking. However, we should really provide the ability to overlay widgets at tile positions. Note that it's not possible to paint a widget to a canvas. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see. I looked into it, then there is now way to avoid Stack. I suppose could then preserve Stack and Tile approach and use that if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not necessarily. I suppose a good compromise would be to allow a point in the painting process that can be influenced by the user, ie. they can paint to the same canvas, but are constrained to a smaller I guess it's also worth verifying those performance improvements on a few different devices, just to see if we're making a correct decision here, and the pros outweigh the cons, IYSWIM. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. tile_builder has been nice to debug tile loading. I wouldn't prefer to have widgets mixed in with the canvas. Most if not everything that has been done with widgets before should be possible directly on the canvas too. |
||
|
||
/// Default [tilePaint]er | ||
static Paint get defaultTilePaint => Paint() | ||
JaffaKetchup marked this conversation as resolved.
Show resolved
Hide resolved
|
||
..isAntiAlias = false | ||
..filterQuality = FilterQuality.high; | ||
|
||
/// Create a painter with the specified [TileModel]s and paint | ||
/// | ||
/// [tilePaint] indirectly defaults to [defaultTilePaint]. | ||
TilePainter({ | ||
required this.tiles, | ||
this.tilePaint, | ||
}); | ||
|
||
@override | ||
void paint(Canvas canvas, Size size) { | ||
for (final tile in tiles) { | ||
// Draw tiles if they have an image | ||
if (tile.tileImage.imageInfo != null) { | ||
// Simplify tile positions and sizes | ||
final left = tile.tileImage.coordinates.x * tile.scaledTileSize - | ||
tile.currentPixelOrigin.x; | ||
final top = tile.tileImage.coordinates.y * tile.scaledTileSize - | ||
tile.currentPixelOrigin.y; | ||
final width = tile.scaledTileSize; | ||
final height = tile.scaledTileSize; | ||
final image = tile.tileImage.imageInfo!.image; | ||
|
||
canvas.drawImageRect( | ||
image, | ||
// Source rectangle | ||
Rect.fromLTWH( | ||
0, | ||
0, | ||
image.width.toDouble(), | ||
image.height.toDouble(), | ||
), | ||
Rect.fromLTWH(left, top, width, height), // Destination rectangle | ||
(tilePaint ?? defaultTilePaint) | ||
..color = (tilePaint ?? defaultTilePaint) | ||
.color | ||
.withOpacity(tile.tileImage.opacity), | ||
); | ||
} | ||
} | ||
} | ||
|
||
@override | ||
bool shouldRepaint(covariant TilePainter oldDelegate) { | ||
return oldDelegate.tiles != tiles; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should deprecate this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I deprecated it initially, but tests didn't pass anymore, as class
_TileState extends State<Tile>
was using deprecatedTile
class. So I am not sure how to manage this? Just delete class altogether?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deprecation is transitive in some way. Mark both classes as deprecated, and it should work. However, just don't worry about this for now, and I'll do it at the end to make sure it's correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tile
is not exported publically so it should just get removed with this pr. It got completely replaced it withTileModel