ofPath adds an initial straight line segment when creating curves in COMMAND mode

[mikewesthad]

ofPath defaults to adding a straight line vertex when creating curves in COMMAND mode.

The addCommand(...) method of ofPath defaults to adding a moveTo command if the commands std::vector is empty. moveTo creates a new polyline in the path and adds a straight line vertex via addVertex(...). Here's a link to the relevant line of code.

This has caused problems for a few people (forum post) who are using ofPath's curveTo(...) method. Users could switch to creating paths in POLYLINE mode, but that undercuts some of the power of using paths.

For example, if you run this code:

ofPath path;
std::vector<ofVec2f> points;

void ofApp::setup(){
    points.push_back(ofVec2f(100, 50));
    points.push_back(ofVec2f(200, 250));
    points.push_back(ofVec2f(300, 50));
    points.push_back(ofVec2f(400, 250));
    points.push_back(ofVec2f(500, 50));
    points.push_back(ofVec2f(600, 250));
    points.push_back(ofVec2f(700, 50));
    for(int i=0; i<points.size(); i++) {
        path.curveTo(points[i].x, points[i].y, 0.0);
    }
    path.setFilled(false);
}

void ofApp::draw(){
    ofBackground(0);
    ofSetColor(255);
    path.draw();
}

You get:

I'm a bit confused as to why ofPath is set up to default to using moveTo to add a straight line vertex. If I comment out this line from ofPath, the above code works as expected (and it also works as expected for straight line paths):

Is removing that line of code an appropriate fix, or is there some more hidden reason why that addVertex(...) command exists?

(I'm running Windows 7 (64) with oF 0.8.0 in Code::Blocks 12.11, but this bug is not IDE/OS specific and not fixed in the master branch.)

[arturoc]

yes if you don't have that and try to draw a line using moveTo, lineTo the line would be drawn from 0.0 to the point specified in lineTo

[mikewesthad]

@arturoc hm, I'm a bit confused.

It was a bit short-sighted of me to suggest commenting out that line as a fix. It would be problematic for anyone using moveTo to define their first vertex (of a subpath). Though commenting it out and running moveTo followed by lineTo as you suggest does not draw a line from (0, 0) to the point specified. It just add a single vertex at the point specified. Did I misunderstand what you were saying?

What I should have suggested as a fix was altering the addCommand(...) method of ofPath. I'm not sure why the first command in every subpath is forced to be a moveTo command, though I assume there is a reason.

[arturoc]

On Fri, 2014-04-11 at 13:21 -0700, Michael Hadley wrote:

@arturoc hm, I'm a bit confused.

I do see how commenting out that line would be problematic for users
who are using moveTo to define their first vertex (of a subpath).
Though commenting it out and running moveTo followed by lineTo as you
suggest does not draw a line from (0, 0) to the point specified. It
just add a single vertex at the point specified. Did I misunderstand
what you were saying?

no that's right, but in any case it'll break the implementation, if you
do:

moveTo(20,20)
lineTo(40,40)

you should get a line from 20,20 to 40,40, if we remove the extra vertex
on moveTo you'll only get one on 40,40

What I should have suggested as a fix was altering the addCommand(...)
method of ofPath. I'm not sure why the first command in every subpath
is forced to be a moveTo command, though I assume there is a reason.

mmh yeah that seems like a bug, even the logic of the conditional
doesn't seem to make much sense, if((commands.empty() || commands.back().type==Command::close) then it's impossible that
command.type==Command::moveTo. the first implementation of ofPath used
subPaths instead of commands, i changed it at some point to simplify it
and fix some things and this seems like a left over from the old
implementation that wasn't correctly ported, if you want to take a look?

[mikewesthad]

Hmm, the logic of the line seems purposeful to me:

If [(there are no existing commands) or (the last existing command closed a subpath)] and (the current command is anything other than moveTo), then add a moveTo command.

The input command could be a moveTo command when commands is empty or when the last command is a close because the conditional is being executed before command is pushed to the commands vector. Right?

I can spend some time and dig deeper into the source to see if I can find a reason why the default would be to add a moveTo command at the beginning of every subpath. If I can't find find a reason then I can submit a PR that removes that requirement from the addCommand method.