Re: More changes in path drawing

[Robert Park <rbpark exolucere ca>]

Hey Jiri, as usual I am stuck at work and won't be able to test any of
this until I get home.

On Thu, Feb 17, 2011 at 8:48 AM, Jiří Techet <techet gmail com> wrote:
> In python the prefix gets removed so they both become set_position().
> ChamplainMarker inherits from ClutterActor and also implements
> ChamplainLocation. So what happens in your code is that
> clutter_actor_set_position() gets called instead of
> champlain_location_set_position() because it's found first (interfaces
> have probably lower priority). I bet if you use the "latitude" and
> "longitude" properties, everything will work fine.
>
> I fear that this would be confusing for every language supporting
> inheritance. Therefore, I'll rename the set_position() function to
> set_location() - after all the interface is called ChamplainLocation.

This would also explain that time that I called get_position() with
the expectation that it was the complement to set_position, and it
returned (0.0,0.0) instead of raising AttributeError, even before
set_position stopped working.

> 2. You don't initialize threads. Please do it - libsoup loads tiles on
> background in separate threads and gobject has to be aware of that. In
> the old python bindings we would call
>
> gobject.threads_init()
>
> before clutter initialization. Not sure about how it should be called now.

I guess just GObject.threads_init(). Will try this. Can you elaborate
on why this kind of thing is necessary? I feel as though there are
subtle changes going on at very deep levels that are beyond my
understanding, because this suggestion of yours is highly reminiscent
of that time that I had to switch from calling Clutter.init([]) to
GtkClutter.init([]). It always worked before, why do I have to switch
it now? What changed to make GtkClutter necessary? What changed to
make threads_init() necessary?

Anyway, I'll try this tonight.

> 3. DON'T use clutter_actor_set_parent() unless you want to make your
> life hard (clutter documentation says something very similar).
> ClutterStage implements ClutterContainer interface so use
> clutter_container_add_actor() instead. I'm pretty sure this will
> resolve the animation problems of actors inserted into the stage.

Ok, this one is news to me. I'm not sure why I started using
set_parent. Most likely my widgets weren't appearing and I don't
really know much about Clutter, so I quickly googled and that was the
first thing I found that 'worked', and I obviously missed the warning
(also I never heard of add_actor(), so I should probably review
Clutter docs).

> 4. Then there's this:
>
> self.photo_layer = Champlain.MarkerLayer()
>
> This calls the "default constructor" and initializes the selection
> mode to CHAMPLAIN_SELECTION_NONE. Inside the add_marker method of the
> layer there's the code
>
>  champlain_marker_set_selectable (marker, mode != CHAMPLAIN_SELECTION_NONE);
>
> that is, whatever you set before is reset according to the current
> layer mode. I'm wondering whether your markers are selected correctly
> because IMO the selection mode is reset once you insert the marker
> into the layer.

I'm not sure what you're getting at here. When I select photos in my
GtkListStore, the corresponding ChamplainLabels appear correctly with
correct selection color, no problems. Even if you ignore the problem
where the wrong set_position is causing clicked-on markers to
disappear, the selection color does flash briefly before the Label
disappears, so that indicates to me that the selection is working
fine.

This might be a good time for me to ask you, does the libchamplain
concept of a 'selected' marker have any meaning beyond simply
highlighting that marker with the system theme's selection color? Is
any other functionality associated with this concept? Because in my
app I already am using GtkTreeSelection to keep track which photos are
selected, and in the old API I was simply using get/set_highlighted to
identify ChamplainMarkers that corresponded to photos that were
selected in the GtkListStore. When you changed the API and took away
get/set_highlighted, I literally just did a
s/et_highlighted/et_selected/g in my source code and didn't think much
else about it. So to me, the get/set_selected methods are no more than
simple color highlights, and I already have well-tested code that
ensures only the correct Labels are set as 'selected'. Is this an
incorrect usage of "selections" in libchamplain?

> This is about what I've noticed after quickly looking at your code.
> The image corruption is strange and shouldn't happen - I'm now adding
> clutter_actor_queue_redraw/relayout where necessary which could help
> to fix some redrawing problems.

Ok, when I get home I will try out add_actor() instead of set_parent()
to see if this reduces any problems. Thanks!

-- 
http://robru.github.com/GottenGeography/