Add wrapX support for vector layers (canvas renderer only) #3571
Conversation
ahocevar
force-pushed
the
vector-wrapx
branch
5 times, most recently
from
b6cb1d6 to
ffc0145
Compare
ahocevar
force-pushed
the
vector-wrapx
branch
2 times, most recently
from
e90559c to
ec110c9
Compare
examples_src/draw-features.js
Outdated
There was a problem hiding this comment.
If you rebase on master, I think the 3-way merge will do the right thing (make these changes in examples instead of examples_src).
|
Rebased, 3-way merge did the trick. |
|
@ahocevar - you mentioned on Thursday that you were going to make some changes. Is this ready for review now? |
externs/olx.js
Outdated
There was a problem hiding this comment.
I haven't tried, but is 0° really a problem? Or only -180° and 180°?
|
@tschaub Yes, this is ready for review - and thanks for starting the review. |
|
Any more comments on this? |
src/ol/proj/proj.js
Outdated
There was a problem hiding this comment.
Not sure about this one. The Mollweide projection is global but it doesn't/cannot have an extent.
There was a problem hiding this comment.
Makes sense. This means I have to remove this assertion and check for extent available elsewhere before doing any wrapping. I can do that.
There was a problem hiding this comment.
Maybe you can skip the isGlobal test?
There was a problem hiding this comment.
When using a projection for transforming coordinates only, you don't need an extent - and you also don't need to set isGlobal. I think the best will be to remove the assertion altogether like you suggested. To avoid checking for the same conditions over and over, I can add a canWrapX function (which returns with a cached value) to ol.proj.Projection.
There was a problem hiding this comment.
I was just wondering why wrapX would only work for "global" projections. What makes "global" projections special wrt wrapping the "world"?
There was a problem hiding this comment.
For "global" projections, we assume that the projection's extent is the extent of the world. So if we wrap e.g. a Swiss projection around its extent, Switzerland will be repeated several times across the globe.
There was a problem hiding this comment.
Not sure I completely follow, but it's probably me so can you just ignore me for now :)
I thought someone may want to use wrapX even though his projection is not "global" (i.e. covering the earth).
There was a problem hiding this comment.
We could avoid the test by making the offsetX argument non-optional, and passing 0 when no offset is wanted.
|
@elemoine I may have a better approach for map replays: render them on the correct world instead of repeating them. I'll give myself an hour to see if this is feasible and report back here. |
|
@elemoine I was able to implement what I had in mind, and I am very happy with the result. I hope you are too - there is no more canvas replay wrapping 😄. |
|
Cool. Do you mind explaining how it now works? I don't understand the relation to the frame state focus. And I don't understand how features from feature overlays are now repeated when the viewport spans multiple words. Thanks. |
|
Feature overlays are not repeated/wrapped any more. Instead, the frameState's focus is used to determine the world where the feature overlay is rendered. You can see this in action e.g. In the vector-layer.html example: if you zoom out so that multiple worlds are visible and hover over a country, it only gets highlighted under your pointer, and not in other worlds. Same for feature selection in the select-features.html example. |
|
Thanks for the explanations @ahocevar! It's much clearer now. I've just added one last comment. Please merge when addressed. |
By using the frameState's focus, we can adjust extent and transform and render it for the world of interest instead of wrapping it and rendering for every visible world.
Add wrapX support for vector layers (canvas renderer only)
|
one thing I noticed which might be related to this PR, the zoom box control can behave strangely, sometimes it doesn't show or it shows up in a different world. |
|
Another gentle reminder that the Box interaction should draw a dom element and not a feature on a vector overlay... |
|
Right |
4 participants
This pull request adds wrapX support for vector layers, vector overlays, and the map's replay group. It also ensures that
ol.Map#forEachFeatureAtPixel()works when viewing a wrapped world.