rnmapbox / maps

A Mapbox react native module for creating custom maps
MIT License
2.27k stars 849 forks source link

[Bug]: inconsistent iconSize between iOS and Android #3009

Closed paulschreiber closed 1 year ago

paulschreiber commented 1 year ago

Mapbox Implementation

Mapbox

Mapbox Version

default

Platform

iOS, Android

@rnmapbox/maps version

10.0.11

Standalone component to reproduce

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

const styles = {
  mapView: { flex: 1 },
  iconLayer: {
    iconSize: 3.0,
    iconImage: 'example'
  },
};

const features = {
  type: 'FeatureCollection',
  features: [
    {
      type: 'Feature',
      id: 'a-feature',
      geometry: {
        type: 'Point',
        coordinates: [-74.00597, 40.71427],
      },
    },
  ],
};

class BugReportExample extends React.Component {
  render() {
    return (
      <MapView style={styles.mapView}>
        <Camera centerCoordinate={[-74.00597, 40.71427]} zoomLevel={14} />
        <Images images={{ example: require('../assets/example.png') }} />
        <ShapeSource id={'shape-source-id-0'} shape={features}>
          <SymbolLayer
            id="symbol-id"
            style={styles.iconLayer}
          />
        </ShapeSource>
      </MapView>
    );
  }
}

export default BugReportExample;

Observed behavior and steps to reproduce

Images on iOS are are ≈2.5x larger than on Android:

image image

Expected behavior

Same size on both device types.

Notes / preliminary analysis

React 18.2.0 React Native 0.72.4 @rnmapbox/maps 10.0.11

Additional links and references

Previous related bug: https://github.com/rnmapbox/maps/issues/68

shrouxm commented 1 year ago

note from paul's teammate: we haven't been able to determine yet whether this is a bug in our code which generates the icon image per platform, or a bug in rnmapbox's icon scaling, so if this can't be reproduced it should be safe to close out

andrei-tofan commented 1 year ago

We are having the same problem, please write an update if it was a bug in your code. We run into this issue while updating to v10.

shrouxm commented 1 year ago

update: current best guess is that it is due to rnmapbox ignoring the scale attribute of ImageURISources passed into Mapbox.Images's images argument on Android only. in our case we couldn't avoid using the scale attribute due to the way our vector icon library generates images, so we are trying to work around by multiplying the size of the icon on Android only by PixelRatio.get(). will update again if this resolves on all platforms once it gets pushed to more users' devices.

andrei-tofan commented 1 year ago

We noticed that this issue happens for us when we import an image like this:

import defaultMapLayerIcon from './empty-icon.png';
const images  = {
  example: defaultMapLayerIcon
}

when we are loading images from disk like this is rendered correctly:

const images = {
  example: { uri: imagePathOnDisk, scale: pixelRatio }
}
andrei-tofan commented 1 year ago

I've done a bit more digging around this issue and I think what is causing the issue is that some Android devices have a pixel ratio with decimal points (on one of my devices is 1.75 for example).

For example, when an image is loaded from the disk in our case we default to scale: 1 because is not 2

{"scale": 1, "uri": "/data/path-to/image.png"}

and this renders correctly.

but when the image is loaded from the bundler the source looks like this:

 {"__packager_asset": true, "height": 24, "scale": 2, "uri": "http://localhost:8081/assets/path-to/image@2x.png?platform=android&hash=a7a7ce8f1a114b666057e5661c93e6d8", "width": 16}

so it loads the scale: 2 images so I think that's what causing the issue.

On iOS it works fine because the scale is 2 when calling PixelRatio.get().

The solution I found for this issue is to set the iconSize: PixelRatio.get() when the PixelRatio.get() returns a number with decimal points.

Not sure how this case was handled on the old repo, but this workaround was not needed.

JasonSanford commented 1 year ago

I'm seeing something similar, but with the built-in user heading icon. On iOS in the dev client the image is the proper size, but when built for production the image looks about 3x too big.

Dev Client

IMG_2503

Production Build

IMG_2504

Vednus commented 1 year ago

It's really strange for me. Only certain icons are different sizes, some are rendered the same size regardless of platform. Tried resizing the inconsistent one, but didn't seem to resolve it.

snazarkoo commented 1 year ago

Hey, I think a potential fix could be done here android/src/main/java-v10/com/mapbox/rctmgl/components/images/RCTMGLImages.kt We might need to replace (1.0/((160.0/bitmap.density)) * scale).toFloat() , with scale.toFloat(). Tested it on 2 devices with different ratio sizes. Works well on my end Update: Just checked with the V9 and on v10 it looks a bit smaller, so it's not the the right fix

mfazekas commented 1 year ago

I think those are the factors: 1.) Image DPI (a 200x200 image with 144 DPI should be half the size on the map than a 200x200 image with 77 DPI). 2.) RN scale. For resolved images (generated using import) RN should be generating a scale value. If the image has multiple variants @2x and @3x suffix, RN should be using the one matching the scaling of the screen. 3.) Screen scaling (I think this mostly affects android). Device density can go from 160 DPI/MDPI => XXXHDPI aka 640DPI) 4.) Debug/release version, in debug version the image is coming from metro bundler and I think it's a url, wile in release it's from the disk, and has different structure. 5.) New In @mapbox/maps is that you can also specify a scale when defining an image

