Snapping text objects to guidelines dont work right.

[Bug Importer]

Repro steps:

1. Make 2 text objects, first one "Blah", second "Quack!"
2. Make horisontal guideline
3. Drag both objects on that guide.

-> Result is that 'Blah' and 'Quack' text objects are
not aligned correctly.

When aligning text objects to guidelines, font baseline
should be used, not font bounding box.

This bug is font dependant. It atleast is reproducable
with default font (Bitstream Vera Sans).

My contact email: <email address hidden>

[Bryce Harrington (bryce)]

Originator: NO

Confirmed on Gentoo Linux.

Agreed that snapping for guidelines ought to snap to font baselines. We
have the ability to do this - it aligns correctly when using the
align-baseline-anchors in the alignment dialog.

[Buliabyak-users (buliabyak-users)]

Originator: NO

Confirmed. SPText does have sp_text_snappoints that sets a snappoint at
baseline, but it doesn't work for some reason. dvlierop2, can you please
have a look why it doesn't work?

[Dvlierop2 (dvlierop2)]

Originator: NO

It's because of

    _snap_points = selection->getSnapPointsConvexHull()

which discards the baseline points. We could also use

    _snap_points = selection->getSnapPoints()

but this might return too much snapppoints. Do we need something like
this

    _snap_points = selection->getSnapPointsConvexHullAndFontBaseline()

???

[Buliabyak-users (buliabyak-users)]

Originator: NO

I don't think it can return too many snappoints without the hull. Path
nodes are not snappable in text objects, so I think it will be only the
baseline and the bbox that snap. Convex hull was introduced partly as an
attempt to fix the brokenness of the current system. Namely, it was
discovered that with "snap nodes" mode, moving an object with thousands of
nodes took literally hours while it decided which node best to snap. So the
number of snappable nodes was limited by the convex hull. This will
eventually become unnecessary if we switch Selector to always work with the
bounding box (although the bbox may be enclosing the nodes, being thus
roughly similar to the convex hull). In all other cases where the number of
special snappable points is known and limited, I think we can remove the
convex hull already and just use all the points for snapping.

[Dvlierop2 (dvlierop2)]

Originator: NO

Using getSnapPoints() instead of getSnapPointsConvexHull() will indeed do
for textobjects. This method however is being called in seltrans.cpp for a
selection, which could be a single text object, but could also be a
selection having a huge number of nodes, right? So should I check whether
the selection is a text-object, and only if so use getSnapPoints()? I could
also use getSnapPoints() only if the number of nodes is smaller than e.g.
1000.

Thanks for your elaborate reply, this is really helpful!

[Buliabyak-users (buliabyak-users)]

Originator: NO

