closePaths adds an extra lineTo

[typemytype]

import pathops
from fontPens.printPen import PrintPen

path = pathops.Path() 

pen = path.getPen()
pen.moveTo((100, 100))
pen.lineTo((100, 200))
pen.closePath()

path.draw(PrintPen())

results in:

pen.moveTo((100.0, 100.0))
pen.lineTo((100.0, 200.0))
pen.lineTo((100.0, 100.0))
pen.closePath()

[anthrotype]

I’ll check thanks. Note that this is super WIP as I’m still in exploration stage.

[anthrotype]

And I really appreciate you guys keeping a close eye on it ;)

[anthrotype]

I see it is redundant but it doesn’t actually change the semantics. I’ll remove the unnecessary lineTo because it’s implied in both Skia and FontTools

[typemytype]

I know the cutting edge if this thing :) and Im just posting things that I encounter that are different then what I would expect

maybe more important to be able to test is to open the builder so we can actually test if skia does it better, faster and can be tested against all the use cases Ive encountered, even the one's that made Tal and me decide to build our own thing instead of using FDK checkoutlines, I cannot share that data as its a collection from different foundries with old and unpublished work.

[anthrotype]

Yes. I’ll be working on that in the afternoon. Stay posted

[typemytype]

the reason why this matter is that you can send that triangle to skia and whenever there is no change in the outline (by any operation) I would expect to have the same triangle back.

thanks a lot!!

[anthrotype]

actually this looks like the default behavior of SkPath::close() method to always add a lineTo "verb" when the last lineTo is not already on the same point of the starting point. You can see by calling path.dump() (which prints to standard output a text representation of the C++ object) after you have called closedPah on the pen (which simply forwards to the SkPath::close()).

>>> path = Path()
>>> pen = path.getPen()
>>> pen.moveTo((100, 100))
>>> pen.lineTo((100, 200))
>>> pen.closePath()
>>> path.dump()
path.setFillType(SkPath::kWinding_FillType);
path.moveTo(100, 100);
path.lineTo(100, 200);
path.lineTo(100, 100);
path.close();

I think this is a non-issue, because both segment-wise and point-wise the two contours (with or without the last implied lineTo) are identical. To prevent that extra lineTo being called on the pen inside the Path.draw method, I would have to delay forwarding any incoming lineTo verbs from the SkPath::Iter::next until I'm sure that the following verb is not a close verb. Sure it could be done, but maybe at a later stage.

[typemytype]

thats fine

[khaledhosny]

I think at this point we should focus on speed and font building pipeline (I’m personal OK with this module not being suitable for interactive editors). Has anyone benchmarked this yet?

[adrientetar]

Cosimo told me about 10x faster, I think.

[typemytype]

@khaledhosny speed is not the holy grail

This example shows an unmodified union where the output has a different starting point, different contour direction and an extra point is added...

import pathops
from fontPens.printPen import PrintPen
from defcon import Glyph

g = Glyph()
pen = g.getPen()

pen.moveTo((100, 100))
pen.lineTo((100, 200))
pen.lineTo((200, 200))
pen.lineTo((200, 100))
pen.closePath()

print "source:"
g.draw(PrintPen())
print "union result:"
pathops.union(g, PrintPen())

This is important and not only for editors. Its what you expect to when doing an operation, nevertheless is at the end of a production chain or in the middle of a design process.

Im just adding those issues as those where issues BooleanOperation solved along the way...

[khaledhosny]

For this particular project, speed is the most important thing (after correctness, of course). My personal motivation is that building one of my font families takes currently ~15 minutes and its is very counter-productive for me, so whatever percent of time we can squeeze out of this counts IMO.

I don’t think different start point is an issue, neither is the extra lineTo (it is implied already AFAIK), but changed contour direction is indeed an issue.

I appreciate these issues and I’m not saying we shouldn’t look into them, I’m just stating my personal priorities.

[adrientetar]

For the two remaining issues I think it's because the result we get has evenodd fill (and not winding) which might be a bug in skia since it seems the code tries to restore the original fill type. I asked the question, let's wait for replies: https://groups.google.com/forum/#!topic/skia-discuss/9xieDIhMoxk