WIP: Re-enable bucket discard optimization #5921

Draft

pastk wants to merge 2 commits from pastk-drape-bucket-discard-optimization into master

pastk commented

2023-09-03 14:59:20 +00:00

Member

There is some render bucket discard optimization, which is not working for quite some time.

Looks like it was broken by 9c36be925a (the minVisibleScale var changes do not get passed to the insertShape() anymore).

Originially the optimization was added in mapsme/omim@519e60d (minVisibleScale was calculated always).
Then it was modified in mapsme/omim#2570 so that minVisibleScale was calculated for point features and areas with point styles only, i.e. overlays only, but w/o lines overlays (pathtexts/shields). I don't know why.

@vng you were reviewing the changes back then so maybe you remember the logic behind it?

I don't really understand the surrounding code and how this optimization is supposed to work:

why there are several render buckets per render group?
why render buckets are not discarded immediately upon zoom level change?
why this optimization was modified to be used for some overlays only? (according to debug output it was affecting DepthLayer::GeometryLayer buckets only, so not sure how its connected to overlays)

Commit 1 just restores the minVisibleScale passing code that was used before 0ac765a27f. But it doesn't work (SetFeatureMinZoom gets a 0 always).

Sample code in commit 2 actually works (both the original "calc minVisibleScale always" version and a new lighter weight attempt) and some buckets are being discarded, though I don't see any difference on the screen (not the picture/animation, not the speed) and its risky to re-enable it without fully understanding how it works.

There is some render bucket discard optimization, which is not working for quite some time. Looks like it was broken by 9c36be925a1af4c2f0689a0f9fc5289da99f35b3 (the minVisibleScale var changes do not get passed to the insertShape() anymore). Originially the optimization was added in mapsme/omim@519e60d (minVisibleScale was calculated always). Then it was modified in mapsme/omim#2570 so that minVisibleScale was calculated for point features and areas with point styles only, i.e. overlays only, but w/o lines overlays (pathtexts/shields). I don't know why. @vng you were reviewing the changes back then so maybe you remember the logic behind it? I don't really understand the surrounding code and how this optimization is supposed to work: - why there are several render buckets per render group? - why render buckets are not discarded immediately upon zoom level change? - why this optimization was modified to be used for some overlays only? (according to debug output it was affecting DepthLayer::GeometryLayer buckets only, so not sure how its connected to overlays) Commit 1 just restores the minVisibleScale passing code that was used before https://git.omaps.dev/organicmaps/organicmaps/commit/0ac765a27f3825fba7e614495da736f01c5ed1a6. But it doesn't work (SetFeatureMinZoom gets a `0` always). Sample code in commit 2 actually works (both the original "calc minVisibleScale always" version and a new lighter weight attempt) and some buckets are being discarded, though I don't see any difference on the screen (not the picture/animation, not the speed) and its risky to re-enable it without fully understanding how it works.

vng (Migrated from github.com) reviewed 2023-09-03 14:59:20 +00:00

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

No reviewers

vng