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

[Jeff Muizelaar]

On Mon, Apr 10, 2006 at 10:31:03AM -0700, Carl Worth wrote:
> 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.)

The accuracy improvements do not seem to have an effect on any of the
test suite images.


> 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? 

Actually, zero-length segments are rendered as just caps after patch #3.

> 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.

There is a fundamental difference between adding the caps to zero-length
segments vs. zero-length paths: zero-length paths have no slope whereas
zero-length segments do because they sit on a nonzero-length line.
Because of the this difference, dealing with zero-length paths is really
a different issue.

According to the pdf spec, zero-length paths are only drawn with round
caps because in with round caps the result is just a dot and the result
is the same regardless of the slope chosen.

> 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).

I have something like this sitting on the 'dashing' branch in my public
repository. I added the test case at the begining of the entire patchset.
The results will be meaningless up to patch #3. After patch #3 the
results are correct except for the issue addressed in patch #4.

> 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.)
> 

I'll try and explain with an example.

Suppose we have a dash array of [2.0, 4.0, 6.0] and a dash offset of 6.0.

In this case the dash start code will do the following:

	(6.0 >= 2.0) {
		offset -= 2.0;
		on = !on;
	}

	(4.0 >= 4.0) {
		offset -= 4.0;
		on = !on;
		dash_index = 2;
	}

	(0.0 >= 6.0) - false

so at this point dash_index = 2 and the on = true which means that we
are starting at the third segment with dashing on. This is correct
behaviour.

If however we have a dash array of [2.0, 4.0, 0.0] and the same dash
offset of 6.0 the current code will do the following:

	(6.0 >= 2.0) {
		offset -= 2.0;
		on = !on;
	}

	(4.0 >= 4.0) {
		offset -= 4.0;
		on = !on;
		dash_index = 2;
	}

	(0.0 >= 0.0) {
		offset -= 0.0;
		on = !on;
		dash_index = 3;
	}

	(0.0 >= 2.0) - false

so now instead of dash_index being 2 it is 3, on is false and we have
skipped the zero length segment.

What this means visually is that the with the first dash array the line
will start with a capped line at the begining. As the value of third
element in the array decreases the caps get closer and closer
together and one would expect that we would eventually just be left with
the caps.  However, with the current code when the 3rd element of the
array reaches 0.0 the cap disapears because it is skipped over.

I hope this explanation makes thing more clear. Also, as I was writting
this explanation, I realized that the check 'stroker->style->dash[i] ==
0.0' is actually redundant because of the test above it. I think that I
may have originally left this in for clarity, but that has obviously not
worked out. Overall, finding a clean and clear way to address this issue
has been a bit of a challenge and I'd welcome suggestions for ways to
improve it.

-Jeff