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

toDataURL() enlarges image three times #855

Closed
msageryd opened this issue Nov 23, 2018 · 32 comments · Fixed by BlueWallet/BlueWallet#847 or mdfkbtc/veles-mobile-wallet-mdfkbtc#3 · May be fixed by GraoMelo/hathor-wallet-mobile#1
Closed

Comments

@msageryd
Copy link

It seems that toDataURL scales up the png image tree times. The following code should give a 100x100px image file, shouldn't it? I get a 300x300px file.

If this is by design, I'll go ahead and compensate for it. But if it's a bug that will be fixed eventually my compensation will lead to downscaled images when it's fixed.

    return (
      <View
        onLayout={() =>
          this.svg.toDataURL((base64) => {
            fs.writeFile(localPath, base64, 'base64');
          })}
      >
        <Svg
          width={100}
          height={100}
          ref={(ref) => {
            this.svg = ref;
          }}
        >
          <SvgRect
            x={5}
            y={5}
            width={90}
            height={90}
            stroke={'red'}
            strokeWidth={2}
            fill={'yellow'}
          />
        </Svg>
      </View>
    );
@msageryd
Copy link
Author

msageryd commented Nov 30, 2018

I tried to follow the code in the hope to find any clues. But I found nothing. Does anyone know where to look for this odd behaviour?

Since the enlargement is exactly 3 times in both width and height I thought I'd find anything related to RGB. I don't know anything of the PNG format, but I suppose an RGB file is 3 times larger than a B/W file (not taking compression differences into account). Could this factor have been erroneously used when calculating pixel width and height somewhere?

@msand can you push me in the right direction?

Edit:
Actually, I'll take my RGB thought back. There is an Alpha channel in PNGs as well, isn't it? In that case my RGB theory is no good. I guess I had bad luck when I tried to think.

@msand
Copy link
Collaborator

msand commented Dec 7, 2018

@msageryd I think the scaling might be related to this:
https://github.com/react-native-community/react-native-svg/blob/e74ff2b3a3c6aee7c501bd3a230de165b0b15ab6/android/src/main/java/com/horcrux/svg/SvgView.java#L67
It'll probably be different with different devices/emulators depending on the screen density

Could probably take the density into consideration when creating the bitmap, and add a scaling matrix to the canvas, before rendering:
https://github.com/react-native-community/react-native-svg/blob/e74ff2b3a3c6aee7c501bd3a230de165b0b15ab6/android/src/main/java/com/horcrux/svg/SvgView.java#L293-L305

@msand
Copy link
Collaborator

msand commented Dec 7, 2018

Try perhaps something like this:

    String toDataURL() {
        float invScale = 1f / mScale;
        Bitmap bitmap = Bitmap.createBitmap(
                Math.round(getWidth() * invScale),
                Math.round(getHeight() * invScale),
                Bitmap.Config.ARGB_8888);

        Canvas c = new Canvas(bitmap);
        c.scale(invScale, invScale);
        drawChildren(c);
        ByteArrayOutputStream stream = new ByteArrayOutputStream();
        bitmap.compress(Bitmap.CompressFormat.PNG, 100, stream);
        bitmap.recycle();
        byte[] bitmapBytes = stream.toByteArray();
        return Base64.encodeToString(bitmapBytes, Base64.DEFAULT);
    }

@msand
Copy link
Collaborator

msand commented Dec 7, 2018

The scaling call might actually causing another extra scaling, accounting for it in the bitmap size might be enough.

@paulwalczewski
Copy link

@Mand, tried that modified function above and it doesn't seem to work. For testing purposes I updated the function within node modules and then rebuild within xcode - perhaps I did something wrong and that's why I do not see the change (output is still 3x the normal size)?

@msand
Copy link
Collaborator

msand commented Jan 13, 2019

These changes are for android not ios, you need to rebuild in android studio instead of xcode. And, i haven't tested these changes

@paulwalczewski
Copy link

paulwalczewski commented Jan 15, 2019

Thanks @msans.
Since solution was untested and Android-only (and I don't know Swift/C++ to tackle the native iOS one), I've temporarily bypassed the issue by utilising react native PixelRatio.
Basically I call PixelRatio.get() to get the density, then I shrink the image by density value before passing it to toDataURL().
This way when image is wrongly multiplied by this library, it still has desired output size.

Obviously this approach is a workaround which requires re-rendering of svg, so ideally this issue should be solved natively in react-native-svg instead.

@msand
Copy link
Collaborator

msand commented Feb 9, 2019

I think for now at least, unless we want to make a major version jump, if we want to make any changes here we need to keep them backwards compatible. We could perhaps introduce a parameter, to say that you want a downscaled version rather than the scaled bitmap size.

@msageryd
Copy link
Author

msageryd commented Feb 9, 2019

I think a parameter would be great. I think there are two distinct use cases for toDataUrl().

  • Create a bitmap for local use in the app, i.e. show on the same screen it was captured on

  • Create a bitmap for external use (outside of the app) where you need a specific resolution

@msand
Copy link
Collaborator

msand commented Feb 10, 2019

I've implemented support for an optional second argument to the toDataURL method, as an options object parameter having a width and height property. Would this cover your needs?
It's available in the toDataURL branch

https://github.com/react-native-community/react-native-svg/tree/toDataURL

diff: https://github.com/react-native-community/react-native-svg/compare/text-anchor-subtree-calculation...toDataURL?expand=1

Example:

        <Svg
          ref={ele => {
            ele.toDataURL(
              res => {
                console.log(res);
              },
              {
                width: 100,
                height: 100,
              },
            );
          }}
          width="100%"
          height="100%"
          viewBox="0 0 100 100"
        >

@msageryd
Copy link
Author

@msand strikes again =)
To me, this is a perfect solution. Thank you very much. I won't dig into this part of my app in another week or two. Should I use your branch, or will you merge this into master anytime soon?

