Fixes #3945: Flickering when adding a new polygon entity

[roderickgreen]

This fixes the flickering when adding or modifying entities within a batch (#3945). Primitives are now created with show=false, then modified to show=true once the primitive is ready and the oldPrimitive is cleaned up.

Note that this same approach does not work for StaticGroundGeometryColorBatch. When creating a GroundPrimitive with show = false, the primitive never becomes "ready." Reading the update code in GroundPrimitive this should be pretty easy to see, but I'm not sure if it's expected. I can provide a trivial failing unit test for this if you like. I decided to leave StaticGroundGeometryColorBatch unchanged.

[cesium-concierge]

@roderickgreen, thanks for the pull request! Maintainers, we have a signed CLA from @roderickgreen, so you can review this at any time.

⚠️ I noticed that CHANGES.md has not been updated. If this change updates the public API in any way, fixes a bug, or makes any non-trivial update, please add a bullet point to CHANGES.md and comment on this pull request so we know it was updated. For more info, see the Pull Request Guidelines.

---

I am a bot who helps you make Cesium awesome! Contributions to my configuration are welcome.

🌍 🌎 🌏

[hpinkos]

Thanks for the contribution @roderickgreen! This approach seems good to me. We really need to figure out what do to for StaticGroundGeometyrColorBatch though because those are probably used more frequently when users add polygons. Maybe we can do something by waiting for the primitive.readyPromise @mramato do you have any ideas?

[roderickgreen]

Thanks for looking at this, @hpinkos. I'm really not sure what to do with StaticGroundGeometyrColorBatch. When asynchronously creating primitives, I think they have to be created with show : false or they will show up in the scene before we have a chance to remove the old primitive. That doesn't work at all with GroundPrimitive because of how it is implemented. There might be a solution if we create the GroundPrimitive with asynchronous : false, but that doesn't seem great. Or we can refactor GroundPrimitive so it can be created with show : false and shown when it is ready. Hopefully someone else has another suggestion.

FWIW, at first I was using readyPromise to show the new primitive and to lift the oldPrimitive cleanup out of update. I backed off of that because I thought it was higher risk because it might change the behavior in error paths. I'm happy to rebase my pull request to use this code instead if you like this approach more and don't think it will cause problems:

                primitive = new Primitive({
                    show : false,
                    asynchronous : true,
                    ...
                });

                var that = this;
                primitive.readyPromise
                    .then(function(primitive) {
                        primitive.show = true;
                    })
                    .always(function() {
                        if (defined(that.oldPrimitive)) {
                            that.primitives.remove(that.oldPrimitive);
                            that.oldPrimitive = undefined;
                        }
                    });

[mramato]

When creating a GroundPrimitive with show = false, the primitive never becomes "ready."

We should probably change GroundPrimitive to remove this restriction CC @bagnell

[roderickgreen]

This is blocking on #6428. After that is merged, the StaticGroundGeometryColorBatch tests will pass.

[hpinkos]

Thanks @roderickgreen! We should be able to review #6428 and this PR hopefully in the next few weeks. We're pretty busy working towards a May 1st milestone, so it just depends on our progress towards that goal.

[mramato]

@roderickgreen I just merged #6428, so if you can merge master into your branch and bump when ready, we'll try to get this into the May 1st release.

[roderickgreen]

Thanks, @mramato. I merged master and updated changes.md, so I think this is done.

[hpinkos]

Thanks @roderickgreen, this is great! I'll let @mramato take a final look but this should be good to go

[mramato]

Thanks @roderickgreen!