I think you are right. Selecting multiple objects in Selector is one
situation where convex hull may be necessary, but not always. There's no
reason to restrict the snappoints when only a few objects are selected;
this most likely will not be expected by the user. But when too many
objects are selected, we may be forced to hull them for performance
reasons. Unfortunately the performance of the snapper seems to be very
nonlinear - more like exponential - depending on the number of nodes
(although perhaps this only applies to node-to-node or node-to-path
snapping? I don't remember details). You might want to experiment to find
out which modes need the hull restriction and at what number of nodes it
needs to start.

So, just to summarize our plan so far, sorry for endless repetition - I do
it for myself too, to make sure I have the full picture without missing
anything:

- transform origin option is removed, origin always at bbox

- the snap nodes/bbox options are removed (in all three snapping
categories)

- an option is added for visual/geometric bboxes (mental is working on
that)

- all shape handles always snap to all active snapping modes
(grid/guides/path) [DONE] except when dragging with Shift

- all object-creating tools (except calligraphic and paintbucket) snap the
initial point of drawing; I think this is already done for shape tools and
pen/pencil, but you need to ensure that for gradient and text tools (i.e.
clicking or dragging in text tool must produce a text object already
snapped)

- all shapes must provide their special points for snapping: at least all
positions where their handles are, plus things like center, rotation axis
(settable by seltrans), baseline for text, ends of spiral, etc.

- in node tool, all _selected_ path nodes (but not bbox or special
snappoints) must snap when moving by mouse. Only when this is necessary for
performance reasons, selected nodes are hulled and the hull is snapped.

- in selector, always snap the bbox(es) and the special snappoints (but
not path nodes). Again, when necessary for performance reasons with too
many objects selected, snappoints are hulled and the hull is snapped.

The only problem that I foresee is that if the user uses visual bbox
(including stroke) but some of the special snappoints (such as a star's
vertices) coincide with the path nodes. Then the distance between them
would be too small (equal to half the stroke width) and snapping in
Selector may be slightly ambiguous. But I don't think it's going to be much
of a problem.

Let me know if I missed anything.

[Dvlierop2 (dvlierop2)]

Originator: NO

I've had a look at it, but the performance of the snapper appears to
depend on the number of nodes in the selection, the number of nodes outside
the selection, whether they're overlapping, which snap options are turned
on, etc. And it certainly isn't linear. This makes it hard to make some
intelligent decision on whether to hull or not. therefore I opt for bluntly
limiting the number of nodes to 100. At that number the lag can already be
very annoying (with all snap options turned on, on my AMD 3000+ box).

I guess to only real solution would be to use a watchdog timer, but I
don't like that idea very much.

This is the new code:

    _snap_points = selection->getSnapPoints();
    if (_snap_points.size() > 100) {
        /* Snapping a huge number of nodes will take way too long, so
limit the number of snappable nodes
        An average user would rarely ever try to snap such a large number
of nodes anyway, because
        (s)he could hardly discern which node would be snapping */
        _snap_points = selection->getSnapPointsConvexHull();
        // Unfortunately, by now we will have lost the font-baseline
snappoints :-(
    }

As far as I can tell, you didn't miss anything in your list. I'll have
this bug report bookmarked for reference ;-)

[Buliabyak-users (buliabyak-users)]

Originator: NO

OK, until someone goes into the guts of the snapper and invents some smart
optimization, I think the <100 hack is the best approach for the time
being.

Well, two optimizations that come to mind immediately: When, at current
zoom, two snappoints are closer to each other than one screen pixel, one of
them is discarded. Also, only those snappoints that are actually on screen
are used; those that are out of view are discarded. I think this will
reduce the snappoints to a manageable number in most cases; only if these
approaches fail we can take a hull.

However I cannot yet close this bug: text now snaps with the baseline, but
only if you enable "snap nodes". Just like shape handles, it should snap
whenever any snapping mode is on, until (hopefully soon) these modes are
removed completely (see plan below :)

[Dvlierop2 (dvlierop2)]

Originator: NO

Currently, we either snap to the bounding box or to the snappoints. In the
latter case, the baseline is included, but in the former it isn't, which is
the reason this bug hasn't been closed yet.

As discussed before you plan to have the "snap nodes / snap bbox" option
removed. The node tool will only snap to nodes of course, but will the
Selector tool then ONLY snap to bbox? If so, the Selector tool will never
snap to font-baselines. If on the other hand we make the Selector tool snap
to anything (bbox+nodes), then the baseline will always be snapped. Is this
the way to go? I think this would make the selector tool useless for
objects having a large number of nodes. It makes it impossible to align its
hull, as one could never tell if the bbox is snapping or one of its many
nodes...

This is from our discussion in bug #1540195:

Further, perhaps by this change we will also be able to get rid of the
"snap bboxes/snap nodes" choice. Selector will always snap bbox (although
this bbox may be either visual as now, or including only nodes). Node
tool
will, naturally, always snap only nodes. Less preferences overall and a
saner, more predictable experience. What do you think?

[Buliabyak-users (buliabyak-users)]

Originator: NO

> As discussed before you plan to have the "snap nodes / snap bbox"
option
removed. The node tool will only snap to nodes of course, but will the
Selector tool then ONLY snap to bbox?

No, see plan below:

> in selector, always snap the bbox(es) and the special snappoints (but
not path nodes).

So you snap bbox and special points, which includes baseline. But not path
nodes - they only snap in node tool (which works approximately as selector,
treating each node as a zero-dimensions object that just snaps when
moving).

So, special points are different from path nodes. And no object will have
too many special points, so your fears are not justified :) It may have
thousands of path nodes but always a limited set of special points,
depending on its type.

[Dvlierop2 (dvlierop2)]

Originator: NO

I think it's dawning on me ;-). It took me the better part of this day
though to find out how all special snappoints were being returned to the
Selector tool. The way the virtual functions are being handled was new to
me, so I learned a lot today :-)

For the selector tool we currently use

_snap_points = selection->getSnapPoints();

which returns all special snappoints but also the path nodes. I guess the
first thing to do would be to introduce a parameter for this method,
specifying whether the path nodes should be included or not. For the time
being we can set this TRUE, but later when we implement the new selector
tool we can set it to FALSE. At that point we will use bbox +
"special-points excluding the path nodes".

This is one of the changes under the hood which we can make at this point,
without bothering the users, and keeping the transition phase to the new
selector tool as short as possible. I still have to replace all origins
with the bbox_origin...

BTW Bulia, I still need some input on

https://sourceforge.net/tracker/?func=detail&aid=1673807&group_id=93438&atid=604309

Could you have a look?

[Buliabyak-users (buliabyak-users)]

Originator: NO

Does the Node tool use the same selection->getSnapPoints() ? If it does,
then yes, we need to add a parameter to it telling it to return either path
nodes or special points (not both). But if, as I suspect, it does not, then
we don't need to add this parameter. Just make it ignore the path nodes and
that will be it.

And I think we can do this right now, no need to wait when the new bbox
option gets checked in.

I will have a look at that other bug now.

Thanks!

[Dvlierop2 (dvlierop2)]

Originator: NO

You were right once again!

The selector tools no longer snaps to path nodes. That was part one of
this quest...

[Buliabyak-users (buliabyak-users)]

Originator: NO

OK, now text snaps with baseline, though not by default - you need to
enable "snap nodes". Still I guess we can close this.

Diederik, what is the next step? Can you already eliminate the snap
nodes/snap bbox options? Or the scale origin option?