Re: [PR] MapWidget enhancements

2017-08-08 Thread Linus Torvalds
On Tue, Aug 8, 2017 at 2:45 PM, Lubomir I. Ivanov  wrote:
>
> updated the PR with a second patch to fix this:
> https://github.com/Subsurface-divelog/subsurface/pull/536
>
> it was caused by missing those dive sites which are skipped for being
> too close to each other when adding the flags.
> now it should work.

Ack. Tested and confirmed. This works.

Thanks,

Linus
___
subsurface mailing list
subsurface@subsurface-divelog.org
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface


Re: [PR] MapWidget enhancements

2017-08-08 Thread Lubomir I. Ivanov
On 8 August 2017 at 22:53, Linus Torvalds  wrote:
> On Sun, Aug 6, 2017 at 5:32 PM, Lubomir I. Ivanov  wrote:
>>
>> this PR implements the requested functionality to be able to:
>>
>> select a trip or any list of dives and the Map widget should estimate
>> a center coordinate and a zoom level so that the list of selected
>> dives locations are centered in the Map viewport
>> select a list of dives in the dive list based on selecting a list of
>> markers on the map
>
> Hmm. I think this would work fine as an interface, except for some
> reason it just doesn't work.
>
> When I zoom out to see all of the Palau sites, and then do "Select
> visible", it selects *most* of them. But definitely not all. And when
> I then select one of the sites it didn't pick, it zooms in to the area
> I just had selected, so it's not that I'd have the wrong GPS location
> for that non-picked site.
>
> I'll export my dives as xml, verify that it happens with that too, and
> send Lubomir a copy of that xml in private.
>

updated the PR with a second patch to fix this:
https://github.com/Subsurface-divelog/subsurface/pull/536

it was caused by missing those dive sites which are skipped for being
too close to each other when adding the flags.
now it should work.

the Map code still uses "fromCoordinate" (which i think doesn't always
work), so it would be good if the new functionality gets more testing.

lubomir
--
___
subsurface mailing list
subsurface@subsurface-divelog.org
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface


Re: [PR] MapWidget enhancements

2017-08-08 Thread Lubomir I. Ivanov
On 8 August 2017 at 22:53, Linus Torvalds  wrote:
> On Sun, Aug 6, 2017 at 5:32 PM, Lubomir I. Ivanov  wrote:
>>
>> this PR implements the requested functionality to be able to:
>>
>> select a trip or any list of dives and the Map widget should estimate
>> a center coordinate and a zoom level so that the list of selected
>> dives locations are centered in the Map viewport
>> select a list of dives in the dive list based on selecting a list of
>> markers on the map
>
> Hmm. I think this would work fine as an interface, except for some
> reason it just doesn't work.
>
> When I zoom out to see all of the Palau sites, and then do "Select
> visible", it selects *most* of them. But definitely not all. And when
> I then select one of the sites it didn't pick, it zooms in to the area
> I just had selected, so it's not that I'd have the wrong GPS location
> for that non-picked site.
>
> I'll export my dives as xml, verify that it happens with that too, and
> send Lubomir a copy of that xml in private.
>

yes, i can confirm the issue.
the logic in MapWidgetHelper::selectVisibleLocations() seemed pretty
straightforward, yet there might be a gotcha with the fromCoordinate()
method. i already reported it giving bad results for no apparent
reason.

i need to investigate this further.

lubomir
--
___
subsurface mailing list
subsurface@subsurface-divelog.org
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface


Re: [PR] MapWidget enhancements

2017-08-08 Thread Linus Torvalds
On Sun, Aug 6, 2017 at 5:32 PM, Lubomir I. Ivanov  wrote:
>
> this PR implements the requested functionality to be able to:
>
> select a trip or any list of dives and the Map widget should estimate
> a center coordinate and a zoom level so that the list of selected
> dives locations are centered in the Map viewport
> select a list of dives in the dive list based on selecting a list of
> markers on the map

Hmm. I think this would work fine as an interface, except for some
reason it just doesn't work.

When I zoom out to see all of the Palau sites, and then do "Select
visible", it selects *most* of them. But definitely not all. And when
I then select one of the sites it didn't pick, it zooms in to the area
I just had selected, so it's not that I'd have the wrong GPS location
for that non-picked site.

I'll export my dives as xml, verify that it happens with that too, and
send Lubomir a copy of that xml in private.

 Linus
___
subsurface mailing list
subsurface@subsurface-divelog.org
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface


Re: [PR] MapWidget enhancements

2017-08-07 Thread Lubomir I. Ivanov
added a new PR.

i think i found QDeclarativeMap (QML Map) bugs, so i try to work around those.

-
The QML Map has a couple of methods - toCoordinate(someQPointF) and
fromCoordinate(someQGeoCoordinate). Ideally, if one passes a point to
toCoordinate() and then attempts to convert the resulted coordinates
back to the point, the same point in the Map view port with minimal error
should be retrieved. That's not always the case - e.g. near
47.400200 -123.142066, which means that the methods are not exactly
*reliable* and there might be Map class bugs at hand.

The new zoom-in and zoom-out improvements try to work around the above:
- for both centerOnRectangle() and centerOnCoordinate(), if no good
zoom-out level is found (newZoomOut), the zoom-out is set to the default
value of defaultZoomOut. In practice, this prevents the case where the map
does not zoom-out at all when going from one place to another
- centerOnRectangle() now uses rectangle diagonals to estimate a fit,
instead of checking if 2 points (top-left and bottom-right) are visible
in the viewport. The usage of fromCoordinate() was giving bad results
in this case and comparing distances (diagonals) is more reliable
but more expensive on the CPU.

Due to the inconsistencies of toCoordinate() and fromCoordinate()
the dynamic zoom-in and zoom-out are still not ideal, but the current
implementation is somewhat usable with decent accuracy.
-

lubomir
--
___
subsurface mailing list
subsurface@subsurface-divelog.org
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface