[cairo] [PATCH 0/5] Fix dashing with zero length dash intervals.

[Carl Worth]

On Sun, 9 Apr 2006 23:10:36 -0400, Jeff Muizelaar wrote:
> 
> The following series of patches fixes dashing when the segment lengths are 0
> (#5561). Currently cairo ends up using unitialized data for the faces of the
> lines and consequently doesn't render anything useful. The patchset can be
> applied by piping all of the messages into git-am.

Thanks for contributing these patches. They look quite useful.

Patches #1 to #3 look entirely fine to me. Please feel free to push
them out to the central tree if you would like. (Though do also update
any test suite reference images if the improved dash accuracy affects
them.)

As for patch #4, (the one that specifically deals with the zero-length
dash behavior), and #5 that adds the test for this---I'd like to know
a bit more about those.

What you're doing here is rendering a zero-length dash segment as just
the caps, right? I do think I would like to see that land only with
zero-length paths rendered the same way. Otherwise, I don't think we
have good justification for the behavior. 

> Also, throughout the stroking code there are comments about drawing degenerate
> paths (ones with zero length) as much as possible e.g. patch 1/5. According to
> the PDF spec we do this wrong. Instead of drawing nothing always we should a
> circle if the line cap is round. I don't address this problem in the patchset.

Right, so I think this should be handled at the same time.

Also, path #5 adds a test case, but it doesn't add a reference
image. Ideally, I would like to see a commit sequence that first adds
a test case with a reference image (so it fails), and then adds a
change to the code that makes it pass. (This may require some
out-of-order committing since you need the final code in order to
generate the reference image in the first place---but it's still the
order I prefer to see the patches land in).

With a commit sequence like that, it's so much easier to accept the
correctness of a patch, (and understand what its intent is).

If you'd like to use a personal git tree for advertising that kind of
patch sequence, I've setup something that you should be able to push
to with:

	git+ssh://git.cairographics.org/~jrmuizel/cairo

I think I have that URL correct---I had been using the non-URL form
before, though it's definitely less desirable as it does not make the
protocol explicit. Something like:

	git.cairographics.org:~jrmuizel/cairo

This repository can also be seen on the web:

	http://gitweb.freedesktop.org/?p=users-jrmuizel-cairo;a=summary

Finally, I need to think a bit more about the logic in path #4. Exact
identity tests of floating-point values against 0.0 always raise some
mental suspicion on my part. It may be perfectly correct, but I'll
need a bit more convincing. (And the test case with reference image
will help here.)

> This series also improves the accuracy of the dashed lines. See
> http://people.freedesktop.org/~jrmuizel/dash-accuracy-before.png,
> http://people.freedesktop.org/~jrmuizel/dash-accuracy-after.png and
> http://people.freedesktop.org/~jrmuizel/dash-accuracy.c
> This is of course, however, not a realistic scenario but we can still have
> warm fuzzies.

I always like the results to be more accurate. So this is greatly
appreciated.

Thanks again,

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