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.

Sign up for GitHub

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

Jump to bottom

A smarter setStyle() #1341

Closed
1 of 4 tasks
scothis opened this issue Jun 26, 2015 · 4 comments
Closed
1 of 4 tasks

A smarter setStyle() #1341

scothis opened this issue Jun 26, 2015 · 4 comments

Comments

@scothis
Copy link
Contributor

scothis commented Jun 26, 2015

Calling map.setStyle() currently throws away the current style. For minor style changes, this not only wastes resources, but causes the map to flash as the new context is built up and re-rendered.

Instead of performing a full reload, setStyle() should diff the new stylesheet with the current styles, and incrementally update the current styles. This incremental update should occur in a batch mode, as individual mutation calls preform significant work to flush and recreate state that is wasted when immediately followed by another mutation.

  • get current map style as an exportable stylesheet (plain JS object)
  • semantically aware stylesheet diff utility
  • batch mode for efficient application of multiple mutations
  • replace current setStyle() with new smartSetStyle()
@1ec5 1ec5 mentioned this issue Jun 26, 2015
Closed
scothis added a commit that referenced this issue Jun 29, 2015
@scothis
Track the stylesheet as a pure JSON object
d33c899 
Adds `style.json()` which returns the stylesheet as a pure JSON object
that is equivalent to the stylesheet necessary to recreate the style
object.

Mutations to the style object are reflected by subsequent calls to
`.json()`

Issue: #1341
scothis added a commit that referenced this issue Jun 29, 2015
@scothis
Batch mode for efficiently mutating Style instances
819de3e 
Mutation operations on a Style instance are designed to be atomic.
Depending on the operation, there is a significant amount of work that
must be preformed to flush and recreate state. This work is wasted when
multiple mutation methods are invokes sequentially.

Batching these mutations allows deferring this work until the batch is
complete and deduping the work.

    style.batch(function() {
        style.addLayer(layer1);
        style.addLayer(layer2);
        ...
        style.addLayer(layerN);
    });

Internally, calls to four methods are deferred:
- style._groupLayers (will only fire once)
- style._broadcastLayers (will only fire once)
- style._reloadSource (will fire once per source)
- style.fire (each call will fire)

A casual benchmark shows the time to add 80 layers dropping from ~800 ms
to ~20 ms when run as a batch.

Issue: #1341
scothis added a commit that referenced this issue Jun 29, 2015
@scothis
Utility to diff stylesheets
06efa05 
Two plain JS stylesheets are accepted as inputs, the output is an array
of operations necessary to bring the first stylesheet to be equivalent to
the second.

    var operations = styleDiff.diff(oldStylesheet, stylesheet);

Operations are a command and args. A command is the name of the method on
the style to invoke with the args. Operations are JSON serializable.

The diff for a stylesheet can be apply to a style object via patch. Patch
will throw if it is unable to fully apply the diff.

    styleDiff.patch(map.style, operations);

Issue: #1341
scothis added a commit that referenced this issue Jun 29, 2015
@scothis
Use styleDiff to apply setStyle changes incrementally
1a5a95c 
Previously, calls to map.setStyle() created a new Style instance.
Creating a new Style instance is an expensive operation that abandons the
current layers and sources. For simple updates, this is very wasteful.

Instead of creating a Style object for every call of map.setStyle() we
now diff the new style with the current style and apply the delta within
a batch update.

Situations where the non-incremental setStyle is used:
- initial style for a map
- the new stylesheet is already a Style instance
- the current style is not done loading
- the style diff cannot be applied incrementally

Issue: #1341
scothis added a commit that referenced this issue Jun 29, 2015
@scothis
Utility to diff stylesheets
8ac3fcf 
Two plain JS stylesheets are accepted as inputs, the output is an array
of operations necessary to bring the first stylesheet to be equivalent to
the second.

    var operations = styleDiff.diff(oldStylesheet, stylesheet);

Operations are a command and args. A command is the name of the method on
the style to invoke with the args. Operations are JSON serializable.

The diff for a stylesheet can be apply to a style object via patch. Patch
will throw if it is unable to fully apply the diff.

    styleDiff.patch(map.style, operations);

Issue: #1341
scothis added a commit that referenced this issue Jun 29, 2015
@scothis
Use styleDiff to apply setStyle changes incrementally
8bd05b3 
Previously, calls to map.setStyle() created a new Style instance.
Creating a new Style instance is an expensive operation that abandons the
current layers and sources. For simple updates, this is very wasteful.

Instead of creating a Style object for every call of map.setStyle() we
now diff the new style with the current style and apply the delta within
a batch update.

