Skip to content
This repository has been archived by the owner on Apr 10, 2018. It is now read-only.

Style diff #324

Merged
merged 2 commits into from
Aug 10, 2015
Merged

Style diff #324

merged 2 commits into from
Aug 10, 2015

Conversation

scothis
Copy link
Contributor

@scothis scothis commented Aug 5, 2015

Proposal for what style diffing could look like within mapbox-gl-style-spec. This code has worked well with mapbox-gl-js. Particularly interested in thoughts from @jfirebaugh @incanus and @lucaswoj

Fast, semantically aware style diff utility. The differ compares two
stylesheets creating a list of changes. Operations produced by the diff
resemble the mapbox-gl-js API in spirit. Any error creating the diff will
fall back to the 'setStyle' operation.

Operations:
- setStyle
- addLayer
- removeLayer
- setPaintProperty
- setLayoutProperty
- setFilter
- setLayerZoomRange
- setLayerProperty
- addSource
- removeSource
- setConstant
- setSprite
- setGlyphs
- setTransition

Example diff:
    [
        { command: 'setConstant', args: ['@water', '#0000FF'] },
        { command: 'setPaintProperty', args: ['background', 'background-color', 'black'] }
    ]

Applying the diff is an exercise left to each mapbox-gl implementation. A
crude mapbox-gl-js implementation would look like:
    var changes = diff(oldStylesheet, newStylesheet);
    try {
        map.batch(function (batch) {
            changes.forEach(function (change) {
                batch[change.command].apply(batch, change.args);
            });
        });
    } catch (e) {
        console.warn('Unable to apply diff:', e);
        map.setStyle(newStylesheet);
    }
@jfirebaugh
Copy link
Contributor

Makes sense to me to have this code live here if we need to reuse it (are we at that point now?). It should live within lib though, like the other sources.

@scothis
Copy link
Contributor Author

scothis commented Aug 5, 2015

I started with the code in lib, but moved it out to make it easier to require a specific version of the differ. This is the same pattern that is used for migrations and reference.

For example:
require('mapbox-gl-style-spec/diff/v8');

vs:
require('mapbox-gl-style-spec/lib/diff/v8');

@jfirebaugh
Copy link
Contributor

My hunch is we will want to take an approach more like that of validation, where version differences are captured in inline branches, rather than having a completely separate implementation for each version.

@scothis
Copy link
Contributor Author

scothis commented Aug 5, 2015

That would simplify the layout a good bit. The downside is that we'd only ever be able to diff stylesheet of the latest version. If we're ok with that, I'll make the change.

@lucaswoj
Copy link

lucaswoj commented Aug 6, 2015

The idea and the implementation look solid.

I have a nagging fear that, in the long run, making GL style changes more efficient using diffs will be more painful than doing so using smart caching of the underlying resources.

@scothis
Copy link
Contributor Author

scothis commented Aug 6, 2015

Incremental updates to a live style is just one use case. Another would be comparing the cumulative changes before publishing a new style. Another could be differential sync of styles across multiple collaborators. I'm sure there are other use cases I haven't thought of yet.

Minimizing the number of changes should always be more efficient, but I'll shed 😂 when setStyle is fast.

scothis added a commit that referenced this pull request Aug 10, 2015
@scothis scothis merged commit 25641ed into mb-pages Aug 10, 2015
@scothis scothis deleted the v7-style-diff branch August 10, 2015 21:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants