Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

[android] Add support for data driven styling #7752

Merged
merged 2 commits into from
Jan 26, 2017
Merged

Conversation

ivovandongen
Copy link
Contributor

@ivovandongen ivovandongen commented Jan 17, 2017

Adds support for data driven styling to the Android SDK.

TODO:

Functions

  • Camera (Zoom)
    • Exponential
    • Interval
    • Unit tests
  • Source (Property)
    • Categorical
    • Exponential
    • Identity
    • Interval
    • Unit tests
  • Composite (ZoomAndProperty)
    • Categorical
    • Exponential
    • Interval
    • Unit tests

Other:

  • Merge Property and PropertyValue
  • Test activity/examples
  • JavaDoc

@ivovandongen ivovandongen added the Android Mapbox Maps SDK for Android label Jan 17, 2017
@ivovandongen ivovandongen added this to the android-v5.0.0 milestone Jan 17, 2017
@ivovandongen ivovandongen self-assigned this Jan 17, 2017
@mention-bot
Copy link

@ivovandongen, thanks for your PR! By analyzing this pull request, we identified @incanus and @jfirebaugh to be potential reviewers.

@ivovandongen
Copy link
Contributor Author

@mapbox/android I've restored support for exponential zoom functions on this branch and set up the basis for the property / zoom-and-property functions. For this I adhered to the naming in the style spec and not the core naming. So, for example, ZoomFunction and not CameraFunction.

@ivovandongen ivovandongen force-pushed the dds-android-wip branch 2 times, most recently from dc0a02c to 8d08c61 Compare January 19, 2017 15:58
@ivovandongen
Copy link
Contributor Author

ivovandongen commented Jan 19, 2017

After the discussion yesterday I've re-implemented the functions and stops with typed stops in 0a28d4d. Overall, I think it looks good.

We now have:

Functions:

  • CameraFunction
  • SourceFunction - Contains the referenced property
  • CompositeFunction (not implemented yet)

Stops:

  • CategoricalStops (not implemented yet)
  • ExponentialStops - Contains base, stops (and soon colorSpace)
  • IdentityStops - Empty, marker class
  • IntervalStops - Contains stops

The DSL is spread out in Function, Stop and Stops and looks like this for CameraFunctions (exponential):

    //Set
    layer.setProperties(
      fillOpacity(
        zoom(
          exponential(
            0.5f,
            stop(2, fillOpacity(0.3f))
          )
        )
      )
    );

    //Get
    float base = ((ExponentialStops) layer.getFillOpacity().getFunction().getStops()).getBase();
    Stop[] stops = ((ExponentialStops) layer.getFillOpacity().getFunction().getStops()).stops;

And like this for SourceFunctions (identity):

    //Set
    layer.setProperties(
      fillOpacity(property("FeaturePropertyA", Stops.<Float>identity()))
    );

    //Get
    String property = ((SourceFunction) layer.getFillOpacity().getFunction()).getProperty();

The typing is a little uncomfortable when interrogating the style. Suggestions are welcome.

@tobrun
Copy link
Member

tobrun commented Jan 20, 2017

API looks great, can't think of a simpler construction as the proposed DSL.

@ivovandongen
Copy link
Contributor Author

ivovandongen commented Jan 23, 2017

I've implemented the composite functions (unit test generation forthcoming). The DSL for composite functions looks similar to the style spec format for Zoom-And-Property functions:

      fillOpacity(
        composite(
          "FeaturePropertyA",
          exponential(
            0.5f,
            stop(0, 0.3f, fillOpacity(1f)),
            stop(0, 0.5f, fillOpacity(0.3f)),
            stop(1, 0.3f, fillOpacity(1f)),
            stop(1, 0.5f, fillOpacity(0.3f))
          )
        )
      )

Where there is only one stops implementation for all zoom levels.

An alternative which is closer to the representation in the core implementation and the proposal for function revision:

    layer.setProperties(
      fillOpacity(
        composite(
          "FeaturePropertyA",
          stop(0,
            exponential(
              0.5f,
              stop(0.3f, fillOpacity(1f)),
              stop(0.5f, fillOpacity(0.3f))
            )
          ),
          stop(1,
            exponential(
              0.5f,
              stop(0.3f, fillOpacity(1f)),
              stop(0.5f, fillOpacity(0.3f))
            )
          )
        )
      )

This offers more flexibility, but is also more verbose.

The data representation follows the core and stores a Stops implementation per zoom level instead of a list with composite keys (eg Stop<CompositeValue<Z, I>, O>). This involves a awkward conversion step internally as the conversion templates expect the latter format for compatibility with the spec, but this can be optimised later if needed.

In addition, I made some changes to the Stops classes and added a IterableStops abstract base class so it is easier to access the actual stops collections without code like getStops().getStops().length (now getStops().size()). This does involve some generics acrobatics:

CompositeStops<Float, Float, Float, ExponentialStops<Float, Float>> stops = (CompositeStops<Float, Float, Float, ExponentialStops<Float, Float>>) layer.getFillOpacity().getFunction().getStops();

@ivovandongen
Copy link
Contributor Author

@jfirebaugh Finished up on this. After reviews, shall I merge it back into the dds-wip branch?

@jfirebaugh jfirebaugh force-pushed the dds-wip branch 3 times, most recently from 4dda0fe to d867492 Compare January 26, 2017 00:23
@jfirebaugh jfirebaugh merged commit d5a6b8e into dds-wip Jan 26, 2017
@jfirebaugh jfirebaugh deleted the dds-android-wip branch January 26, 2017 01:28
@zugaldia zugaldia mentioned this pull request Feb 14, 2017
10 tasks
@zugaldia zugaldia mentioned this pull request Feb 28, 2017
9 tasks
@zugaldia zugaldia mentioned this pull request Mar 10, 2017
8 tasks
@tobrun tobrun mentioned this pull request Mar 17, 2017
10 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Android Mapbox Maps SDK for Android
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants