Fix material batch entities from disappearing

[lilleyse]

Fixes #4096

createPrimitive was always being incorrectly set to false whenever an entity not in the batch tried to be removed from the batch, causing the entity actually in the batch to never be created. The code in StaticGeometryColorBatch was correct, so I just copied it from there to StaticGeometryPerMaterialBatch.

[mramato]

Wow, this might fix a bunch of other random issues we've seen in the past as well. Thanks for tracking this down. I'll look at it closely tonight.

[kring]

Might be worth a moment's consideration of #3807. I don't understand the code well enough to say for sure, but I think #3807 is caused by incorrect batch updating logic in StaticGeometryColorBatch.

[pjcozzi]

@lilleyse can you update CHANGES.md? Is it also possible to add a reasonable unit test?

@mramato do you want to review before we merge this?

[mramato]

Yes, I'll look at it today.

[mramato]

Might be worth a moment's consideration of #3807. I don't understand the code well enough to say for sure, but I think #3807 is caused by incorrect batch updating logic in StaticGeometryColorBatch.

I actually think @tfili fixed that as part of his entity clamping work and we just forgot to close the issue.

[mramato]

Looks good, thanks again @lilleyse!

[kring]

I actually think @tfili fixed that as part of his entity clamping work and we just forgot to close the issue.

Yep, confirmed. Thanks guys!

[lilleyse]

@lilleyse can you update CHANGES.md? Is it also possible to add a reasonable unit test?

I'll update CHANGES.md in master. I wrote a unit test but I can't seem to replicate (or am just not properly testing) the failing behavior, so it may not be worth adding.

[lilleyse]

Just putting it here for reference.

it('StaticGeometryPerMaterialBatch and StaticGeometryColorBatch update simultaneously', function() {
    var entityCollection = new EntityCollection();
    var visualizer = new GeometryVisualizer(EllipseGeometryUpdater, scene, entityCollection);

    var ellipse = new EllipseGraphics();
    ellipse.semiMajorAxis = new ConstantProperty(2);
    ellipse.semiMinorAxis = new ConstantProperty(1);
    ellipse.material = Color.RED;
    ellipse.height = 100;

    var entity = new Entity();
    entity.position = new Cartesian3(1234, 5678, 9101112);
    entity.ellipse = ellipse;
    entityCollection.add(entity);

    var ellipse2 = new EllipseGraphics();
    ellipse2.semiMajorAxis = new ConstantProperty(3);
    ellipse2.semiMinorAxis = new ConstantProperty(2);
    ellipse2.material = new GridMaterialProperty();
    ellipse.height = 100;

    var entity2 = new Entity();
    entity2.position = new Cartesian3(1234, 5679, 9101112);
    entity2.ellipse = ellipse2;
    entityCollection.add(entity2);

    return pollToPromise(function() {
        scene.initializeFrame();
        scene.render(time);
        return visualizer.update(time);
    }).then(function() {
        entity.material = Color.BLUE;
        entity2.material = new CheckerboardMaterialProperty();
        expect(scene.primitives.length).toBe(2);
        return pollToPromise(function() {
            scene.initializeFrame();
            scene.render(time);
            return visualizer.update(time);
        }).then(function() {
            expect(scene.primitives.length).toBe(2);
            visualizer.destroy();
        });
    });
});