Pixi.js: lineTo with a duplicate point renders a gap in a line drawing.

[XerxesNoble]

Hey!

I was working on some line drawing and came across this potential issue:

If you create a graphics object, and then start drawing a line.. when you add a duplicate point into to lineTo method, the renderer somehow seems to add a gap in the line. I am unintentionally adding the duplicate point, so I'm not sure how much of a 'bug' this is.. However, this is certainly "unexpected behaviour" in my eyes.

Here is a working example using the latest PIXI:

var app = new PIXI.Application(800, 600, { antialias: true });
document.body.appendChild(app.view);

var lines = [
  [
    [-8.154,558.886],
    [121.097,294.348],
    [133.744,282.209],
    [134.591,281.512],
    [166.995,250.542], // < --- Duplicate set of points
    [166.995,250.542], // NOTE: comment this out to close the line
    [167.029,250.518],
    [170.649,248.128],
    [172.666,246.822],
    [176.68,244.274],
    [178.455,243.17],
    [292.488,-14.857]
  ]
]

lines.forEach(function(line){
   var graphics = new PIXI.Graphics();
   graphics.lineStyle(4, 0xffd900, 1);
   line.forEach(function(point, i) {
      if (i === 0) graphics.moveTo(point[0], point[1]);
      else graphics.lineTo(point[0], point[1]);
   })
   graphics.endFill();
   app.stage.addChild(graphics);
})

[finscn]

Yes, I think it's a bug really.
similar to https://github.com/pixijs/pixi.js/issues/3592

[j-funk]

@XerxesNoble this has been affecting me, thanks for figuring it out!

[G-Rath]

This bug is affected me as well. I've been doing pathfinding with A*, and have a function for drawing the paths:

/**
 *
 * @param {Array<Array<number>>} path
 *
 * @return {PIXI.Graphics}
 */
drawPath(path) {
    const graphic = new PIXI.Graphics();

    graphic.lineStyle(1, Utils.hexColor('#ff999f'));

    const startingPoint = path[0];
    graphic.moveTo(startingPoint[0] + 1, startingPoint[1]);

    path.forEach(point => graphic.lineTo(...point));

    gameApp.stage.addChild(graphic);

    return graphic;
}

In order for the first node in the path to be drawn, I have to offset the initial node's x by one pixel when moving the graphic object, otherwise it won't draw it.

I had a quick look into the npm package (specifically Graphics.js), but found it was too complicated for me to try and whip up a quick fix without completely cloning the repo and diving into the inner workings of it all.

I did think that a suitable simple fix might be just to check if the lineTo point is the same as the current line's point, as well as check the last two points in the shape.points array. That would have the added benefit of optimizing the points being stored/drawn, since it would eliminate duplicates.

I have no idea the feasibility or impact of that solution, since when trying a basic implementation I quickly got tripped up by the fact that Graphics#moveTo is done by drawing a Polygon shape (it was around that time I realised it was going to be a rather large rabbit hole that I didn't have time for).

[finscn]

@ivanpopelyshev , Is it fixed in v5 ?

[ivanpopelyshev]

No, its not.

[gooderist]

Ignoring the call on duplicates, as @G-Rath suggested, seems to work even without digging up the moveTo polygon.

Here is an updated fiddle testing the change on this issue

and another fiddle showing the change working on #3592 despite not checking against the initial moveTo.

The change itself would look something like this

    lineTo(x, y)
    {
        const points = this.currentPath.shape.points;
        const prevY = points[points.length - 1];
        const prevX = points[points.length - 2];

        // Skip if x,y matches last lineTo coordinates
        if (prevX !== x || prevY !== y)
        {
            this.currentPath.shape.points.push(x, y);
            this.dirty++;
        }

        return this;
    }

Have I overlooked anything?

[XerxesNoble]

@gooderist It fixes my above issue when overwriting the prototype with your code.

var app = new PIXI.Application(800, 600, { antialias: true });
document.body.appendChild(app.view);

var lines = [
  [
    [-8.154,558.886],
    [121.097,294.348],
    [133.744,282.209],
    [134.591,281.512],
    [166.995,250.542], // < --- Duplicate set of points
    [166.995,250.542], // NOTE: comment this out to close the line
    [167.029,250.518],
    [170.649,248.128],
    [172.666,246.822],
    [176.68,244.274],
    [178.455,243.17],
    [292.488,-14.857]
  ]
]

PIXI.Graphics.prototype.lineTo = function(x, y) {
  const points = this.currentPath.shape.points;
  const prevY = points[points.length - 1];
  const prevX = points[points.length - 2];

  // Skip if x,y matches last lineTo coordinates
  if (prevX !== x || prevY !== y)
  {
    this.currentPath.shape.points.push(x, y);
    this.dirty++;
  }

  return this;
}

lines.forEach(function(line){
   var graphics = new PIXI.Graphics();
   graphics.lineStyle(4, 0xffd900, 1);
   line.forEach(function(point, i) {
      if (i === 0) graphics.moveTo(point[0], point[1]);
      else graphics.lineTo(point[0], point[1]);
   })
   graphics.endFill();
   app.stage.addChild(graphics);
})

[XerxesNoble]

I will say that I fixed my issue by just cleaning the list of points prior to sending it to the lineTo function. Having two of the exact same point render one after another isn't valuable and just leads to more accidental iteration.

I ran some tests and found that having 300000 duplicate points sequentially render is reduced from 150ms to about 50ms using your lineTo implementation.

So I'd say it's definitely a viable option 👍

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[G-Rath]

@bigtimebuddy is the Won't Fix label correct for this issue?

My understanding is #5084 fixes this - It's not a big deal, but I'd be worried the label could put off anyone who finds this issue in the future :)

[themoonrat]

I've removed that label; it's this stale bot that's going through years worth of old pr's and issue and cleaning them up for us. A few things like this are getting labelled as won't fix when the reality is they're already been fixed :) Cheers for letting us know

[G-Rath]

That's what I thought - I've had a look at the other issues, and ya they've all got that label, which could be a bit of a problem 😕

[lock[bot]]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.