[cairo] Finding minimum distance to a stroke or fill

[Carl Worth]

On Sat, 12 Feb 2005 02:28:14 -0500, Ashwin Bharambe wrote:
>
> Attached is a patch which adds two new functions to the cairo API.

Hi Ashwin,

Thanks for the contribution.

I apologize for not having replied any sooner. I thought the patch
looked useful, but not exactly ready yet, so I left it in my inbox to
get back to at some point.

Now, a recent new thread reminded me of this patch, so I'm looking at
it again.

And fortunately, we're using better tools than we were a year ago. So
now, instead of letting a not-quite-ready-patch just sit, I can
actually do something with it.

So, I created a distance-to-path branch in my personal repository.

The gitweb interface actually provides a very good view of this:

http://gitweb.freedesktop.org/?p=users-cworth-cairo;a=log;h=distance-to-path

Only the first few commits are unique to this branch, (the first has
Ashwin as the author).

A couple of things to notice from this exercise:

1) I had a place to easily commit a patch, (even one that didn't
   compile).

2) The patch as-committed retains proper attribution, (Ashwin as
   author, me as committer).

3) I was able to fix the bitrot with a separate commit along the
   branch (so the head of the distance-to-path branch does compile and
   work now).

4) Even though the current result still isn't ready to be committed to
   cairo itself, it's already visible to the public and in a way that
   people can very easily build on.

(OK, I'll stop praising git now---but it is definitely doing exactly
what I hoped it would do for my bad patch backlog here.)

Back to the topic of the patch itself, I'd still like to change some
things before we merge it in to cairo itself. The proposed API is:

> double
> cairo_min_distance_stroke (cairo_t *cr, double x, double y, double
> *px, double *py);
> 
> double
> cairo_min_distance_fill (cairo_t *cr, double x, double y, double *px,
> double *py);

I don't like "min" in there. It's both an abbreviation, and also
redundant, ("distance" alone is enough to carry the meaning of minimum
distance I think).

How about the following names instead:

	cairo_distance_to_stroke

	cairo_distance_to_fill

And does the return point really belong in this interface? I think I'd
prefer to just return the distance. That functionality matches the
names well. Of course, if its actually the nearest point that is
desired, that would require separate new functions, but I think I'd
prefer to add that functionality under a separate name rather than
using one name for both purposes here.

Ashwin, in your original use case, do you need the point or is the
distance alone sufficient for what you were doing?

The other optimization suggested down-thread was returning a squared
distance to avoid a bunch of calls to sqrt() that might not be
necessary at all in common uses of this function.

Internally to cairo I've used naming of "distance_squared" to make the
distinction clear from any "distance" function. Should we actually
provide all four functions:

	cairo_distance_squared_to_stroke
	cairo_distance_to_stroke
	cairo_distance_squared_to_fill
	cairo_distance_to_stroke

That puts a rather long name on the commonly needed function, (which
could be a minor performance trap), and it does put two extremely thin
convenience functions in to cairo (which is something we don't usually
do).

However, I don't think I'd like to just define
cairo_distance_to_stroke/fill as returning squared distances since
that buries some fairly essential (in certain cases) information about
the return value in the documentation rather than the name, (which is
something else we try to avoid in cairo).

Thoughts and suggestions?

> Thanks,
> Ashwin
> 
> PS: Also, I must say the cairo API and code is so amazingly clean that
> it was a pleasure hacking this up! Kudos!

You're most welcome. It really made my day to read this last
year. (And I've waited all this time to be able to tell you that since
I thought the least you deserved at the same time was some comment on
your patch.) Again, I'm sorry this review was so slow.

-Carl
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : http://lists.freedesktop.org/archives/cairo/attachments/20060301/8ca24882/attachment.pgp