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

Android - make it possible to change image for existing image ID, improve performance of DownloadMapImageTask #3431

Merged
merged 1 commit into from
Apr 27, 2024

Conversation

Buthrakaur
Copy link
Contributor

@Buthrakaur Buthrakaur commented Mar 22, 2024

Description

  • it was not possible before to change image for existing image ID contrary to iOS even though it's supported in the native component
  • DownloadMapImageTask image loading was slightly improved to load images in parallel

Fixes #3331

Checklist

  • I've read CONTRIBUTING.md
  • I updated the doc/other generated code with running yarn generate in the root folder
  • I have tested the new feature on /example app.
    • In V11 mode/ios
    • In New Architecture mode/ios
    • In V11 mode/android
    • In New Architecture mode/android
  • I added/updated a sample - if a new feature was implemented (/example)

Screenshot OR Video

Component to reprocuce the issue you're fixing

const Markers: React.FC<MarkersProps> = (props) => {
  const { map } = useMap();
  const boundsChanged = useMapCameraIdle().cameraIdleBounds;

  const [renderedFeatureIds, setRenderedFeatureIds] = useState<string[]>([]);

  useEffect(() => {
    if (!boundsChanged) return;
    const func = async () => {
      const renderedFeatures = await map.current?.queryRenderedFeaturesInRect(
        [0, 0, 9000, 9000],
        [],
        [`SymbolLayer${props.layerId}Large`]
      );

      if (!renderedFeatures) return;

      setRenderedFeatureIds([
        ...renderedFeatures.features.map((f) => `${f.id}`),
        // keep image for previously rendered features to not cause flickering, cap it to 100 to prevent infinite growth
        ...renderedFeatureIds.slice(0, 100),
      ]);
    };

    func();
  }, [map, boundsChanged]);

  const featuresCollection = useMemo<GeoJSON.FeatureCollection>(() => {
    const features: GeoJSON.FeatureCollection['features'] = props.markers.map((m, i) => {
      return {
        type: 'Feature',
        id: m.id,
        properties: { ...getCollectionPropperties(m, i) },
        geometry: {
          type: 'Point',
          coordinates: m.coordinates,
        },
      };
    });

    return {
      type: 'FeatureCollection',
      features,
    };
  }, [props.markers]);

  const images = useMemo<SymbolLayerStyle['images']>(() => {
    const result: { [key: string]: MapboxGL.ImageEntry } = {};

    props.markers.forEach((m) => {
      const isRendered = renderedFeatureIds.includes(m.id);
      const { width, height } = { width: POI_IMG_W, height: POI_IMG_H };

      if (isRendered) {
        result[m.id] = {
          uri: getRemoteImageUri(m),
          width,
          height,
        };
      } else {
        // non-rendered markers get a placeholder image to prevent map loading all images at once
        result[m.id] = getImagePlaceholder(m);
      }
    });

    return result;
  }, [props.markers, renderedFeatureIds, getRemoteImageUri, getImagePlaceholder]);

  return (
    <>
      <MapboxGL.Images images={images} />
      <MapboxGL.ShapeSource
        id={props.layerId}
        shape={featuresCollection}
        onPress={onFeaturePress}
        hitbox={useMemo(() => ({ width: 0, height: 0 }), [])}
      >
        <MapboxGL.SymbolLayer
          id={`SymbolLayer${props.layerId}Small`}
          minZoomLevel={props.minZoomLevel}
          maxZoomLevel={props.maxZoomLevel}
          style={pinSmallStyle}
        />
        <MapboxGL.SymbolLayer
          id={`SymbolLayer${props.layerId}Medium`}
          minZoomLevel={props.minZoomLevel}
          maxZoomLevel={props.maxZoomLevel}
          style={pinMediumStyle}
        />
        <MapboxGL.SymbolLayer
          id={`SymbolLayer${props.layerId}Large`}
          minZoomLevel={props.minZoomLevel}
          maxZoomLevel={props.maxZoomLevel}
          style={pinLargeStyle}
        />
      </MapboxGL.ShapeSource>
    </>
  );
};

