CartoDB / mobile-sdk

CARTO Mobile SDK core project
https://carto.com/docs/carto-engine/mobile-sdk/
BSD 3-Clause "New" or "Revised" License
179 stars 65 forks source link

font size based ordering not working #502

Closed farfromrefug closed 2 years ago

farfromrefug commented 2 years ago

We often discussed the case that we could use the font size to apply drawing rule based on rank or other data. It happens with shield and text

I can now say for sure that it does not always work. I think it fails mostly at tiles bounds Those screenshots show that at zoom 9 what happens. at that place we have 2 rank 1 peaks because it is right on tiles bounds At zoom 9.3 and 9.4 (same data as same int zoom) cartoSDK does not choose the same peak to draw. Though as seen in the screenshot i use the ele in font size to define the ordering rule.

can i ask so that i understand better how the grid for labels is defined ? is it tile based on screen based? I wondering if this issue is related to another issue where you see changes of label ordering happening on screen sides as you pan a little.

Screenshot 2022-06-08 at 09 46 20 Screenshot 2022-06-08 at 09 46 28
farfromrefug commented 2 years ago

I should add it is indeed right at tile boundaries, and that because of buffering both peaks are in both tiles.

farfromrefug commented 2 years ago

@mtehver I am wondering about that code https://github.com/cartodb/mobile-carto-libs/blob/cfdef956c9f0fae167c4304b1cdcd44459e30a8e/vt/src/vt/GLTileRenderer.cpp#L985. Shouldnt it also sort by font size and opacity like it is done here https://github.com/cartodb/mobile-carto-libs/blob/cfdef956c9f0fae167c4304b1cdcd44459e30a8e/vt/src/vt/LabelCuller.cpp#L136 ?

mtehver commented 2 years ago

@farfromrefug Regarding 'grid for labels', you mean the grid used in label culler? It is screen based and used as an acceleration structure for label overlap testing.

Regarding the sorting in GLTileRenderer, it is done to reduce the number of draw calls and does not affect overlap testing. So I am still not entirely sure why this label flipping happens. It is also very difficult to debug, as there are multiple asynchronous processes that seem to affect this.

farfromrefug commented 2 years ago

@mtehver i do understand the difficulty. I am 100% sure something is wrong, i see failures in sorting based on font size all over the place (same with placement priority). I am not sure how i can help you with this. About the grid yes i mean the grid used in label culler. What i was thinking is that if it is screen based then the smallest pan can completely change the render because the grid is different and you can add one label which changes it all. My thought was that if it was tile based then it should not happen.

mtehver commented 2 years ago

@farfromrefug You can try the following ordering logic in LabelCuller.cpp, perhaps this will make it more stable:

            if (labelInfo1.layerIndex != labelInfo2.layerIndex) {
                return labelInfo1.layerIndex < labelInfo2.layerIndex;
            }
            if (labelInfo1.priority != labelInfo2.priority) {
                return labelInfo1.priority > labelInfo2.priority;
            }
            if (labelInfo1.label->getGlobalId() != labelInfo2.label->getGlobalId()) {
                return labelInfo1.label->getGlobalId() > labelInfo2.label->getGlobalId();
            }
            if (labelInfo1.size != labelInfo2.size) {
                return labelInfo1.size > labelInfo2.size;
            }
            return labelInfo1.opacity > labelInfo2.opacity;
farfromrefug commented 2 years ago

@mtehver no it did not make much difference. Now i have been debugging this and i think i found out what it is for most cases. The LabelCuller does its job and the list are always correctly ordered. The issue appears simply because other labels appearing before in the validLabelList prevent the highest ranked on in other layers from rendering (too close).

I understand there is no solution for this and that it is the normal / expected behavior. Though it is a really bad user experience because the user does not understand what we do understand here. I think the only solution for this is to implement placements properties as i explainedhere. it wont "solve" the issue but it would drastically reduce probability of this happening. I do understand that placements might be difficult to implement. Maybe we can find example implementations in other frameworks? I ll close this issue as debugging clearly showed that font size based ordering works

farfromrefug commented 2 years ago

@mtehver this issue is closed but to go further on my discussion about grid screen based and the influence of entering/lleaving features on the whole rendering. Here is a video showing it. A label "Taisnieres-sur-Hon" which is very far from the screen borders is appearing/disappearing while i just pan a bit. I Dont think this should happen. Let me know what you think and if you want me open an issue for it

https://user-images.githubusercontent.com/655344/174835823-3833433e-9c8f-4869-9386-d1bd10bfa46f.mov

farfromrefug commented 2 years ago

@mtehver i understand that i comes again from rules. Like in this case there is a new label appearing at very bottom left. And with places labels min distance then the "Taisnieres-sur-Hon" disappear. It does make sense on the algo part. Was is not correct is the rendering, it causes too many jumps in rendering for the user. My thinking is what if we were to use a grid larger than the screen, maybe even tile based ? Maybe only when using preloading? That way we would see much less jumps as labels offscreen would be taken into account while panning.

mtehver commented 2 years ago

@farfromrefug No sure what is happening here, as the labels are really far apart. Have to do some checking.