Situations where the non-incremental setStyle is used:
- initial style for a map
- the new stylesheet is already a Style instance
- the current style is not done loading
- the style diff cannot be applied incrementally

Issue: #1341
scothis added a commit that referenced this issue Jun 29, 2015
@scothis
Utility to diff stylesheets
bb4322d 
Two plain JS stylesheets are accepted as inputs, the output is an array
of operations necessary to bring the first stylesheet to be equivalent to
the second.

    var operations = styleDiff.diff(oldStylesheet, stylesheet);

Operations are a command and args. A command is the name of the method on
the style to invoke with the args. Operations are JSON serializable.

The diff for a stylesheet can be apply to a style object via patch. Patch
will throw if it is unable to fully apply the diff.

    styleDiff.patch(map.style, operations);

Issue: #1341
scothis added a commit that referenced this issue Jun 29, 2015
@scothis
Use styleDiff to apply setStyle changes incrementally
592058a 
Previously, calls to map.setStyle() created a new Style instance.
Creating a new Style instance is an expensive operation that abandons the
current layers and sources. For simple updates, this is very wasteful.

Instead of creating a Style object for every call of map.setStyle() we
now diff the new style with the current style and apply the delta within
a batch update.

Situations where the non-incremental setStyle is used:
- initial style for a map
- the new stylesheet is undefined
- the new stylesheet is already a Style instance
- the current style is not done loading
- the style diff cannot be applied incrementally

Issue: #1341
@scothis scothis mentioned this issue Jun 29, 2015
Closed
@anandthakker
Copy link
Contributor

This is excellent.

FWIW, I'd also love for the batching to be available through the public API (not only behind setStyle), so that, e.g., I could group dynamic updates without having to keep track of my own style JSON object.

scothis added a commit that referenced this issue Jun 30, 2015
@scothis
Batch mode for efficiently mutating Style instances
6628608 
Mutation operations on a Style instance are designed to be atomic.
Depending on the operation, there is a significant amount of work that
must be preformed to flush and recreate state. This work is wasted when
multiple mutation methods are invokes sequentially.

Batching these mutations allows deferring this work until the batch is
complete and deduping the work.

    map.batch(function(batch) {
        batch.addLayer(layer1);
        batch.addLayer(layer2);
        ...
        batch.addLayer(layerN);
    });

Methods on the batch object provided to the work function mirror methods
on Map and Style, and include:
- addLayer
- removeLayer
- setPaintProperty
- setLayoutProperty
- setFilter
- addSource
- removeSource

Internally, calls to four methods are deferred:
- style._groupLayers (will only fire once)
- style._broadcastLayers (will only fire once)
- style._reloadSource (will fire once per source)
- style.fire (each call will fire)

A casual benchmark shows the time to add 80 layers dropping from ~800 ms
to ~20 ms when run as a batch.

Issue: #1341
@scothis scothis mentioned this issue Jun 30, 2015
Merged
scothis added a commit that referenced this issue Jun 30, 2015
@scothis
Batch mode for efficiently mutating Style instances
07d4f2c 
Mutation operations on a Style instance are designed to be atomic.
Depending on the operation, there is a significant amount of work that
must be preformed to flush and recreate state. This work is wasted when
multiple mutation methods are invokes sequentially.

Batching these mutations allows deferring this work until the batch is
complete and deduping the work.

    map.batch(function(batch) {
        batch.addLayer(layer1);
        batch.addLayer(layer2);
        ...
        batch.addLayer(layerN);
    });

Methods on the batch object provided to the work function mirror methods
on Map and Style, and include:
- addLayer
- removeLayer
- setPaintProperty
- setLayoutProperty
- setFilter
- addSource
- removeSource

Internally, calls to four methods are deferred:
- style._groupLayers (will only fire once)
- style._broadcastLayers (will only fire once)
- style._reloadSource (will fire once per source)
- style.fire (each call will fire, 'change' will fire once)

A casual benchmark shows the time to add 80 layers dropping from ~800 ms
to ~20 ms when run as a batch.

Issue: #1341
@scothis
Copy link
Contributor Author

scothis commented Jun 30, 2015

@anandthakker there's a batch method on Map now as well.

@anandthakker
Copy link
Contributor

@scothis 👯 !

@lucaswoj lucaswoj mentioned this issue Jul 15, 2015
Closed
@jfirebaugh
Copy link
Contributor

Followups in #1391.

@kkaefer kkaefer mentioned this issue Sep 29, 2015
Closed
2 tasks
This was referenced Jan 25, 2016
Closed
Closed
@jingsam jingsam mentioned this issue Apr 14, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@jfirebaugh @scothis @anandthakker