- improve performance of DownloadMapImageTask - download images in parallel using Kotlin coroutines
Copy link
Contributor

@mfazekas mfazekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Buthrakaur thanks much for the PR, looks nice!

Pls add a simple component to the PR description to reproduce the issue this solves.

@Buthrakaur
Copy link
Contributor Author

Hi @mfazekas , sorry for the late response - I just updated the PR description. The update of image in images for an existing image ID due to isRendered change didn't work on Android before the changes in this PR. The same problem was not on iOS and it just works.

@hexadecy
Copy link
Contributor

hexadecy commented Apr 21, 2024

I have tested the new feature on /example

  • In v11 mode, Android 9
  • In v11 mode, New Architecture, Android 9
  • In v10 mode, Android 9

@mfazekas
Copy link
Contributor

@Buthrakaur I wasn't able to use your component as it uses lot of thing not included. I could reproduce the issue and confirm the fix using this component:

import React from 'react';
import {
  MapView,
  Images,
  Camera,
  SymbolLayer,
  ShapeSource,
} from '@rnmapbox/maps';
import { Button } from 'react-native';

import exampleIcon from '../assets/compass1.png';
import exampleIcon2 from '../assets/compass2.png';

const styles = {
  icon: {
    iconImage: ['get', 'icon'],

    iconSize: [
      'match',
      ['get', 'icon'],
      'example',
      1.2,
      'airport-15',
      1.2,
      /* default */ 1,
    ],
    iconAllowOverlap: true,
  },
  matchParent: { flex: 1 },
};

const featureCollection = {
  type: 'FeatureCollection',
  features: [
    {
      type: 'Feature',
      id: '9d10456e-bdda-4aa9-9269-04c1667d4552',
      properties: {
        icon: 'example',
      },
      geometry: {
        type: 'Point',
        coordinates: [-117.20611157485, 52.180961084261],
      },
    },
    {
      type: 'Feature',
      id: '9d10456e-bdda-4aa9-9269-04c1667d4552',
      properties: {
        icon: 'example2',
      },
      geometry: {
        type: 'Point',
        coordinates: [-117.205908, 52.180843],
      },
    },
  ],
};

class Example extends React.Component {
  state = {
    images: {
      example: exampleIcon2,
      example2: exampleIcon,
    },
    i: 0,
  };

  render() {
    const { images, i } = this.state;

    return (
      <>
        <MapView style={styles.matchParent}>
          <Camera
            defaultSettings={{
              zoomLevel: 16,
              centerCoordinate: [-117.20611157485, 52.180961084261],
            }}
          />
          <Images images={images} />
          <ShapeSource id="exampleShapeSource" shape={featureCollection}>
            <SymbolLayer id="exampleIconName" style={styles.icon} />
          </ShapeSource>
        </MapView>
        <Button
          title="Change"
          onPress={() =>
            this.setState({
              images: {
                ...this.state.images,
                example: i === 0 ? exampleIcon : exampleIcon2,
                example2: i === 0 ? exampleIcon2 : exampleIcon,
              },
              i: i === 0 ? 1 : 0,
            })
          }
        />
      </>
    );
  }
}

export default Example;

@mfazekas mfazekas merged commit 7737dfe into rnmapbox:main Apr 27, 2024
10 checks passed
@@ -142,6 +142,10 @@ dependencies {
// React Native
implementation "com.facebook.react:react-native:+"

// kotlin coroutines
implementation "org.jetbrains.kotlinx:kotlinx-coroutines-core:${rootProject.ext.has('kotlinVersion') ? rootProject.ext.get('kotlinVersion') : '1.6.21'}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately kotlinx-coroutines-core doesn't correlates to the Kotlin version 1 to 1, this break RN 0.74 code which sets kotlinVersion = '1.9.22'

https://central.sonatype.com/artifact/org.jetbrains.kotlinx/kotlinx-coroutines-core/1.8.1-Beta/versions

https://react-native-community.github.io/upgrade-helper/?from=0.73.7&to=0.74.0

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.

[Bug]: SymbolLayer Images are not rendered once they are loaded from url
3 participants