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

fix: make horizontal repetition CRS dependent #1978

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

monsieurtanuki
Copy link
Contributor

What

Files

New file:

  • epsg3006_crs.dart: new example around EPSG3006, needed for tests

Impacted files:

  • camera.dart: special case around replicatesWorldLongitude
  • crs.dart: new bool getter replicatesWorldLongitude
  • main.dart: added EPSG3006 example
  • map_interactive_viewer.dart: special case around replicatesWorldLongitude
  • menu_drawer.dart: added EPSG3006 example
  • tile_coordinates.dart: moved code to new class TileCoordinatesSimplifier
  • tile_image_manager.dart: now using a TileCoordinatesSimplifier for more flexibility
  • tile_image_view.dart: now using a TileCoordinatesSimplifier for more flexibility
  • tile_layer.dart: special case around replicatesWorldLongitude
  • tile_range.dart: special case around replicatesWorldLongitude

New file:
* `epsg3006_crs.dart`: new example around EPSG3006, needed for tests

Impacted files:
* `camera.dart`: special case around `replicatesWorldLongitude`
* `crs.dart`: new `bool` getter `replicatesWorldLongitude`
* `main.dart`: added EPSG3006 example
* `map_interactive_viewer.dart`: special case around `replicatesWorldLongitude`
* `menu_drawer.dart`: added EPSG3006 example
* `tile_coordinates.dart`: moved code to new class `TileCoordinatesSimplifier`
* `tile_image_manager.dart`: now using a `TileCoordinatesSimplifier` for more flexibility
* `tile_image_view.dart`: now using a `TileCoordinatesSimplifier` for more flexibility
* `tile_layer.dart`: special case around `replicatesWorldLongitude`
* `tile_range.dart`: special case around `replicatesWorldLongitude`
@arneke
Copy link

arneke commented Oct 17, 2024

This branch works well in my application.

Still think the 1 << z is a bit problematic. Does https://demo.fleaflet.dev/crs_epsg4326 work for you locally ? (that is a grid with two tiles at z = 0)

@monsieurtanuki
Copy link
Contributor Author

This branch works well in my application.

Cool!

Still think the 1 << z is a bit problematic. Does https://demo.fleaflet.dev/crs_epsg4326 work for you locally ? (that is a grid with two tiles at z = 0)

@arneke You're right: I've just fixed that. Thank you for your meticulous review!

@JaffaKetchup JaffaKetchup requested review from mootw, JaffaKetchup and josxha and removed request for mootw and JaffaKetchup October 21, 2024 15:08
Copy link
Member

@JaffaKetchup JaffaKetchup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the long wait, had a busy week!

lib/src/layer/tile_layer/tile_coordinates.dart Outdated Show resolved Hide resolved
@JaffaKetchup JaffaKetchup changed the title fix: now works for crs with world replication or not fix: make horizontal repetition CRS dependent Oct 21, 2024
@JaffaKetchup JaffaKetchup linked an issue Oct 21, 2024 that may be closed by this pull request
Copy link
Member

@JaffaKetchup JaffaKetchup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Sorry for the wait, I've had a hugely busy few weeks!)

I think I'm OK with this, but would appreciate a review from someone else!

@monsieurtanuki
Copy link
Contributor Author

Thank you @JaffaKetchup for your review!
I'm open to new reviews and new suggestions.

Copy link
Contributor

@josxha josxha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have the feeling that the fling animation has stutters when using a map with longitude repetition. Most notifable when crossing the -180/180 longitude. Could anyone check if it happens to them as well?

Comment on lines 68 to 69
urlTemplate:
'https://maps.trafikinfo.trafikverket.se/MapService/wmts.axd/BakgrundskartaNorden.gpkg?layer=Background&style=default&tilematrixset=Default.256.3006&Service=WMTS&Request=GetTile&Version=1.0.0&Format=image%2Fpng&TileMatrix={z}&TileCol={x}&TileRow={y}')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are the usage policy for this tile source? Source attribution would be a good think, too.

The tile source has long loading times for me. When I run the app on windows the tiles don't load but I got an exception. Not sure if this only happens on my machine though.

======== Exception caught by image resource service ================================================
The following HandshakeException was thrown resolving an image codec:
Handshake error in client (OS Error: 
	CERTIFICATE_VERIFY_FAILED: unable to get local issuer certificate(../../../flutter/third_party/boringssl/src/ssl/handshake.cc:393))

