Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Polygon entity blinking when updating its color on mouseover #8365

Closed
jeremiebailleul opened this issue Nov 6, 2019 · 9 comments
Closed

Comments

@jeremiebailleul
Copy link

jeremiebailleul commented Nov 6, 2019

Here is a sandcastle of my problem.

Expected behaviour: I have three groups of polygons, and I want to add each group to the viewer as a CustomDataSource, and instanciate each polygon as entities in these dataSources. When hovering a polygon, I want to change the polygon color and reset the color of the previous hovered polygon (if there was one).

Actual behaviour: In the provided example, as mouse moves inside the polygon on the right side, it starts flickering, while it works fine on other polygons. I then noticed that if I add another polygon entity in the same dataSource (uncomment line 15 to do so), it doesn't bug anymore. It seems like it bugs out only when having a single entity in the dataSource. I've logged the pickedObject in the console and when moving the mouse over the "bugged" polygon, undefined value is sometimes returned by viewer.scene.pick method instead of the expected entity.
Have I messed something up in this code ?

Browser: Tested on Chrome/Firefox
Operating System: Tested on Linux/Windows

Thank you in advance for your help ;)

NB: In case the following information is relevant : I've seen posts suggesting that color changing on hover events like this example can be done using a CallbackProperty, but I can not do it this way because I've got a use case where I have up to 2320 polygons and the scene won't even load when instanciating each entity with a CallbackProperty for the color.

@OmarShehata
Copy link
Contributor

The picking is actually working correctly - the problem is that when you update an entity, this update happens asynchronously. When the entity disappears for a few frames, the picking result becomes undefined (because there's nothing rendered).

Ideally you could just say "I want this entity to be update synchronously" for cases when it won't update every frame, but when it does update it'll be on user interaction and you'd want it to be synchronous. You can read a bit more on this here: #7934 (comment)

@jeremiebailleul
Copy link
Author

Hello @OmarShehata and thank you for your answer.
My bad for incriminating the picking, I misunderstood the problem !

I've read #7934 and I understand the following available solutions:

  • using a CallbackProperty to make the entity dynamic but it would result in poor perfs for my use case of thousands of polygons
  • or use the primitives directly with geometryInstances as said in this comment

Nevertheless, I can't help wondering how come there isn't any blinking when I have several entities in the EntityCollection of the CustomDataSource, but there is blinking only when having a single polygon entity.

See in this simpler sandcastle how it blinks on hover when I declared only one array of coordinates to be translated as an entity, but if you uncomment line 5 and add another polygon with [0,0] coordinates, it doesn't blink anymore.

GIF

@jeremiebailleul jeremiebailleul changed the title Picking is buggy when having a single polygon entity in a viewer's datasource Polygon entity blinking when updating its color on mouseover Nov 13, 2019
@jeremiebailleul
Copy link
Author

I edited the title so it matches the actual problem and so it doesn't incriminate the picking !

If you have any hint on why this behaviour is happening when the entity is declared alone, but not happening if I declare several entities in the dataSource ?

Maybe some collection is dropped in first case when removing/recreating the entity and not in the second case or something like that.

@OmarShehata
Copy link
Contributor

@jeremiebailleul I'm closing this since the blinking is expected behavior as explained in my earlier comment. The fact it doesn't happen in all situations is a good question, but I think what would really solve this problem for you is being able to control when entities are sync/async, which is the feature request here: #5127

Feel free to 👍 that to express your interest in seeing that feature in the engine.

@Akbalder
Copy link

I think that I found why the blinking happen only on some situations.

When we are updating a polygon, we are removing it from the batch before inserting it again.

GeometryVisualizer.js:

            //If in a single update, an entity gets removed and a new instance
            //re-added with the same id, the updater no longer tracks the
            //correct entity, we need to both remove the old one and
            //add the new one, which is done by pushing the entity
            //onto the removed/added lists.
            if (updaterSet.entity === entity) {
                updaterSet.forEach(function(updater) {
                    that._removeUpdater(updater);
                    that._insertUpdaterIntoBatch(time, updater);
                });
            } else {

If the polygon is alone in a batch, the batch is destroyed when removing the updater. The function _insertUpdaterIntoBatch will have to create a new batch.

StaticGroundGeometryPerMaterialBatch.js:

    StaticGroundGeometryPerMaterialBatch.prototype.remove = function(updater) {
        var items = this._items;
        var length = items.length;
        for (var i = length - 1; i >= 0; i--) {
            var item = items[i];
            if (item.remove(updater)) {
                if (item.updaters.length === 0) {
                    items.splice(i, 1);
                    item.destroy();
                }
                break;
            }
        }
    };

Not destroying the batch when are updating an entity seems to solve the issue.
The color update is still a bit slow but the polygon doesn't disappear before being displayed with its new color.

@OmarShehata
Copy link
Contributor

Here is the recommended way of changing a polygon's color on mouse over that avoids any flickering. You need to use a ColorMaterialProperty, that has a CallbackProperty for its color value.

This will not mark the entity as dynamic, so it should be efficient to have this on thousands of entities. This will simply change the color attribute of the underlying primitive. Here's a version of this implemented in the primitive API to make it clear what's happening under the head. I'd be happy to review a new Sandcastle example that adds this to the list of examples since it's not obvious this is the right way to do it.

@Akbalder
Copy link

Thank you for your help.

The solution using entities and a CallbackProperty is too slow to be used when there are 100 polygons. We were able to display a lot more entities without the CallbackProperty.

The solution with the primitive API seems to work fast enough for about 2000 polygons.

@mramato
Copy link
Contributor

mramato commented Feb 25, 2020

@OmarShehata FYI, the primitive/Entity versions you link to above are not doing the same thing. The primitive one does not confirm the Polygon to terrain, while the Entity one does. You need to use GroundPrimitive to have the polygon on terrain. That is most likely the main reason for the performance difference. I'm pretty sure there is also a performance bug in the Entity API that I'm looking into as well but may be something different.

@mramato
Copy link
Contributor

mramato commented Feb 25, 2020

Just opened #8630 which greatly improves performance in the Entity API for this use case and should scale to thousands of polygons.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants