Fix polyline color issue involving duplicate positions

[j9liu]

Fixes #9379. This PR changes how colors are handled in PolylineGeometry.createGeometry:

• arrayRemoveDuplicates now accepts an optional removedIndices parameter. If any entries are removed from the original array during this function, the indices of those entries will be stored in removedIndices in sorted order.
• createGeometry will use this array to remove the colors that correspond to those extraneous positions. This should work for both segment coloring and per-vertex coloring.

Additionally, I noticed that if wrapAround were set to true in the parameters, and if an array's first and last entries were equal, arrayRemoveDuplicates would remove the first entry. However, the examples in the documentation suggested that the last entry was supposed to be removed instead. I changed the function to match the example.

---

Segment Coloring

This sandcastle was linked in the issue to demonstrate the bug. It sets up the following polyline:

In the current version of Cesium, if the orange segment is collapsed, the remaining segments will be colored incorrectly as shown.

The changes in PR result in this polyline:

---

Vertex Coloring

We can modify the sandcastle to have per-vertex coloring like so.

If we try to remove the black vertex by making it a duplicate, in the current version of Cesium, we'll find that it just shifts over to the other vertices:

However, we can remove it with these PR's changes:

cc @lilleyse for review

EDIT: now that I'm thinking about it, in the per-vertex example it probably makes more sense for the orange point to be removed rather than the middle black vertex, because that preserves the original coloring. I'll work a fix tomorrow

[cesium-concierge]

Thanks for the pull request @j9liu!

• ✔️ Signed CLA found.
• ❔ CHANGES.md was not updated.
  • If this change updates the public API in any way, please add a bullet point to CHANGES.md.

Reviewers, don't forget to make sure that:

• Cesium Viewer works.
• Works in 2D/CV.
• Works (or fails gracefully) in IE11.

[lilleyse]

Thanks for the detailed summary at the top of PR. It was good to see that you tested a bunch of different cases. And nice catch with the wrapAround bug.

EDIT: now that I'm thinking about it, in the per-vertex example it probably makes more sense for the orange point to be removed rather than the middle black vertex, because that preserves the original coloring. I'll work a fix tomorrow

Thank for pointing this out. I'll look out for your fix tomorrow.

[lilleyse]

These two separate sections of code that handle wrapAround can be combined. It should be fine to check the first and last element of values rather than cleanedValues, the answer ought to be the same.

Even nicer would be to combine all the code into a single for loop, but only if it can be done cleanly.

[j9liu]

Context for just-committed changes: I ran into a case today where if I have an array [A, B, C, ...., A, A, A], the function deletes the last two A entries, and then it will also remove the final duplicate A when it sees wrapAround is true. But it doesn't properly account for the index of that A with respect to the original array; its index is length - 3, but the code will only push values.length - 1. Even if I used cleanValues.length - 1, the indices will be out of order because it takes care of this case at the very end.

This commit should take care of this case, but the wrapAround step is a check that happens at the end of the loop. I'm not sure how to cleanly integrate it into the existing for loop. Will work on PolylineGeometry.createGeometry next.

[j9liu]

@lilleyse - Fix just pushed. Now the sandcastle should look like this.

I handled endpoint cases differently than cases where the point is in the middle of the polyline. If a polyline looks like [A, B, B, C] with the colors [red, green, blue, white], it arbitrarily the color of the point's first appearance, so the line's colors will become [red, green, white]. But when the point is on the end, then it makes more sense to have it drop the first-appearing color.

[j9liu]

@lilleyse - made the changes you suggested; let me know if there's anything else that needs adjustment!

[lilleyse]

Thanks, the updates look great!