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

Incorrect Image top/bottom alignment xMidYMin/xMidYMax between same iOS/Android SVG #1138

Closed
oleksandr-dziuban opened this issue Oct 7, 2019 · 18 comments · Fixed by BlueWallet/BlueWallet#847 or mdfkbtc/veles-mobile-wallet-mdfkbtc#3 · May be fixed by GraoMelo/hathor-wallet-mobile#1
Labels
Missing repro This issue need minimum repro scenario

Comments

@oleksandr-dziuban
Copy link

oleksandr-dziuban commented Oct 7, 2019

Bug

If preserveAspectRatio used to align image to top/bottom on iOS and Android we can see diametrically opposite alignment. Looks like Y axis is incorrect on one of the platforms.

Environment info

React native info output:

 System:
    OS: macOS 10.14.6
    CPU: (8) x64 Intel(R) Core(TM) i7-7700HQ CPU @ 2.80GHz
    Memory: 23.68 MB / 16.00 GB
    Shell: 3.2.57 - /bin/bash
  Binaries:
    Node: 12.4.0 - /usr/local/bin/node
    Yarn: 1.19.0 - /usr/local/bin/yarn
    npm: 6.9.0 - /usr/local/bin/npm
    Watchman: 4.9.0 - /usr/local/bin/watchman
  SDKs:
    iOS SDK:
      Platforms: iOS 13.0, DriverKit 19.0, macOS 10.15, tvOS 13.0, watchOS 6.0
    Android SDK:
      API Levels: 23, 25, 26, 27, 28
      Build Tools: 27.0.3, 28.0.3
      System Images: android-24 | Google Play Intel x86 Atom, android-25 | Google APIs Intel x86 Atom, android-26 | Google APIs Intel x86 Atom, android-27 | Google APIs Intel x86 Atom, android-27 | Google Play Intel x86 Atom, android-28 | Google APIs Intel x86 Atom, android-28 | Google Play Intel x86 Atom, android-29 | Google APIs Intel x86 Atom
  IDEs:
    Android Studio: 3.5 AI-191.8026.42.35.5791312
    Xcode: 11.0/11A420a - /usr/bin/xcodebuild
  npmPackages:
    @react-native-community/cli: 2.9.0 => 2.9.0 
    react: 16.9.0 => 16.9.0 
    react-native: 0.61.2 => 0.61.2 
  npmGlobalPackages:
    react-native-cli: 2.0.1
    react-native-create-library: 3.1.2
    react-native: 0.61.2 => 0.61.2 

Library version: 9.9.5

Steps To Reproduce

<Image 
  x="0" y="100" width="50 height="150" source={ { uri: ... } } 
  preserveAspectRatio="xMidYMin meet" />
<Image
  x="200" y="100" width="50" height="150" source={ { uri: ... } }
  preserveAspectRatio="xMidYMax meet" />

Guys, please check, thanks a lot!

@msand
Copy link
Collaborator

msand commented Oct 7, 2019

Can you make a complete reproduction?

@msand msand added the Missing repro This issue need minimum repro scenario label Oct 7, 2019
@oleksandr-dziuban
Copy link
Author

oleksandr-dziuban commented Oct 7, 2019

Can you make a complete reproduction?

In my app I have dynamic code for images, a lot of code is needed to paste here.
But I will try to prepare small reproduction here, 5 min

@oleksandr-dziuban
Copy link
Author

oleksandr-dziuban commented Oct 7, 2019

@msand Done, please check

import React from 'react';
import { View } from 'react-native';
import Svg, { G, Image, Rect } from 'react-native-svg';

const App = () => {
  return (
    <View style={{ flex: 1, backgroundColor: 'orange' }}>
      <Svg width="100%" height="600" viewBox="0 0 400 600" style={{ backgroundColor: 'green' }}>
        <G>
          <Rect
            x="0"
            y="100"
            width="120"
            height="400"
            stroke="blue"
            fill="transparent"
          />
          <Image
            x="0"
            y="100"
            width="120"
            height="400"
            href={{ uri: 'https://cdn.pixabay.com/photo/2018/12/04/22/38/road-3856796__340.jpg' }}
            preserveAspectRatio="xMidYMin meet" />
          <Rect
            x="200"
            y="100"
            width="120"
            height="400"
            stroke="blue"
            fill="transparent"
          />
          <Image
            x="200"
            y="100"
            width="120"
            height="400"
            href={{ uri: 'https://cdn.pixabay.com/photo/2018/12/04/22/38/road-3856796__340.jpg' }}
            preserveAspectRatio="xMidYMax meet" />
        </G>
      </Svg>
    </View>
  );
};

export default App;

@oleksandr-dziuban
Copy link
Author

oleksandr-dziuban commented Oct 7, 2019

Updated with Rectangles for better visibility