@msand
Copy link
Collaborator

msand commented Feb 10, 2019

Would be great to validate the implementation before merging and releasing it at least. I haven't opened the output or drawn it on screen yet, only made sure that it gives output on both ios and android so far.

@msageryd
Copy link
Author

Ok, sure.
I'm in the middle of a release, so I won't have time to try out your branch until the end of the week. I'll ping back here as soon as I've tested.

@msand
Copy link
Collaborator

msand commented Mar 11, 2019

@msageryd Can you try with the develop branch? I think i have it working correctly now:

import React, { Component } from 'react';
import { View, Image } from 'react-native';
import Svg, { Path } from 'react-native-svg';

class SvgComponent extends Component {
  state = {};
  onRef = ref => {
    if (ref && !this.state.base64) {
      ref.toDataURL(
        base64 => {
          this.setState({ base64 });
        },
        { width: 200, height: 200 },
      );
    }
  };
  render() {
    return (
      <View style={{ flex: 1 }}>
        <Svg
          ref={this.onRef}
          viewBox="0 0 400 400"
          width="100%"
          {...this.props}
          height="250"
        >
          <Path fill="none" stroke="#00f" d="M1 1h398v398H1z" />
          <Path
            d="M100 100h200L200 300z"
            fill="red"
            stroke="#00f"
            strokeWidth={3}
          />
        </Svg>
        <View
          style={{
            width: '100%',
            flex: 1,
            marginTop: 5,
            borderWidth: 1,
            alignItems: 'center',
            justifyContent: 'center',
          }}
        >
          {this.state.base64 && (
            <Image
              source={{ uri: `data:image/png;base64,${this.state.base64}` }}
              style={{ width: 250, height: 250 }}
            />
          )}
        </View>
      </View>
    );
  }
}

export default SvgComponent;

@msand
Copy link
Collaborator

msand commented Mar 11, 2019

Btw, I've added support for calling toDataUrl already in the ref function, don't have to wait for onLayout in javascript anymore.

@msageryd
Copy link
Author

Sorry for the delay on my side, I'm swamped for the moment..
Trying this out right now, but It's not working for me.

Everything seems to work as usual if I omit the size object in the toDataUrl call.

When I apply a size object the following happens:

  1. The svg image gets cropped, not resized
  2. The size is still 3x the size I enter (in simulator with iPhoneX device)

@msageryd
Copy link
Author

More info..

  • If I specify a viewBox to cover the size of the svg I get rit of the cropping problem
    N.B.1. If I don't supply your new size parameter to toDataURL there is no need for a viewBox
    N.B.2. There seems to be no need for width='100% and size='100%, is 100% implied?

  • The output size is still 3x the size I specify in the size parameter

@msageryd
Copy link
Author

Btw, I've added support for calling toDataUrl already in the ref function, don't have to wait for onLayout in javascript anymore.

Kind of works..

  • If I draw a simple rect in the Svg, this rect will be included in the base64 output.
  • More advanced rendering is not included (In my case a map of special svg-components is missing). Maybe because of the async nature of .map()?

If I call toDataUrl in onLayout, all components are included in the base64 output.

@msageryd
Copy link
Author

@msand I'm sorry, but this version gets me into more trouble. Somehow my since long trusted rendering loop is now delayed at least one tick. My SVG objects are not rendered until I pan the screen, i.e. forces a re-render. The first render doesn't seem to happen.

This part in my app has been stable for a long time, so I suspect something fishy is going on in your develop branch. Do you have any clues, or should I try to isolate the problem (might take a while)?

@msand
Copy link
Collaborator

msand commented Mar 13, 2019

This sounds quite strange, relatively little has changed in the code since v9.2.4
There's three main changes, the toDataUrl with options, vector-effect: non-scaling-stroke, work on text alignment, and then there has been some minor js optimizations. My first suspect would probably be the text logic changes.
But nothing should have changed when the rendering happens. Possibly there is more reuse of memory and objects, causing some setter not to call invalidate in case the values are equal already. But seems that should've been true from before as well.
Would you be able to create a minimal reproduction of the issues?

@msand
Copy link
Collaborator

msand commented Mar 13, 2019

I'll probably need to port the logic from android to ios, for handling the case where not all your content is rendered yet. Android needed it even for small amounts of content, the view creation commands would be in the queue when toDataUrl gets called, and even putting it in the end of the queue then wasn't enough. So it needs to be queued to happen only when the rendering is actually done.

@msageryd
Copy link
Author

In that case I suspect that something was lingering in my cache again. The cache seems to bite me quite often when I jump in and out of different versions libraries. I've just found some new magic that might help. I'll give it a go later today and report back here.

The new magic:
rm -rf $TMPDIR/metro-* && rm -rf $TMPDIR/haste-map-*

It seems that the cache is problematic for more than me. I really hope that this can be addressed in RN going forward. https://gist.github.com/jarretmoses/c2e4786fd342b3444f3bc6beff32098d

@msand
Copy link
Collaborator

msand commented Mar 18, 2019

Can you try with v9.3.5 ?

@msand
Copy link
Collaborator

msand commented Mar 21, 2019

@msageryd There were some issues with text layout I've mostly fixed now, not perfectly spec conformant yet, but probably good enough for most cases. Can you try v9.3.6 ?

@msageryd
Copy link
Author

I feel guilty for not putting time into this while you are working so hard. Right now I'm in the middle of a release and I'm working around the clock. If all goes well I've upgraded both my app and the servers by tonight.

Right after this release, testing your fixes is on top of my list.

@msageryd
Copy link
Author

At last I have had some time for this.

  • Still the same x3 problem with output size (on iPhoneX simulator).

  • If the {width,height} argument is not used, there will be no dataUrl. I tried both directly in the ref event and in onLayout. Is the output size now a mandatory input to toDataUrl? It was optional before.

  • Still not possible to call toDataUrl directly from the ref event. Have to wait until onLayout. But this seems to behave a little different now. Shapes are rendered, but the are not yet scaled as per the transform inside of a G element. Although I think it's perfectly fine to wait until onLayout.

pseudo code:
The output from renderShapes() is rendered, but the transform is only applied if I wait until onLayout.

<Svg>
  <G transform={{scale: 2}}>
    {this.renderShapes()}
  </G>
</Svg>

@msageryd
Copy link
Author

BTW, the base64 output includes linebreaks, but only sometimes. Is the linebreaks inserted when the files are really big? My test file is about 7000x5000px. I think that the linebreaks should be fine in general, but maybe iOS has problems with this? Anyway, I have to strip them out in order to get a working png file.

//Strip the header in order to get a png file
let fileData = base64.replace(/^data:image\/\w+;base64,/, '');

//strip line breaks to get the file working all the time
fileData = fileData.replace(/\r?\n|\r/g, '');

//write the png file
RNFetchBlob.fs.createFile(docPath, fileData, 'base64');

@msageryd
Copy link
Author

msageryd commented Apr 1, 2019

@msand please tell me if you want me to test this in any other way.

@msand
Copy link
Collaborator

msand commented Oct 4, 2019

Can you try with this in the latest version?

App.js

import React, {Component} from 'react';
import {View, Image, StyleSheet} from 'react-native';
import Svg, {Path} from 'react-native-svg';

class SvgComponent extends Component {
  state = {};
  onRef = ref => {
    if (ref && !this.state.base64) {
      ref.toDataURL(
        base64 => {
          this.setState({base64});
        },
        {width: 200, height: 200},
      );
    }
  };
  render() {
    return (
      <View style={styles.container}>
        <Svg
          width="100%"
          height="250"
          ref={this.onRef}
          viewBox="0 0 400 400"
          {...this.props}>
          <Path fill="none" stroke="#00f" d="M1 1h398v398H1z" />
          <Path
            d="M100 100h200L200 300z"
            fill="red"
            stroke="#00f"
            strokeWidth={3}
          />
        </Svg>
        <View style={styles.imageView}>
          {this.state.base64 && (
            <Image
              source={{uri: `data:image/png;base64,${this.state.base64}`}}
              style={styles.image}
            />
          )}
        </View>
      </View>
    );
  }
}

const styles = StyleSheet.create({
  image: {width: 250, height: 250},
  container: {flex: 1},
  imageView: {
    flex: 1,
    marginTop: 5,
    width: '100%',
    borderWidth: 1,
    alignItems: 'center',
    justifyContent: 'center',
  },
});

export default SvgComponent;

@donataskairys
Copy link

It is still an issue on iOS. The size still gets tripled.

It works fine on android. Tested with v9.13.3.

@msand
Copy link
Collaborator

msand commented Nov 6, 2019

@damnhipster @msageryd Can you try with the latest commit from the develop branch?

@msand msand closed this as completed in 45b0859 Dec 9, 2019
msand pushed a commit that referenced this issue Dec 9, 2019
## [9.13.4](v9.13.3...v9.13.4) (2019-12-09)

### Bug Fixes

* initialize PathView with empty path ([45192bd](45192bd))
* **ios:** Fix image size when calling getDataURL with bounds. fixes [#855](#855) ([45b0859](45b0859))
@msand
Copy link
Collaborator

msand commented Dec 9, 2019

🎉 This issue has been resolved in version 9.13.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment