Re: New tile unref logic and various leak fixes

[Jiří Techet <techet gmail com>]

Hi,

I've just pushed some more patches fixing bugs of libchamplain. Most
notably, the following commits fix important bugs that are kind of
blockers for my application:

12463dff
In commit 98ab7c94 I've introduced moving marker in the animated
marker demo (that kind of simulates GPS unit). Without the patch
applied, you can see that the demo doesn't work properly - there
appear larger and larger stripes of non-loaded tiles in the top of the
window. The reason is that priv->viewport_size wasn't updated after
recalculating anchor in champlain_view_center_on().

686bbd6e
There was a bug that when you were panning north (i.e. moving the
contents of the window down), after some time you couldn't pan any
further and the window position kept returning back to the previous
position (try the demos/launcher demo, keep the default zoom and just
pan directly to the north. After about  30s of panning you'll get
blocked). The reason was the condition determining whether anchor
should be recalculated, which was wrong. I've changed it to the
following:

  if (priv->anchor_zoom_level != priv->zoom_level ||
      x - priv->anchor.x - priv->viewport_size.width / 2 >= G_MAXINT16
- 2 * priv->viewport_size.width ||
      y - priv->anchor.y - priv->viewport_size.height / 2 >=
G_MAXINT16 - 2 * priv->viewport_size.height ||
      x - priv->anchor.x - priv->viewport_size.width / 2 <= 0 +
priv->viewport_size.width ||
      y - priv->anchor.y - priv->viewport_size.height / 2 <= 0 +
priv->viewport_size.height )
    need_update = TRUE;

Might need some explanation - first, I perform the update one viewport
size before the border just to be sure (to avoid some rounding errors
and so on). So without this safety margin, the condition would look as
follows:

  if (priv->anchor_zoom_level != priv->zoom_level ||
      x - priv->anchor.x - priv->viewport_size.width / 2 >= G_MAXINT16
- priv->viewport_size.width ||
      y - priv->anchor.y - priv->viewport_size.height / 2 >=
G_MAXINT16 - priv->viewport_size.height ||
      x - priv->anchor.x - priv->viewport_size.width / 2 <= 0 ||
      y - priv->anchor.y - priv->viewport_size.height / 2 <= 0 )
    need_update = TRUE;

The extra viewport size in the maximal margin check are there because
we are checking the position of top left corner, so the whole viewport
will be between G_MAXINT16 - priv->viewport_size.width and G_MAXINT16
and we don't get over G_MAXINT16. For the minimal margin check we
don't need this extra space because there is nothing above top-left
corner. Compare this with the original condition to see what was
wrong. (The reason for subracting priv->anchor.x and
priv->viewport_size.width / 2 is that you pass the _centre_ of the
"full" [including the anchor] coordinates as the function arguments.)

The other fixes are a) fixes of potential problems that could occur
and b) some readability improvements.

Please let me know if you'll apply the fixes. So far I haven't
received any feedback from you in this respect. I want to base my
application on libchamplain (that's why I did all the work) but
without the fixes present in 0.6 this would be practically unusable
because of the hard to accept leaks (I use libchamplain for
downloading tiles offline and when downloading 16000 tiles the leaks
reached such dimensions that my 4GB RAM and 4GB swap weren't enough)
and because the above two bugs make it impossible to use libchamplain
with GPS unit for position tracking.

Thanks,

Jiri



On Wed, Mar 10, 2010 at 13:11, Pierre-Luc Beaudoin
<pierre-luc pierlux com> wrote:
> On Wed, 2010-03-10 at 01:59 +0100, Jiří Techet wrote:
>> Done. The difference when launcher-gtk is maximized (well, and if you
>> have 24' monitor like me) and scroll fast is quite significant.
>
> Indeed we can feel the change when panning fast!
>
> Good job!
>
> Pierre-Luc
>