@msand
Copy link
Collaborator

msand commented Oct 7, 2019

I'm getting the same output on ios, android and web

@oleksandr-dziuban
Copy link
Author

oleksandr-dziuban commented Oct 7, 2019

I'm getting the same output on ios, android and web

Which library version do you use?
I use 9.9.5

@msand
Copy link
Collaborator

msand commented Oct 7, 2019

Actually, ios seems to have opposite, this is the develop branch / latest version

@oleksandr-dziuban
Copy link
Author

Actually, ios seems to have opposite, this is the develop branch / latest version

on react-native-svg@9.9.5 I see that iOS has incorrect top/bottom alignment. Looks like YMax and YMin are opposite to each other, Android and Web works fine.

@msand
Copy link
Collaborator

msand commented Oct 7, 2019

@oleksandr-dziuban can you try the latest commit from the develop branch? c69e9e2

@oleksandr-dziuban
Copy link
Author

oleksandr-dziuban commented Oct 7, 2019

@msand Thank you for this change, but I can try only tomorrow, is it possible to just check it on my reproduction example?

@oleksandr-dziuban
Copy link
Author

oleksandr-dziuban commented Oct 7, 2019

@msand if it works correctly there it will work fine in production project too

@msand
Copy link
Collaborator

msand commented Oct 7, 2019

at least works fine for me locally

@oleksandr-dziuban
Copy link
Author

You could deploy new version then, in your commit I see -1 * <y axis value> - it is definitely what I saw on iOS: Y Axis === - Y Axis

@oleksandr-dziuban
Copy link
Author

Thanks a lot for your help @msand !

@oleksandr-dziuban
Copy link
Author

@msand I have checked you commit as npm dependency, but it looks the same, maybe it doesn't install your version correctly with git+https://github.com/react-native-community/react-native-svg.git#c69e9e2

Could you please deploy new version to npm?

@msand
Copy link
Collaborator

msand commented Oct 9, 2019

Hmm, interesting, did you try cleaning the build and rebuilding?
There's still some work remaining in the develop branch, or at least I know of some bugs / spec conformance issues in new functionality.

@oleksandr-dziuban
Copy link
Author

oleksandr-dziuban commented Oct 9, 2019

@msand Yes, I tried, anyway problem in package installation process, at the end it throws error, because develop branch contains some post install scripts which are failing on my machine.

No worries, it is not urgent, I applied a small hotfix in my app to invert Y axis for iOS. Let's publish npm package when you finish all your required work.

Thanks a lot for your help

@msand msand closed this as completed in c69e9e2 Oct 19, 2019
msand pushed a commit that referenced this issue Oct 19, 2019
# [9.12.0](v9.11.1...v9.12.0) (2019-10-19)

### Bug Fixes

