Setting the initial map scroll position about mapcompose HOT 27 CLOSED

For the problem of too many arguments on the MapState constructor, it could have a fluent-style options object. Something like:

MapState(4, 4096, 4096, mapStateOptions().scroll(0.5f, 0.5f).scale(2f).magnifyingFactor(2))

Don't mind the naming, I just mean the idea.

from mapcompose.

This is exactly what I had in mind. For the naming, something like initialValues().

This is a bit verbose, and also requires launching a coroutine, which is not possible/convenient in all contexts.
Another issue is that such a created map requests tiles for the top-left map corner, which is extra HTTP requests that are useless.

Good point. This isn't appropriate for initial values. However, the fact that snapScrollTo is a suspend function is perfect for subsequent sequential operations (after MapState is initialized).

from mapcompose.

I'll take this one

from mapcompose.

You can have a look at the init-values branch.

from mapcompose.

Sweet, having a look now.

from mapcompose.

I'll have to add rotation. But you get the idea.

from mapcompose.

I can't get it to work in my app, but can't get it to not work in the test app. Fun...

from mapcompose.

When applyInitialValues is called, layoutSize is 0 for me, so offset isn't applied and the position ends up wrong (top left corner instead of middle of the map). Not sure why layoutSize is 0 for me when it works in the test app, I'm trying to find out.

from mapcompose.

I have an idea. In my last commit I made a change so that, in theory, we don't get layout size 0. Apparently that wasn't enough. Anyway it's getting late here, I'll add that tomorrow.

from mapcompose.

The idea is to only apply the initial values if we have a non null value and layout size is different than IntSize.Zero
Ideally, the MapState should never get a 0 size, but I'll fix that.

from mapcompose.

It's not as simple as it may look like. While fixing the issue you mentioned, I came across a new problem. What should we expect from this:

MapState(
     4, 4096, 4096,
     initialValues = InitialValues().rotation(45f)
).apply {
     addLayer(tileStreamProvider)
     enableRotation()
     rotation = 0f
}

Should the initial rotation be 45f or 0f? I would say, the last value wins (0f).
I have a working implementation, but since it all turned out to be not as easy as I thought, I'll make extra checks until I'm confident with this strategy.

from mapcompose.

Someone not knowing the internals of the library would expect 0f to win, I would say. You initialized the value to 45 and then set it to 0.

from mapcompose.

I was playing around with the code a bit and was thinking to just pass the initialValues to zoomPanRotateState as initial values there instead of zeros. Seems like it would make sense, but that would require more code to work But of course we still need to update the scroll by the offset when we have the layout size.

from mapcompose.

I was playing around with the code a bit and was thinking to just pass the initialValues to zoomPanRotateState as initial values there instead of zeros. Seems like it would make sense, but that would require more code to work But of course we still need to update the scroll by the offset when we have the layout size.

That's the idea for my last strategy. Will push it soon.

from mapcompose.

Here you go. I hope that this time it will work for your app.

from mapcompose.

Thank you, I likely won't have time to try it today, but will do tomorrow.

from mapcompose.

The scroll position seems to work fine now, but the scale does not, I think because it's constrained to the max scale before I have a chance to increase the max scale range. This would be solved by adding max scale to the InitialValues.

I have some thoughts on the API itself, I will post them later today.

from mapcompose.

I think because it's constrained to the max scale before I have a chance to increase the max scale range.

Yes indeed. I forgot to add a maxScale parameter. I'm adding that.

from mapcompose.

I proposed a bunch of updates in #39

from mapcompose.

Thanks, most of your PR is now in v2.

from mapcompose.

Not sure how I feel about you committing everything under your name. Same with my test tiles earlier. Could have just made your changes on my PR and merged the PR.

from mapcompose.

That's fair. I'm worried about making sure I'm not making unattended changes, that's why I prefer starting from a state that I know. This is how I work. What I can do is to change the author of the last commit and force push.

from mapcompose.

Given the amount of changes, I was immediately not feeling comfortable, even if most of it was actually good. There were some good ideas, and others that I definitely won't take. This kind of mix feeling has always the same consequence with me: I start from scratch.
These situations can be avoided by:

• discussing changes before making a PR
• making smaller PR

from mapcompose.

That PR was supposed to be a discussion of the changes (while seeing concrete code). I expected we'd reach some consensus (like we did), and then I'd update the PR to only contain the changes you liked.

from mapcompose.

Sure. In that case, I'm changing the git history -- sorry for that.

from mapcompose.

Done!

from mapcompose.

Thank you.

from mapcompose.