Label clustering of empty strings

[hpinkos]

Reported on the forum: https://groups.google.com/forum/#!topic/cesium-dev/9jEuuGdzmbU

In 1.28, labels cluster when they were provided with an initial text value of an empty string

http://cesiumjs.org/releases/1.28/Apps/Sandcastle/?src=Hello%20World.html&label=Showcases&gist=7f5e646122494eac4a36396cb3582c96

In 1.29, labels provided with an initial text value of an empty string do not cluster.
http://cesiumjs.org/releases/1.29/Apps/Sandcastle/?src=Hello%20World.html&label=Showcases&gist=6e32b2a41f58c8e493964342be7f066e

In 1.29, nothing appears on the globe

var viewer = new Cesium.Viewer('cesiumContainer');
var ds = new Cesium.CustomDataSource();

ds.clustering.enabled = true;

viewer.dataSources.add(ds);

var ppa = ds.entities.add({
    position : Cesium.Cartesian3.fromDegrees(-75.1641667, 39.9522222),
    label : {
        text : '' //'Philadelphia'
    }
});

var htx = ds.entities.add({
    position : Cesium.Cartesian3.fromDegrees(-95.36978, 29.76045),
    label : {
        text : '' //'Houston'
    }
});

var hin = ds.entities.add({
    position : Cesium.Cartesian3.fromDegrees(-87.25414, 41.53085),
    label : {
        text : '' //'Hobart'
    }
});

[billwritescode]

I did a little bit of thumbing through some code changes and this commit stood out as a candidate for investigation: 3eb09e4

[pjcozzi]

Thanks for narrowing this down, @billwritescode.

@emackey do you have any bandwidth to look at this?

[emackey]

Yeah I should look at this.

@billwritescode you said you did a bit of thumbing, did you actually run git bisect on this? I should be able to find some time for that in the next few days, but if you want to beat me to it that would be helpful.

[emackey]

Also, a bit of a confession since I got pulled off a project recently and didn't report this yet, but label un-clustering seems completely broken for the past few releases. Check out the clustering demo in Sandcastle, you can zoom in and un-cluster the icons but you never see any un-clustered labels when clustering is enabled overall. I was planning an issue and a proposed fix and then got pulled away.

[emackey]

Also, silly question but, why would a user want to see clusters of empty labels? If they zoom in on the cluster of empty labels, wouldn't it just disappear anyway?

[billwritescode]

@emackey thanks for looking into this! Running git bisect points to 10045f6 as the first bad commit.

As for the question "Why would anyone want this?", one possibility is to jerry-rig clustering of polygons using clusterable empty labels, distance display conditions on some polygons, and toggling clustering on the datasource at the appropriate moments.

[emackey]

Thanks for the bisect. Part of the logic that was in the process of changing there was better calculation of glyph placement, such as not assuming 0, 0 was part of the glyph. For example a minus sign floats well above 0, 0 but an underscore may appear only below it. So if you ask for the screen-space dimensions of an empty label, you can no longer respond by saying width 0 height 0 at a 0, 0 point on the baseline, because the baseline is no longer a guaranteed part of the glyph's screen space. Empty labels have undefined screen space now, and that makes them hard to cluster.

Of course this could all be worked around by hard-coding a 0x0 at 0, 0 response for empty labels, but I'm circling back to the question of why. It seems odd for a typical Cesium user using this the typical way to want the screen space of something that isn't on the screen.

Maybe you can work around it with something invisible that does have screen space, such as transparent points:

http://cesiumjs.org/releases/1.29/Apps/Sandcastle/?src=Hello%20World.html&label=Showcases&gist=5b903d4064dfd18c1efa3d184cfd20b5

Transparent points work because although invisible, they have known screen space, a 1x1 pixel directly on the anchor position. Contrast this with a label that contains a single minus sign, the screen space of that label is a small rectangle, several pixels above the anchor position, not including the anchor position. If you get rid of the minus sign, the screen space becomes completely undefined. That's why I think points would be better for this.

[emackey]

