Skip to content
New issue

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

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

Already on GitHub? # to your account

Replace no-op style filter expressions #62

Merged
merged 10 commits into from
Jul 29, 2024
Merged

Replace no-op style filter expressions #62

merged 10 commits into from
Jul 29, 2024

Conversation

lily-chai
Copy link
Contributor

@lily-chai lily-chai commented Jul 26, 2024

This PR adds a "pre-filter" that removes no-op style filter expressions as ['pitch'], ['distance-from-center'] by replacing them with an expression that always evaluates to true.

@lily-chai lily-chai marked this pull request as ready for review July 26, 2024 11:57
Comment on lines 133 to 138
[
"case",
[">=", ["distance-from-center"], 1],
true,
[">=", ["pitch"], 45]
],
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we change the expression a little to be:

[
              "case",
              [">=", ["distance-from-center"], 1],
              false,
              [">=", ["pitch"], 45]
          ],

The case expression will return false, excluding the feature. Therefore, we probably need to aggressively set the entire case expression to ['literal', true].

@lily-chai lily-chai requested a review from zmiao July 29, 2024 12:43
@@ -33,15 +33,15 @@ function styleToFilters(style) {
if (styleMin < layers[layerName].minzoom) layers[layerName].minzoom = styleMin;
if (styleMax > layers[layerName].maxzoom) layers[layerName].maxzoom = styleMax;
// Modify filter
if (layers[layerName].filters === true || !style.layers[i].filter) {
if (layers[layerName].filters === true || !style.layers[i].filter || isNoOpFilter(style.layers[i].filter)) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this would be too aggressive, so for such case:

            "filter": [
                "all",
                [
                    "case",
                    ["<=", ["pitch"], 45],
                    true,
                    ["<=", ["distance-from-center"], 1]
                ],
                ["match", ["get", "type"], ["Toll booth"], true, false]
            ],

we would want to replace it with:

  "filter": [
                "all",
                [
                    "literal",
                     true
                ],
                ["match", ["get", "type"], ["Toll booth"], true, false]
            ],

instead of just ['literal', true]

@lily-chai lily-chai requested a review from zmiao July 29, 2024 15:09
Copy link

@zmiao zmiao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@lily-chai lily-chai merged commit 9df32fc into master Jul 29, 2024
2 checks passed
@lily-chai lily-chai deleted the prefilter branch July 29, 2024 17:47
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants