WIP: [drape] Make path_texts use minVisibleScale for rendering order #4428

Closed

pastk wants to merge 1 commit from pastk-drape-path_text_zoom_mask into master

pastk commented

2023-02-10 15:37:56 +00:00

Owner

At the moment path_texts (e.g. street names) are ignoring minVisibleScale value and all priority comparisons with them a made using the priority value derived from z-index only.

In more detail - path_texts don't use the first "zoom" byte here for comparisons with other overlays:
[1 byte - zoom][4 bytes - priority][1 byte - rank][2 bytes - 0xFFFF]

This path_text-specific exception was introduced in b577a55b1e and the reason for it is not clear (it was a small part of a big Drape2.0 refactoring).

So far I don't see any good reason to keep this exception.
The immediate results of removing this exception are mixed - sometimes it looks better, sometimes worse.
What is the most important IMO that removing this exception will make the priorities pipeline more simple, predictable, and overall based on a systemic approach instead of layering situational hacks and workarounds over each other.
And then we can work on improving priorities in a more generic, standard way.

On small zooms (z11-13) it looks generally better IMO:
city names are visible instead of river names and motorway names are more visible.

From z14- onwards results are mixed:
its generally good to see more street names, but sometimes they displace important POIs (e.g. train stations).

This is because ATM generally road path_texts have very high priority because they take minVisibleScale value from road lines which are drawn earlier than the most of POIs.
We can change it later to use minVisibleScale to the actual visibility of the draw element (path_text in this case).
After that we'll be able to fine-tune render order inside "zoom buckets" by changing z-index values.

At the moment path_texts (e.g. street names) are ignoring minVisibleScale value and all priority comparisons with them a made using the priority value derived from z-index only. In more detail - path_texts don't use the first "zoom" byte here for comparisons with other overlays: `[1 byte - zoom][4 bytes - priority][1 byte - rank][2 bytes - 0xFFFF]` This path_text-specific exception was introduced in https://github.com/mapsme/omim/commit/b577a55b1e0145ad94d25a96f32b6ffde38b4f49 and the reason for it is not clear (it was a small part of a big Drape2.0 refactoring). So far I don't see any good reason to keep this exception. The immediate results of removing this exception are mixed - sometimes it looks better, sometimes worse. What is the most important IMO that removing this exception will make the priorities pipeline more simple, predictable, and overall based on a systemic approach instead of layering situational hacks and workarounds over each other. And then we can work on improving priorities in a more generic, standard way. On small zooms (z11-13) it looks generally better IMO: city names are visible instead of river names and motorway names are more visible. ![path_texts_Zurich_z10-11](https://user-images.githubusercontent.com/18434508/218131651-6ddf728a-8d14-4ae3-963b-8d66c8d79e1f.png) ![path_texts_Zurich_z12-13](https://user-images.githubusercontent.com/18434508/218131749-270bfa96-005b-4183-89d1-b892af5f6abd.png) From z14- onwards results are mixed: its generally good to see more street names, but sometimes they displace important POIs (e.g. train stations). ![path_texts_Zurich_z14-15](https://user-images.githubusercontent.com/18434508/218131901-43fb8acb-c99e-4272-b26e-614d33880fb9.png) ![path_texts_Zurich_z16-17](https://user-images.githubusercontent.com/18434508/218131944-4d2f2ef9-3615-41d8-82a3-b90b031d212f.png) This is because ATM generally road path_texts have very high priority because they take minVisibleScale value from road lines which are drawn earlier than the most of POIs. We can change it later to use minVisibleScale to the actual visibility of the draw element (path_text in this case). After that we'll be able to fine-tune render order inside "zoom buckets" by changing z-index values.

biodranik (Migrated from github.com) reviewed 2023-02-10 15:37:56 +00:00

vng (Migrated from github.com) reviewed 2023-02-10 16:33:30 +00:00

vng (Migrated from github.com) left a comment

I agree that now streets are rare-visible sometimes and are displaced even with bicycle parkings. We have a bunch of issues about street names.

I suspect that we should investigate tradeoff here, especially for bus_stop, parking, charging icons that are usually placed along the street.

I agree that now streets are _rare-visible_ sometimes and are displaced even with bicycle parkings. We have a bunch of issues about street names. I suspect that we should investigate _tradeoff_ here, especially for bus_stop, parking, charging icons that are usually placed along the street.

This repo is archived. You cannot comment on pull requests.

No reviewers

biodranik