@billwritescode Any thoughts on all this? I'd like to close this issue before the Feb 1 release.

[billwritescode]

@emackey, thanks for the high level technical description. I would think that not breaking user space is fairly important in this case, perhaps more important than the temporary ugliness of a hardcoded return value.

There is a workaround until the 1.28 behavior is restored like this: http://cesiumjs.org/releases/1.29/Apps/Sandcastle/index.html?src=Hello%20World.html&label=Showcases&gist=1d4b61c2c17a82ff6a7bf4882cc1d2ca

The real issue that this illustrates is that clustering for polygons is really needed. Until that is implemented, preserving the behavior of 1.28 is I would think the best approach in order to preserve user space. Of course, once clustering for polygons is implemented, that hotfix could be pulled back.

[emackey]

@billwritescode I think we missed the window for the 1.30 release. Does my workaround solve this issue for you?

http://cesiumjs.org/releases/1.29/Apps/Sandcastle/index.html?src=Hello%20World.html&label=Showcases&gist=5b903d4064dfd18c1efa3d184cfd20b5

[billwritescode]

Hey @emackey, thanks for looking into this. More important than hitting the 1.30 release is being sure we have a stable way of keeping this sort of behavior. Reworking the current code to use points is not that exciting, as you might imagine.

But in the same vein as transparent points, I think forcing labels to be transparent is promising.

var viewer = new Cesium.Viewer('cesiumContainer')
var ds = new Cesium.CustomDataSource()

ds.clustering.enabled = true

viewer.dataSources.add(ds)

var ppa = ds.entities.add({
    position : Cesium.Cartesian3.fromDegrees(-75.1641667, 39.9522222),
    label : {
        text : 'Philadelphia',
        showBackground: true,
        fillColor: Cesium.Color.TRANSPARENT,
        backgroundColor: Cesium.Color.TRANSPARENT
    }
})

var hin = ds.entities.add({
    position : Cesium.Cartesian3.fromDegrees(-87.25414, 41.53085),
    label : {
        text : 'Hobart',
        showBackground: true,
        fillColor: Cesium.Color.TRANSPARENT,
        backgroundColor: Cesium.Color.TRANSPARENT
    }
})

http://cesiumjs.org/releases/1.30/Apps/Sandcastle/?src=Hello%20World.html&label=Showcases&gist=1d4b61c2c17a82ff6a7bf4882cc1d2ca

Before closing this issue, could you comment on these two question?

1. Are there any cases in which the above text would show from these labels? I'm kind of suspecting that barring the API consumer changing the showBackground, fillColor, or backgroundColor, values, that the answer is "No" here ...
2. Can we rely on this behavior to be stable in upcoming releases? This is more important than reverting to the 1.28 API, is making sure there is a stable way to do this.

Thanks for all your work on this issue!

[billwritescode]

Hey @emackey, just bumping this thread -- any comments my two questions above?

[emackey]

Hi @billwritescode, I got pulled onto other projects, but this is still at the back of my mind.

1. No, with Cesium.Color.TRANSPARENT selected, the labels should not show at all. Having actual text here (instead of an empty string) is helpful in that it gives the label a definite screen location, similar to the transparent point. So, it seems like it should be fairly future-proof.

But the clustering logic still has lurking bugs, for example the Clustering Sandcastle demo never shows any labels at all when clustering is enabled. The code has typos or bad logic in it that prevents it. In my mind, this is a far larger bug than what's been reported here, and I'm surprised this one is getting all the attention.

2. Some logic will eventually need to change to fix the bug above. I don't know what that fix will entail exactly, so I can't promise anything, but it stands to reason that a label with text set to TRANSPARENT should probably still work for this case.

[bjbrewster]

Hello,

I recently upgraded Cesium from v1.26 (3rd Oct 2016) to the current v1.34 and have noticed a similar problem. Our clustered Site entities (a point and a label) no longer show their label. If I disable dataSource.clustering.enabled, then the labels appear ok. If I disable dataSource.clustering.clusterLabels, then the labels appear but they do not disappear when the points are clustered. I notice the EntityCluster.clusterLabels/Points/Billboards options were added in October last year after v1.26 that we were previously using and I believe it is related to this change.

