Improve tile handling

[JosefWN]

A suggestion to simplify the tile handling a bit:

• Move AnimatedTile into former TileWidget (no need for state, the tiles are managed by the TileManager), the AnimationBuilder does the heavy lifting for us
• Do not recreate AnimationController for every animation

Haven't gotten rid of the opacity widget yet, I think we'd need to crossfade between old and new tile on zoom

[JaffaKetchup]

@JosefWN
Hi there, just getting round to looking at all of these.
Rather than dealing with our own animation controller and ticker etc., can we just use a TweenAnimationBuilder?

[JosefWN]

One reason I kept the animation controller is that we allow for this:

animationController?.forward(from: from);

So you can choose to play the animation from, say, 50% to 100%. I don't know why this is necessary, but we have exposed the option to the users as well (TileLayer.tileFadeInStart). I could have another look at if we can further simplify if we were to remove the option?

[JaffaKetchup]

I see. I guess it depends on how much things can or cannot be simplified, and how often it's used. I'd say if it's not used in the example app, then we can probably get rid of it: although this will be a breaking change. @ibrierley @moonag @TesteurManiak What do you guys think?

[TesteurManiak]

IMO we can replace with a TweenAnimationBuilder the use of the AnimationController seems too specific.

[ibrierley]

I'm a bit hesitant to do breaking changes atm (we've had quite a lot of late), but no problems with these type of changes implemented in a future version where there's several breaking changes if that makes sense.

[JaffaKetchup]

Yeah, I would agree with that. I think leaving an at least 2-3 month gap between breaking releases is the minimum.

In this case, @JosefWN can you explore how much you can simplify things with this option - and any similar options you think fall under the same bracket - and maybe we merge this into a separate branch 'delayed-breaking-changes', which we can merge into main and release at a later date.
Of course, if you don't have time, let us know and we'll just merge this for now.

Thanks :)

[JosefWN]

I can have a look in the coming days!

[mootw]

I know the tile layer stuff is all kinda spaghetti together and hard to keep separate, but we could look at making this the migration to a new tile layer system. Give it a new name, RasterTileLayer, WMSTileLayer, VectorTileLayer, etc.... And then deprecate the old stuff. This would allow for non-breaking changes and allow us to basically rewrite the entire tiling system. I don't think this change has to be breaking for end users.

One thing I would like to see be worked on, I don't have the time right now, but support wrapping on the map left and right; and an extension of that would be to extend the system to support rendering on a sphere.

[JaffaKetchup]

I'm not sure, if you deprecate something in 1.2, and replace it in 1.3, is that OK with SemVer? Or would you need to move it 2.0? Are we even sticking with SemVer (we are atm, and it seems kind of sensible to remain doing so)?

But yes, I would like to see that rewrite and separation as well. This might even pave the way for #1337. But alas, I don't have the time (or much knowledge) to do so either at the moment.
I wonder how @greensopinion would feel about helping out, do you have time? You seem to be pretty knowledgeable about this stuff - and the naming/separation scheme could benefit your plugin, if you see what I mean.

Should we be making this aim more formal? Ie. use a Discussion, Discord channel, and/or Git branch.

---

support wrapping on the map left and right

See #1338, #1201

extend the system to support rendering on a sphere

I think someone had a crack at 3D rendering, but I think they ran out of time.

[greensopinion]

I wonder how @greensopinion would feel about helping out, do you have time?

Hey, thanks for the ping. I'm pretty busy with other things at the moment, unfortunately I don't have any spare cycles to chip in.

I'm a bit hesitant to do breaking changes atm

Improvements are great, but any breaking change is a pain for users and for plug-in maintainers. Unless there's substantial benefit, we should try to avoid it as much as possible. I know that might sound like a drag, but it's a big deal. For example, going to 3.0.0 was a lot of work. For the VectorTileLayer I had to understand the nature of the change and then get it working. It cost me about a full day, and although the code is a little cleaner on the consumer side, it's an incremental benefit. I'm not sure that it's worth it to break the API for such a minor improvement.

I'm not sure what's planned for the new tile layer system. There is quite a lot of logic in vector_map_tiles related to loading tiles of different sizes, depending on the data that's available and to optimize the frame rate. It would be great if we could avoid breaking that.

[JaffaKetchup]

Thanks anyway @greensopinion :)

Yeah, understood with the breaking changes. For me, I don't mind them so much, but I can see & understand they are a pain for a lot of people, especially when the changes are as big as last time.
As I mentioned above, this PR will probably go into a delayed breaking changes branch, until we have accumulated them over maybe 2-3 months. This change should break much anyway - I can't see why anyone would be using it.

Those greater tile system changes probably won't be coming any time soon, so no need to worry about them for now.

[JaffaKetchup]

@JosefWN I've converted to a draft just for now, so we can differentiate between needs-review/in-progress/ready. If you are ready to merge, let me know.

[JaffaKetchup]

Hi @JosefWN, is this ready for review? If not, no pressure!

[mootw]

I propose we just work on new tile layer stuff from the ground up, give it a new widget (ideally plural with different behaviors) with new logic. That is realistically the best way to improve the tile handling at this point. The current system is not extensible, has too many edge cases, and way too many options.

[JaffaKetchup]

@moonag I agree with this, however I'm not sure when this would get finished?

[JaffaKetchup]

Hey @moonag, is there any chance of that rewrite happening any time soon? If not, we should probably get round to merging this.

[mootw]

not right now unfortunately unless someone else wants to take it on

[JaffaKetchup]

Ok. @ibrierley When you're back, do you want to review this for merge? I'll do the same.

[ibrierley]

I'm a little out of touch with the tile handler recently, but I like the visible difference. My acid "feel" test is to open Animated MapController example in Web release, and repeatedly animate from London <-> Paris. It certainly feels better to me with this change (mainly less slight outer grey on animation).

Regarding the code itself, I'm fairly easy and can't initially spot any issues, but it's hard with the tile handler as it's typically been a mess to follow, but maybe that's just me:D. I'm happy with this.

Thanks for the work on this!

[JaffaKetchup]

@ibrierley I can't seem to find where this is a breaking change?

[ibrierley]

Good question, I'm not actually sure if this is or not now...there was mention of the fadeInStart I think...

[JaffaKetchup]

Ok, @ibrierley I haven't tested, but I can't see any issues or why this would be a breaking change. You can merge this if you want.