[Qgis-developer] Latest features ready to commit - which branch?

[Brendan Morley]

Hi Marco,

On Tue, 2006-06-27 at 10:38 +0200, humarco wrote:

> Nice to see you are back!

I am on some holidays at the moment, I think I will be overwhelmed again
come August...

> As far as i see, you changed the snapping code for moveVertex such that it 
> uses snap to segment now instead of snap to point? In my opinion, this is a 
> new feature and not a bugfix.

The new code (internally) snaps to the nearest segment first, then to
the nearest vertex on that segment.

Why?  My main hassle was in the situation where I'm modelling a street
intersection and I want to move one of the "streets" away from that
intersection, previous versions of QGIS would not allow me to choose
which street.  Fine, it would snap to the appropriate vertex
(representing the street intersection) but the street (linestring) that
that vertex belonged to was arbitrary.

> It is pretty risky to make this in the current 
> phase before the release. 

I considered it a usability bug, so acted appropriately.  Perhaps I
should have lodged a bug in trac beforehand, so the bugfix-commit comes
as less of a surprise to people?

> In this case, this new feature breaks rubber 
> banding for moveVertex.

My apologies.  Sometimes bugfixes cause other bugs.  Perhaps I can
attempt to restore the functionality?

> The reason for this is that snap to segment does not set the rubber band 
> indexes. This was not necessary up to now, since snap to segment was only 
> used for 'addVertex' and for this, the rubber band indexes are just 
> vertexNumber-1 and vertexNumber+1. For 'moveVertex', rubber banding is more 
> complicated, because the vertex to be moved could be the first vertex of a 
> line or the first/last vertex of a polygon ring. Therefore, the indexes of 
> the rubber band points are determined in QgsGeometry::closestVertex (but not 
> (yet?) in QgsGeometry::closestSegmentWithContext).

I disagree that rubber banding should be handled in QgsGeometry.  I
believe it is a property of the QgsMapToolVertexEdit (seeing as it is
eye-candy to the Vertex Editing process), and really should have been
coded there.

I suggest the QgsGeometry closest segments tests should be more
generalised, without the overhead of calculating the fore- and
aft-vertex each time.

I agree though that rubberbanding does have its special cases.

> > 3. In Vertex editing, add the ability to turn snap-mode (snap the vertex
> > being edited to other verticies in the same layer) off.
> 
> At the moment, snap-mode can be turned off by setting the tolerance to 0. Ok, 
> it may be not so convinient. If you plan to make a checkbox in the gui i 
> suggest to do so after 0.8.

Hmmm.  The problem it if you set the tolerance to 0, you can't actually
select a vertex to edit or a segment to add a vertex to any more!

Therefore there should be two settings - one to determine how far away
to search to find which segment or vertex to edit (which we already
have) and a second (new) setting to determine if we snap to existing
verticies when editing the target vertex.

I find it's a huge usability issue to not have the second setting
customisable.  Things keep snapping to nearby verticies, and (as a side
issue) each snap search causes the data provider to do a search back to
the source data (this gets expensive when it comes to say a PostGIS
provider, and certainly is not enterprise-friendly).


You'll notice I have a bit of a thing about usability (i.e. things
should "just work" with few surprises for the user), and consider
usability issues as bugs - I'm not sure if that developer body shares my
enthusiasm and therefore again invite comment.


Thanks,
Brendan