A dev asset looks like this:

'asset', { __packager_asset: true,
  width: 44,
  height: 44,
  uri: 'http://localhost:8081/assets/src/assets/example2@3x.png?platform=ios&hash=2557a9ee1ee49ac29dff472336c97a87',
  scale: 3 }

See also: https://github.com/rnmapbox/maps/issues/68

See also the fixes: https://github.com/rnmapbox/maps/pull/191

snazarkoo commented 1 year ago

Thanks, as I understood, in your fixes for old versions the scale is passed directly to sdk methods without modifying. So what scale RN picks depends on what is available in file system (@, @2x, @3x), that image will be created.

In release mode, the asset looks like this { NativeMap: {"__packager_asset":true,"width":29,"uri":"packages_native_src_assets_map_route_bridge_unsafe","height":29,"scale":2} }

Hence, in both cases (debug, prod) the scale property is available and the value is the same. And it's correct as on my device pixelRatio is 1.5 and the image was picked @2x.

mfazekas commented 1 year ago

So I've tried the example in the bug report, and for me it was the same size.

image

image

This was in debug build and using the example in the example.png repo:

example.png

example.png

mfazekas commented 1 year ago

So if anyone have issue with icon size, please write a clear description: 1.) Prod or Debug version 2.) Exact image (was it a multi scale image with `@2x etc version) 3.) Exact images declaration (was scale property used?) 4.) Android device denisity

Screenshots on both platform.

Ideally you can also add a circle layer above the icons, to see how it compares with the desired size:

With the example.png as it's 44x44 pixel sized, and there is a iconSize=3 in the StyleLayer style

I've just added a CircleLayer to verify it's correct size on both platforms:

        <ShapeSource id={'shape-source-id-0'} shape={features}>
          <SymbolLayer id="symbol-id" style={styles.iconLayer} />
          <CircleLayer
            id="circel-layer"
            style={{
              circleRadius: 3 * (44.0 / 2),
              circleOpacity: 0.4,
              circleStrokeColor: 'green',
              circleStrokeWidth: 2,
            }}
          />
        </ShapeSource>

android: image

iOS: image

FWIW it's the code I've used:

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

const styles = {
  mapView: { flex: 1 },
  iconLayer: {
    iconSize: 3.0,
    iconImage: 'example',
  },
};

const features = {
  type: 'FeatureCollection',
  features: [
    {
      type: 'Feature',
      id: 'a-feature',
      geometry: {
        type: 'Point',
        coordinates: [-74.00597, 40.71427],
      },
    },
  ],
};

class BugReportExample extends React.Component {
  render() {
    return (
      <MapView style={styles.mapView}>
        <Camera centerCoordinate={[-74.00597, 40.71427]} zoomLevel={14} />
        <Images images={{ example: require('../assets/example.png') }} />
        <ShapeSource id={'shape-source-id-0'} shape={features}>
          <SymbolLayer id="symbol-id" style={styles.iconLayer} />
          <CircleLayer
            id="circel-layer"
            style={{
              circleRadius: 3 * (44.0 / 2),
              circleOpacity: 0.4,
              circleStrokeColor: 'green',
              circleStrokeWidth: 2,
            }}
          />
        </ShapeSource>
      </MapView>
    );
  }
}

export default BugReportExample;
andrei-tofan commented 1 year ago

@mfazekas thanks for the detailed example, I've managed to reproduce the issue with your code sample. 1) Tested only in debug mode. 2) The exact image can be found in the example project shared. 3) The image is imported using require. 4) Android device density: 1.75

Shared with you the code on this branch: https://github.com/lineinspector/rnmapbox-react-native-background-geolocation/tree/inconsistent-iconSize

Android: Screenshot_20231009-114530_MyMapProject

iOS: IMG_BD150859DFF4-1

mfazekas commented 1 year ago

The issues should have been fixed 10.1.0-beta.10, can you pls confirm?

andrei-tofan commented 1 year ago

Thank you @mfazekas, the issue was fixed!

snazarkoo commented 1 year ago

Thanks @mfazekas. It works as expected in debug, but not in prod

  1. Device density 1.5;
  2. Image for testing scale-test-icon. It should be an image with different resolutions @2, @3 in assets folder.

Note: The size of image looks the same as with this workaround https://github.com/rnmapbox/maps/issues/3009#issuecomment-1739935698

Prod Screenshot 2023-10-12 at 13 04 44 Debug Screenshot 2023-10-12 at 13 05 03

mfazekas commented 1 year ago

@snazarkoo can you test 10.1.0-beta.11 the production now uses the same Fresco pipeline for prod as used for debug

orca-nazar commented 1 year ago

@mfazekas Perfect, icons are properly sized. Thanks for your work

Noitham commented 11 months ago

Hello, I'm having this exact issue, but the other way around. The icons look oversized on the iOS production builds only. Using - "@rnmapbox/maps": "10.1.3".

Noitham commented 10 months ago

Hey @JasonSanford , did you find a solution for your issue?

I'm still experiencing the issue on version "10.1.3" (magnified HeadingIcon & some SymbolLayers).