Re: Many changes in layers and markers

[Jiří Techet <techet gmail com>]

Hi Robert,

On Thu, Feb 10, 2011 at 00:31, Robert Park <rbpark exolucere ca> wrote:
> On Wed, Feb 9, 2011 at 2:46 PM, Jiří Techet <techet gmail com> wrote:
>> I have finally finished the major changes I wanted to get into 0.10
>> (see the code in our git). Sorry for the delay. In the end there were
>> more API changes I originally planned - because the existing programs
>> will have to be updated anyway, I thought it would be better to make
>> all the cleanups in one release rather than during several releases. I
>> hope that the current architecture is flexible enough that no other
>> major changes will be needed in future releases.
>
> I hadn't seen a commit since early January and I was starting to worry
> that libchamplain was dead... nice to see all this work done all of a
> sudden, it's like Christmas all over again ;-)

Yes, that's a bad habit of mine that I don't like when the dirty work
in progress is visible.

>
> Also, my app hasn't been subject to any API breakage in like two
> months, so this'll be fun.
>
>> * The license and the scale actors have been separated from
>> ChamplainView.
>
> Excellent! A while back I had asked for a coordinate display feature
> and you told me that this was a prerequisite to adding such a feature,
> so I'm glad to see this step in the right direction.
>
>> ChamplainBaseMarker -> ChamplainMarker
>> ChamplainMarker -> ChamplainLabel
>> ChamplainPoint - it is now a circular marker that can be inserted into a layer
>> ChamplainCustomMarker - a marker that implements ClutterContainer -
>> use instead of adding ClutterActors into ChamplainMarker
>> ChamplainPolygon -> ChamplainMarkerLayer (also the name _polygon_ was
>> changed to _path_ which is more descriptive in my opinion)
>> ChamplainLayer -> ChamplainMarkerLayer
>
> I like the idea of renaming 'polygon' into 'path' because that is a
> much less technical name (and more indicative of it's use), but I'm
> not sure about the name 'ChamplainMarkerLayer'. The name doesn't make
> it obvious that that's what you need to use when you want to draw
> squiggles on the map. Perhaps it should be called
> ChamplainDrawingLayer? That seems like a more general name that would
> indicate it's used for drawing points or polygons.

It's because you have to insert markers inherited from ChamplainMarker
into ChamplainMarkerLayer, which are then also used as nodes of the
path.
The path is just a property of the marker layer - you display or hide
the connection between the markers inside the layer by
champlain_marker_layer_set_path_visible():

static void
append_point (ChamplainMarkerLayer *layer, gdouble lon, gdouble lat)
{
  ChamplainPoint *point = champlain_point_new ();
  champlain_marker_set_position (CHAMPLAIN_MARKER (point), lon, lat);
  champlain_marker_layer_add_marker (layer, CHAMPLAIN_MARKER (point));
}

...

layer = champlain_marker_layer_new_full (CHAMPLAIN_SELECTION_NONE);
append_point (layer, 45.4095, -73.3197);
append_point (layer, 45.4104, -73.2846);
append_point (layer, 45.4178, -73.2239);
champlain_marker_layer_set_path_visible (layer, TRUE);


A side note: today I made a few more renames mostly in the
ChamplainMarker and ChamplainMarkerLayer code. Hope it won't cause too
much trouble.


>
>> champlain_view_get_coords_at() -> champlain_view_x_to_longitude() and
>> champlain_view_y_to_latitude() to get the latitude and longitude from
>> x, y
>
> If these two methods return only a float with no success boolean, then
> that totally obviates the need for those python overrides I wrote.
> Excellent!

Previously the boolean return value returned always TRUE so it didn't
make any sense to keep it.

>
>> So far I have concentrated on interfaces mostly and the implementation
>> itself definitely deserves some more work:
>>
>> * fix #638652 - this is my top priority bug now.
>
> Dear god *yes*. THANK YOU.
>
>> I know that chaning the API of a library is a good way to be hated, on
>
> Hahaha. Yeah.
>
>> the other hand the changes will make the architecture much more
>> flexible and maintainable. For instance, ChamplainView doesn't have to
>> care about markers and polygons which was the case previously (1200
>> LOC less in ChamplainView).
>
> Excellent work my friend!
>
>> It only knows about the inserted
>> Champlainlayers and its the responsibility of the layers to perform
>> the necessary actions. If you want to create your special layer or
>> marker, no problem - no change in ChamplainView is needed and you can
>> do whatever you wish.
>
> Cool. So what would be the recommended way to go about implementing a
> custom widget on the map that's pinned to the view, but not the map?

I'm not sure if I was clear enough - ChamplainLayer always moves
together with the map so this is not the one you should use for custom
widgets.

How to implement a custom widget? There are absolutely no restrictions
on how the widget should be implemented apart from that it has to be a
ClutterActor. At the moment we have 2 widgets - ChamplainScale and
ChamplainLicense. They are both "active" which means they react to
what happens with ChamplainView. In order to make them active, they
have to connect to the signals of ChamplainView they are interested
in. This is what happens in champlain_license_connect_view() and
champlain_scale_connect_view() - there is also their opposite for
disconnecting. ChamplainScale connects to notify::latitude and
ChamplainLicense to notify::map-source. Of course if your widget is
"passive", you don't need any function like that. You can then use the
layout manager of the view to position the widget where you want.

So for instance to insert the scale to the left bottom corner, use
something like that:

  scale = champlain_scale_new ();
  champlain_scale_connect_view (CHAMPLAIN_SCALE (scale), view);

  /* align to the bottom left */
  layout_manager = champlain_view_get_layout_manager (view);
  clutter_bin_layout_add (layout_manager, scale,
                          CLUTTER_BIN_ALIGNMENT_START,
                          CLUTTER_BIN_ALIGNMENT_END);

Alternatively you can put it at exact position directly into the stage.

That's it. Again, "scale" from the above example can be arbitrary
ClutterActor, there's nothing special about it.

Cheers,

Jiri


>
>> This is a good time to tell me:
>> 1. If something is missing in the new marker API so it can be added
>> 2. Something should be renamed
>> 3. If you think that the change of the API is totally stupid
>> All kinds of feedback are welcome.
>
> I'll begin porting my app over the next couple days and let you know
> what I think of the changes.
>
>> For people who wish to test the new features and don't have GTK3
>> installed yet, I have prepared a patch (see the attachment) which
>> makes libchamplain compile under GTK2. The launcher-gtk demo now
>> contains movable markers connected with a polygon line so you can test
>> how it's updated when dragged.
>
> That sounds like it would be fun to play with. I'm definitely
> interested in this draggable interface, it would be very useful to my
> app as users are allowed to manually position photos. Currently they
> do this by moving the map view, and then there's a button to move the
> selected images into the map center. Being able to drag the markers
> directly into position would certainly be a nice UI improvement.
>
> Thanks for all your hard work!
>
>
> --
> http://exolucere.ca
>