calling ofPath::moveTo() results in ofSubPath::Command::lineTo command

[bgstaal]

This is an issue I found when working on the ofxShivaVG renderer:

moveTo() commands internally adds a new ofSubPath instance with ofSubPath::Command::lineTo as the first command. It is pretty clear from the internals of the moveTo method:

void ofPath::moveTo(const ofPoint & p){
    if(mode==PATHS){
        if(lastPath().size()>0) newSubPath();
        lastPath().addCommand(ofSubPath::Command(ofSubPath::Command::lineTo,p));
        hasChanged = true;
    }else{
        if(lastPolyline().size()>0) newSubPath();
        lastPolyline().addVertex(p);
        bNeedsTessellation = true;
    }
}

First of all, I think this is a design flaw. A lineTo command is conceptually not the same as a moveTo command, even though it results in the same vertex in the default renderer. (In the case were I was implementing the ofxShivaVG renderer I was forced to check if the first command was a lineTo command and then react to that like a moveTo command to get the expected behavior)

Secondly, it actually creates some bugs in the default renderer as well.

See this example:

myPath.moveTo(100, 300);
myPath.curveTo(300, 100);
myPath.curveTo(500, 300);
myPath.curveTo(700, 100);
myPath.curveTo(900, 300);

result:

clearly the first moveTo command is interpreted as a lineTo.

In this case it can be worked around by switching to a curveTo() call, but in the case where I would like to add another curved path, disconnected from the first one I'm getting the same problem over again:

myPath.curveTo(100, 300);
myPath.curveTo(300, 100);
myPath.curveTo(500, 300);
myPath.curveTo(700, 100);
myPath.curveTo(900, 300);

myPath.moveTo(100, 500);
myPath.curveTo(300, 300);
myPath.curveTo(500, 500);
myPath.curveTo(700, 300);
myPath.curveTo(900, 500);

result:

I can come up with several other examples as well (in the cairo renderer also) but I think this illustrates the issue well enough.

My proposed solution is as follows:
When moveTo() is called, add a new subPath as before, and then either:
A. Add an ofSubPath::Command::moveTo command as the subPath's first command.
or
B. Add the supplied point as "origin" on the ofSubPath.

(I would go for A as one would expect that all drawing related operations are represented as commands)

I'm more than happy to give a go at fixing this myself and then submit a pull request, but I would like to hear your thoughts first. Am I missing some obvious reason why it is implemented like this today?

In the process I would also like to see if I can fix #1575 as well. Would you accept a pull request for the whole shebang or should I split it up?

[kylemcdonald]

calling @DamianNZ

there wasn't much activity on damian's issue because i think not enough people are testing all the edge cases of ofPath.

but if you guys are both familiar with these bugs, and you're in agreement about the right way to fix them, i think everyone else would probably be happy to accept a PR fixing them :)

[arturoc]

this should be fixed in this PR #1959 where there's a new Move command for ofPath and there's no subpaths anymore. i did that refactoring while working with nvidia path rendering which is very similar in syntax to open VG so it should work for this too

[bilderbuchi]

Is something holding up the merge of #1959?

[bgstaal]

@arturoc yeah, I saw that PR when searching for related issues. Since it has not been merged I thought maybe it was still in an experimental state. Is it possible to at least merge the ofPath refactoring? I wan't to keep working on my renderer & I would prefer working against a fixed version of oF in stead of adding workarounds for the current issues.

[arturoc]

ok, it's merged, let me know if you still have any issues so we can close this one too

[bgstaal]

Sorry for the late reply on this one. Staying busy these days.

I'm happy to see that the the path/subPath system has been refactored to support moveTo commands. That makes my work on the openVG-renderer a bit easier. It does not fix the issues related to the screenshot's I posted though. I realize now that those issues are more related to how the curveTo command is implemented (in chunks of four and four without regards to moveTo and lineTo commands). Witch kinda makes this a duplicate of #1575 now.

We can either close this one now and I'll add a PR when I've had the time to have a look at fixing #1575 (probably not until next week as I'm working towards a deadline on monday). Or we can wait with closing this until that's done, since parts of the issues I have described here isn't really fixed yet.

[bilderbuchi]

I'll close this as a duplicate, then.
Don't worry, a reference to this issue is already over in #1575, so it will be easy to find any reference dicussion/screenshots. Happy to take your PR for it. :-) Maybe head over to #1575 before, though, to make sure the proper way to solve it is clear/approved.