GNOME Bugzilla – Bug 764611
Champlain_view_ensure_layers_visible doesn't work for markers right on the edge of the map
Last modified: 2016-04-08 22:41:15 UTC
Created attachment 325378 [details] Markers on the edge block the champlain_ensure_layers_visible function. Using champlain_ensure_layers_visible when there are markers with coordinates equal to CHAMPLAIN_MAX(min)_LONGITUDE(latitude) doesn't work. See the attached video! This might look like a very unlikely corner case, but it's actually quite easy for a user to drag markers outside the map so that they get clamped right on the edge. I have to figured out two fixes for this: 1) Change the champlain_bounding_box_is_valid to include extremities as well. Since markers(and other things) can reach the maximum coordinates constants, it would be obvious for the bounding box to be valid in such circumstances as well. I've cooked up a patch in advance for this one since I believe it to be the right fix. 2) if there is a reason why the champlain_bouding_box_is_valid doesn't include the extremities, maybe limiting the markers to coordinates a bit smaller than the max constants is a better idea.
Created attachment 325379 [details] [review] champlain-bounding-box: Include extremities in champlain_bounding_box_is_valid function
(In reply to Marius Stanciu from comment #0) > Created attachment 325378 [details] Hi, Thanks! > Markers on the edge block the champlain_ensure_layers_visible function. > > Using champlain_ensure_layers_visible when there are markers with > coordinates equal to CHAMPLAIN_MAX(min)_LONGITUDE(latitude) doesn't work. > > See the attached video! > > This might look like a very unlikely corner case, but it's actually quite > easy for a user to drag markers outside the map so that they get clamped > right on the edge. > > Is it really? And what is the use-case? > I have to figured out two fixes for this: > > 1) > Change the champlain_bounding_box_is_valid to include extremities as well. > > Since markers(and other things) can reach the maximum coordinates constants, > it would be obvious for the bounding box to be valid in such circumstances > as well. > > I've cooked up a patch in advance for this one since I believe it to be the > right fix. > The Mercantor projection is not valid for > 85.103 latitudes, so the conversion from latitude=>x will yield NaN > 2) if there is a reason why the champlain_bouding_box_is_valid doesn't > include the extremities, maybe limiting the markers to coordinates a bit > smaller than the max constants is a better idea. This might be a better idea. Not sure.
(In reply to Jonas Danielsson from comment #2) > (In reply to Marius Stanciu from comment #0) > > Created attachment 325378 [details] > > Hi, Thanks! > > > Markers on the edge block the champlain_ensure_layers_visible function. > > > > Using champlain_ensure_layers_visible when there are markers with > > coordinates equal to CHAMPLAIN_MAX(min)_LONGITUDE(latitude) doesn't work. > > > > See the attached video! > > > > This might look like a very unlikely corner case, but it's actually quite > > easy for a user to drag markers outside the map so that they get clamped > > right on the edge. > > > > > > Is it really? And what is the use-case? > > > I have to figured out two fixes for this: > > > > 1) > > Change the champlain_bounding_box_is_valid to include extremities as well. > > > > Since markers(and other things) can reach the maximum coordinates constants, > > it would be obvious for the bounding box to be valid in such circumstances > > as well. > > > > I've cooked up a patch in advance for this one since I believe it to be the > > right fix. > > > > The Mercantor projection is not valid for > 85.103 latitudes, so the > conversion from latitude=>x will yield NaN > > > 2) if there is a reason why the champlain_bouding_box_is_valid doesn't > > include the extremities, maybe limiting the markers to coordinates a bit > > smaller than the max constants is a better idea. > > This might be a better idea. Not sure. Or maybe your patch is the best idea, maybe just make sure that the latitude_to_y functions and longitude_to_x function still work at extremes?
(In reply to Jonas Danielsson from comment #3) > (In reply to Jonas Danielsson from comment #2) > > (In reply to Marius Stanciu from comment #0) > > > This might look like a very unlikely corner case, but it's actually quite > > > easy for a user to drag markers outside the map so that they get clamped > > > right on the edge. > > > > Is it really? And what is the use-case? I didn't mean this is useful, it's just a high probability the user will drag markers right on the edge once in a while, since they get clamped there when the pointer leaves the map. > Or maybe your patch is the best idea, maybe just make sure that the > latitude_to_y functions and longitude_to_x function still work at extremes? I've tested the coordinate conversion functions and most of the API, all seems ok with the patch.
LGTM. We already clamp latitude and longitude at other places to the MIN/MAX values and things work so I don't see a reason not to have the min/max borders of the bounding box valid.