Are Android Sprites actually sprites?

[tmcw]

It looks like the Sprite concept in android represents a _single_marker, which is a very different thing than the packed spritesheets of GL Native itself.

[tobrun]

@tmcw
A Sprite on the Android SDK is a wrapper of Bitmap + id.
This indeed conflicts with naming/concept of Spritesheets.

Current implementation:

We are using the following Builder pattern to create a marker.
Note that adding the marker image method is called icon and not sprite.

        mMapView.addMarker(new MarkerOptions()
                .title("Intersection")
                .snippet("H St NW with 15th St NW")
                .icon(SpriteFactory.getInstance(this).fromAsset("london-underground-24.png"))
                .position(new LatLng(38.9002073, -77.03364419)));

Google Maps implementation:

Google uses BitmapDescriptorFactory instead of SpriteFactory

mMap.addMarker(new MarkerOptions()
                          .position(MELBOURNE)
                          .icon(BitmapDescriptorFactory.defaultMarker(BitmapDescriptorFactory.HUE_AZURE)));

Suggestion:

I see 2 possibilities to resolve confusion around this topic:

• We rename everything from Sprite to Icon
• We use same naming as Google by using BitmapDescriptorFactory

cc @bleege @zugaldia

[bleege]

I think we have some legacy issues here. Sprites used to be supported as Maki icons before, but now that's been removed and only custom image assets are supported (outside the default icon). We likely could move to something called IconFactory or BitmapFactory.

[jfirebaugh]

For the styles API and general GL user documentation, we've settled on "sprite" = a set of individual images collected into one object for optimal network/graphics performance, and "icon" = an constituent element of a sprite. So +1 for IconFactory.

[tobrun]

Going to change implementation to IconFacttory, will merge result to the correct release branch.

[tobrun]

I replaced all sprite occurrences with icon in both Java as JNI code.

Results tests for default markers and custom markers:

[tobrun]

This landed in f3de446 on the release branch.

@bleege @zugaldia
Could you validate if changes are ok for release?
Validate if it's is ok to break the api in this manner?
Check if we should rather use deprecation instead?

[bleege]

Thanks @tobrun! This is a big enough API change that we should bump the release to 3.0.0 from 2.4.0 to be more consistent with SemVer. I'll update the release ticket to note this.

[bleege]

@tobrun Is there a PR for this? I'm just trying to make sure that we have everything documented for release.

[tobrun]

This is a big enough API change that we should bump the release to 3.0.0 from 2.4.0

I'm seeing an opportunity here:

Land the MapboxMap class refactor #3145:

• Higher parity with Google Maps
• Makes code modular + testable
• Introduce camera api as default interaction model.
• hides old deprecated MapView implementation for new users
• backwards compatible, you can still use same code as today

End result would be Google Maps drop-in replacement:

public class MapsActivity extends FragmentActivity implements OnMapReadyCallback {
    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_maps);
        SupportMapFragment mapFragment = (SupportMapFragment) getSupportFragmentManager()
                .findFragmentById(R.id.map);
        mapFragment.getMapAsync(this);
    }

    @Override
    public void onMapReady(MapboxMap map) {
        // Add a marker in Sydney, Australia, and move the camera.
        LatLng sydney = new LatLng(-34, 151);
        map.addMarker(new MarkerOptions().position(sydney).title("Marker in Sydney"));
        map.moveCamera(CameraUpdateFactory.newLatLng(sydney));
    }
}

@bleege @zugaldia

[bleege]

Sounds good @tobrun. Please ticket it out so that work can be done on it after 3.0.0 is released early this week. Let's not rush a change this big for the sake of SemVer. Numbers are infinite and we can always do a 4.0.0 release. ;-)

[zugaldia]

Numbers are infinite

Just consulted with my college books. Checks out.