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

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

Closed
paulschreiber opened this issue Aug 24, 2023 · 20 comments · Fixed by #3101 or #3103
Closed

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

paulschreiber opened this issue Aug 24, 2023 · 20 comments · Fixed by #3101 or #3103
Labels
Needs: Author Feedback Needs: Repro This issue could be improved with a clear list of steps to reproduce the issue.

Comments

@paulschreiber
Copy link
Contributor

paulschreiber commented Aug 24, 2023

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: #68

@shrouxm
Copy link
Contributor

shrouxm commented Aug 24, 2023

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
Copy link
Contributor

andrei-tofan commented Aug 29, 2023

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
Copy link
Contributor

shrouxm commented Aug 31, 2023

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
Copy link
Contributor

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
Copy link
Contributor

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/[email protected]?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
Copy link

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
Copy link

Vednus commented Sep 27, 2023

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
Copy link

snazarkoo commented Sep 28, 2023

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
Copy link
Contributor

mfazekas commented Sep 28, 2023

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/[email protected]?platform=ios&hash=2557a9ee1ee49ac29dff472336c97a87',
  scale: 3 }

See also:
#68

See also the fixes:
#191

@snazarkoo
Copy link

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
Copy link
Contributor

mfazekas commented Oct 3, 2023

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 mfazekas added Needs: Repro This issue could be improved with a clear list of steps to reproduce the issue. Needs: Author Feedback labels Oct 3, 2023
@mfazekas
Copy link
Contributor

mfazekas commented Oct 3, 2023

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
Copy link
Contributor

andrei-tofan commented Oct 9, 2023

@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
Copy link
Contributor

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

@andrei-tofan
Copy link
Contributor

Thank you @mfazekas, the issue was fixed!

@snazarkoo
Copy link

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 #3009 (comment)

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

@mfazekas
Copy link
Contributor

@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
Copy link
Contributor

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

@Noitham
Copy link

Noitham commented Dec 21, 2023

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
Copy link

Noitham commented Jan 2, 2024

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs: Author Feedback Needs: Repro This issue could be improved with a clear list of steps to reproduce the issue.
Projects
None yet
9 participants