* handle setting transform attribute on clipPath, fixes [#1152](#1152) ([73b21d1](73b21d1))
* improve handling of transform attribute on clipPath, fixes [#1152](#1152) ([3aa126e](3aa126e))
* **ios:** backwards compatible RCTImageLoader.h handling fixes [#1141](#1141) ([3c22c97](3c22c97))
* **ios:** clipPath rendering, fixes [#1131](#1131) ([2534537](2534537))
* **ios:** deprecation of RCTImageLoader fixes [#1141](#1141) ([5452144](5452144))
* **ios:** fix changes in color/currentColor/tintColor, fixes [#1151](#1151) ([0c7e94d](0c7e94d))
* **ios:** image viewBox opposite handling of y alignment, fixes [#1138](#1138) ([c69e9e2](c69e9e2))
* **js:** allow setting stopColor/Opacity/Offset using styles, fix [#1153](#1153) ([5984e06](5984e06))
* getPointAtLength signature ([2c57af2](2c57af2))
* getScreenCTM calculation ([5c5072d](5c5072d))
* improve native method spec conformance ([c63f9e2](c63f9e2))
* improve types for getBBox ([cecde7d](cecde7d))
* prepare script ([9a3dc4e](9a3dc4e))
* **ios:** memory leak in tspan, fixes [#1073](#1073) ([974f3a8](974f3a8))
* fix native methods spec conformance ([ecedb21](ecedb21))
* Make native methods synchronous ([8ce7611](8ce7611))
* refine types for matrix helpers ([409af91](409af91))
* refine types for matrix helpers ([7a3f867](7a3f867))
* **android:** defineMarker/getDefinedMarker storage ([e6eda84](e6eda84))
* **android:** native method scaling and getScreenCTM offset ([f3e0b19](f3e0b19))
* native method signatures web compatibility / spec conformance ([8687a3d](8687a3d))
* **ios:** optimize extractPathData, clear PathMeasure when no textPath ([df69c26](df69c26))

### Features

* **flow:** add flowgen to generate flow types from typescript, [#1125](#1125) ([fcd66fb](fcd66fb))
* implement getBBox, getCTM, getScreenCTM ([f13d54a](f13d54a))
* implement isPointInStroke ([2ba64df](2ba64df))
* initial implementation of isPointInFill ([203e53b](203e53b))
* support using native methods using promises instead of callbacks ([c28499b](c28499b))
* **android:** implement getTotalLength and getPointAtLength ([cd667d0](cd667d0))
* **ios:** implement getTotalLength and getPointAtLength ([78c4f20](78c4f20))
@msand
Copy link
Collaborator

msand commented Oct 19, 2019

🎉 This issue has been resolved in version 9.12.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@msand msand added the released label Oct 19, 2019
JackWillie added a commit to JackWillie/react-native-svg that referenced this issue Nov 27, 2022
# [9.12.0](software-mansion/react-native-svg@v9.11.1...v9.12.0) (2019-10-19)

### Bug Fixes

* handle setting transform attribute on clipPath, fixes [#1152](software-mansion/react-native-svg#1152) ([73b21d1](software-mansion/react-native-svg@73b21d1))
* improve handling of transform attribute on clipPath, fixes [#1152](software-mansion/react-native-svg#1152) ([3aa126e](software-mansion/react-native-svg@3aa126e))
* **ios:** backwards compatible RCTImageLoader.h handling fixes [#1141](software-mansion/react-native-svg#1141) ([3c22c97](software-mansion/react-native-svg@3c22c97))
* **ios:** clipPath rendering, fixes [#1131](software-mansion/react-native-svg#1131) ([2534537](software-mansion/react-native-svg@2534537))
* **ios:** deprecation of RCTImageLoader fixes [#1141](software-mansion/react-native-svg#1141) ([5452144](software-mansion/react-native-svg@5452144))
* **ios:** fix changes in color/currentColor/tintColor, fixes [#1151](software-mansion/react-native-svg#1151) ([0c7e94d](software-mansion/react-native-svg@0c7e94d))
* **ios:** image viewBox opposite handling of y alignment, fixes [#1138](software-mansion/react-native-svg#1138) ([c69e9e2](software-mansion/react-native-svg@c69e9e2))
* **js:** allow setting stopColor/Opacity/Offset using styles, fix [#1153](software-mansion/react-native-svg#1153) ([5984e06](software-mansion/react-native-svg@5984e06))
* getPointAtLength signature ([2c57af2](software-mansion/react-native-svg@2c57af2))
* getScreenCTM calculation ([5c5072d](software-mansion/react-native-svg@5c5072d))
* improve native method spec conformance ([c63f9e2](software-mansion/react-native-svg@c63f9e2))
* improve types for getBBox ([cecde7d](software-mansion/react-native-svg@cecde7d))
* prepare script ([9a3dc4e](software-mansion/react-native-svg@9a3dc4e))
* **ios:** memory leak in tspan, fixes [#1073](software-mansion/react-native-svg#1073) ([974f3a8](software-mansion/react-native-svg@974f3a8))
* fix native methods spec conformance ([ecedb21](software-mansion/react-native-svg@ecedb21))
* Make native methods synchronous ([8ce7611](software-mansion/react-native-svg@8ce7611))
* refine types for matrix helpers ([409af91](software-mansion/react-native-svg@409af91))
* refine types for matrix helpers ([7a3f867](software-mansion/react-native-svg@7a3f867))
* **android:** defineMarker/getDefinedMarker storage ([e6eda84](software-mansion/react-native-svg@e6eda84))
* **android:** native method scaling and getScreenCTM offset ([f3e0b19](software-mansion/react-native-svg@f3e0b19))
* native method signatures web compatibility / spec conformance ([8687a3d](software-mansion/react-native-svg@8687a3d))
* **ios:** optimize extractPathData, clear PathMeasure when no textPath ([df69c26](software-mansion/react-native-svg@df69c26))

### Features

* **flow:** add flowgen to generate flow types from typescript, [#1125](software-mansion/react-native-svg#1125) ([fcd66fb](software-mansion/react-native-svg@fcd66fb))
* implement getBBox, getCTM, getScreenCTM ([f13d54a](software-mansion/react-native-svg@f13d54a))
* implement isPointInStroke ([2ba64df](software-mansion/react-native-svg@2ba64df))
* initial implementation of isPointInFill ([203e53b](software-mansion/react-native-svg@203e53b))
* support using native methods using promises instead of callbacks ([c28499b](software-mansion/react-native-svg@c28499b))
* **android:** implement getTotalLength and getPointAtLength ([cd667d0](software-mansion/react-native-svg@cd667d0))
* **ios:** implement getTotalLength and getPointAtLength ([78c4f20](software-mansion/react-native-svg@78c4f20))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Missing repro This issue need minimum repro scenario
Projects
None yet
2 participants