When the exception was thrown, this was the stack: 
#0      _SecureFilterImpl._handshake (dart:io-patch/secure_socket_patch.dart:99:46)
#1      _SecureFilterImpl.handshake (dart:io-patch/secure_socket_patch.dart:143:25)
#2      _RawSecureSocket._secureHandshake (dart:io/secure_socket.dart:920:54)
#3      _RawSecureSocket._tryFilter (dart:io/secure_socket.dart:1049:19)
<asynchronous suspension>
#4      _RawSecureSocket._secureHandshake (dart:io/secure_socket.dart:928:9)
<asynchronous suspension>
#5      _RawSecureSocket._tryFilter (dart:io/secure_socket.dart:1049:13)
<asynchronous suspension>
URL: https://maps.trafikinfo.trafikverket.se/MapService/wmts.axd/BakgrundskartaNorden.gpkg?layer=Background&style=default&tilematrixset=Default.256.3006&Service=WMTS&Request=GetTile&Version=1.0.0&Format=image%2Fpng&TileMatrix=13&TileCol=466563&TileRow=111610
Fallback URL: null
Current provider: MapNetworkImageProvider()
====================================================================================================
flutter: HandshakeException: Handshake error in client (OS Error: 
flutter: 	CERTIFICATE_VERIFY_FAILED: unable to get local issuer certificate(../../../flutter/third_party/boringssl/src/ssl/handshake.cc:393))
flutter: HandshakeException: Handshake error in client (OS Error: 
flutter: 	CERTIFICATE_VERIFY_FAILED: unable to get local issuer certificate(../../../flutter/third_party/boringssl/src/ssl/handshake.cc:393))
flutter: HandshakeException: Handshake error in client (OS Error: 
flutter: 	CERTIFICATE_VERIFY_FAILED: unable to get local issuer certificate(../../../flutter/third_party/boringssl/src/ssl/handshake.cc:393))
flutter: HandshakeException: Handshake error in client (OS Error: 
flutter: 	CERTIFICATE_VERIFY_FAILED: unable to get local issuer certificate(../../../flutter/third_party/boringssl/src/ssl/handshake.cc:393))
flutter: HandshakeException: Handshake error in client (OS Error: 
flutter: 	CERTIFICATE_VERIFY_FAILED: unable to get local issuer certificate(../../../flutter/third_party/boringssl/src/ssl/handshake.cc:393))
flutter: HandshakeException: Handshake error in client (OS Error: 
flutter: 	CERTIFICATE_VERIFY_FAILED: unable to get local issuer certificate(../../../flutter/third_party/boringssl/src/ssl/handshake.cc:393))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are the usage policy for this tile source? Source attribution would be a good think, too.

cc @arneke

Regarding CERTIFICATE errors, I've never experienced one, working on an Android emulator working on Windows.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's nothing published (I mention this at the top of the example in #1976). I included it to make it easy to reproduce the bug, was not really expecting this to end up on the Flutter Map website.

I can see if I can make a public service with dummy data available at some point, but not immediately and no guarantees about uptime or longevity.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arneke My initial point was to be able to test the fixes, for instance with the "3006" projection.
Any "non web-mercator" would have done the trick.
I've just removed "your 3006" from the PR.

lib/src/layer/tile_layer/tile_coordinates.dart Outdated Show resolved Hide resolved
Impacted files:
* tile_coordinates.dart: renamed as TileCoordinatesResolver and made const
* tile_image_manager.dart: removed the immutable metatag; used a setter and refactored
* tile_image_view.dart: refactored with a non null resolver
* tile_layer.dart: using a setter instead
Impacted files:
* tile_coordinates.dart: renamed as TileCoordinatesResolver and made const
* tile_image_manager.dart: removed the immutable metatag; used a setter and refactored
* tile_image_view.dart: refactored with a non null resolver
* tile_layer.dart: using a setter instead
@monsieurtanuki
Copy link
Contributor Author

@JaffaKetchup @josxha Actually TileCoordinatesSimplifier isn't only used in TileImageManager, but also in TileImageView.
That said, removing @immutable enables me to code as I would have in a first approach, cf. latest push.

@monsieurtanuki
Copy link
Contributor Author

I have the feeling that the fling animation has stutters when using a map with longitude repetition. Most notifable when crossing the -180/180 longitude. Could anyone check if it happens to them as well?

I suspect that my latest push made things slightly faster, with const classes and fields. Not a big user of flings, but haven't noticed anything problematic.

@josxha
Copy link
Contributor

josxha commented Nov 16, 2024

Do you know why the map is offset between zoom level 1 and 2?
I've seen some thin black lines shimmering through from the custom tile builder. Do you know if it is related to this pr?

recoding.mp4

Copy link
Contributor

@josxha josxha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing the example page, solves my open points. 😄
Looks good! Thanks for the contribution.

@monsieurtanuki
Copy link
Contributor Author

@JaffaKetchup @josxha Thank you for your reviews and comments!

Still, there seems to be an issue with the "Build Android Example App" CI/CD. A bit cryptic to me: maybe the gradle version, maybe the android ndk version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Review in progress
Development

Successfully merging this pull request may close these issues.

[BUG] unbounded horizontal scroll breaks most tile grids
4 participants