I have confirmed this in the current Cesium Sandbox by modifying the Clustering Example to create a CustomDataSource with clustering enabled and 3 entities with both a point and a label:
https://cesiumjs.org/Cesium/Apps/Sandcastle/?src=Hello%20World.html&label=Showcases&gist=d72c50fefffdfad265f41fe075c9851d

The example will zoom to the 3 Site entities that are missing labels. Disable clustering using checkbox widget to see the labels appear. If you uncomment dataSource.clustering.clusterLabels = false you will see the labels appear but they do not disappear with the rest of the clustered entity.

Also in this buggy example, if you uncomment the block at the end to modify the entity.label.text after creation that you can normally do, the label text will partially show - only new characters beyond the length of the original text or where there were spaces in the original text will show. Eg original text 'foo bar' updated to '1234567890' will show ' 4 890' on screen ?!? The partial text does not disappear when the points are clustered though. Is this related to others saying you had to create a label with empty text first and then update?

I would also like to add that if you also have a billboard as part of the entity, then it gets clustered as though its separate to the point. I often use billboards for icons with a point as the background circle but have to set the billboard eyeOffset to e.g. new Cartesian3(0, 0, -5) to bring it in front due to z testing.

Thanks,
Brendan

[hpinkos]

Thanks for the details @bjbrewster! This issue is labeled priority so hopefully we'll get a chance to look into the problem soon

[pjcozzi]

@bjbrewster thanks for all the details, if you have the time, it would be great if you could use git bisect to track down the commit that causes this, and perhaps even propose a fix.

[bjbrewster]

@pjcozzi I can do. Can I confirm the expected behavior that when you have an entity with both label and point/billboard then all the parts should disappear and not be clustered separately? It's a bit ambiguous how the clusterBillboards/Points/Labels options affect the behavior. Does setting EntityCluster.clusterLabels leave the label part unclustered and visible on the screen and only the Point/Billboard parts be hidden when clustered?

[pjcozzi]

@bjbrewster I didn't work on this but that sounds reasonable. @bagnell?

[emackey]

There are a pair of unrelated issues being discussed on the same thread here.

1. The original issue reported here was that Labels don't cluster when their text value is an empty string. The behavior of empty strings changed as a result of the label system getting smarter about glyph positions: An empty string has no defined screen coordinates, because there are no glyphs to measure, and as a consequence it can no longer participates in the cluster (as of 1.29). This is not a bug, and users should not expect empty strings to have screen locations and clustering, because glyphs need not overlap their own anchor points. There is a workaround: Use transparent text of a non-empty string. This works because even though the text is invisible, it has well-defined screen coordinates, and can be clustered.

2. The Label Clustering demo has been broken for a very long time. If you zoom in to where the individual entities de-cluster, you will never see any labels while clustering is enabled. This is a serious bug that has flown under the radar for too long. I suspect it is caused by bad logic in EntityCluster, in particular the calculation of labelIndex is sometimes wrong. This needs more investigation.

[emackey]

Here's a loop of toggling the Enabled checkbox while zoomed in. The expected behavior is, the non-clustered entities (such as "Echostar Gilbert Station") should not hide their labels when clustering is enabled. I believe this is due to the bad logic mentioned in bullet 2 of my comment above.

[emackey]

Further discussion of bullet 2 (non-empty-string label clustering) has moved to #6087.

The original issue here (empty-string label clustering) has been thoroughly explained and analyzed above, so closing this.

[cesium-concierge]

Congratulations on closing the issue! I found these Cesium forum links in the comments above:

https://groups.google.com/forum/#!topic/cesium-dev/9jEuuGdzmbU

If this issue affects any of these threads, please post a comment like the following:

The issue at #4919 has just been closed and may resolve your issue. Look for the change in the next stable release of Cesium or get it now in the master branch on GitHub https://github.com/AnalyticalGraphicsInc/cesium.

---

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

🌍 🌎 🌏