Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

fix: source with transform reloads #789

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

wangxingkai
Copy link

Hey team,

I found an issue when using a source with a transform function. The source will be reloaded whenever the scene.updateConfig() is called. This impacted the performance a lot as my project uses a bundled source with the transform for multiple tiled layers.

The bug is because the this.last_config_sources in the comparison condition is mutated by the in-place compileFunctionStrings function in the following logic. It's no longer the serialised object with "original function strings", but a deserialised object with "compiled real functions", and the real functions are ignored by JSON.stringify. so there will always be differences between the new and last source config which reloads the source.

https://github.com/wangxingkai/tangram/blob/bf7ff0f5e90da963e18df709d8f041c93fd94836/src/scene/scene_worker.js#L117
https://github.com/wangxingkai/tangram/blob/bf7ff0f5e90da963e18df709d8f041c93fd94836/src/scene/scene_worker.js#L123

if add a transform function in the demo code, the bug can be reproduced:

https://github.com/wangxingkai/tangram/blob/7baed5ea035938bc9de4116ff9a1ce039ddebb47/demos/scene.yaml#L110

        attribution: |
            Tiles by <a href="https://www.nextzen.org/" target="_blank">Nextzen</a>
            w/data from <a href="https://www.openstreetmap.org/" target="_blank">OpenStreetMap</a> and
            <a href="https://whosonfirst.org/" target="_blank">Who's On First</a>
+       transform: |
+           function (data) { return data; }
        # request_headers: # send custom headers with tile requests
        #     Authorization: Bearer xxxxxxxxx

Before fix:
https://user-images.githubusercontent.com/6101654/152491059-5f31a474-a6a0-48a0-a283-39f9e5a7d474.mov

After fix:
https://user-images.githubusercontent.com/6101654/152491220-d8152c74-56d6-421a-96d1-f4b2142f0685.mov

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant