st0ffern / react-native-image-cropper

Crop your images with zoom and pan
https://www.npmjs.com/package/react-native-image-cropper
MIT License
158 stars 67 forks source link

Performance issues #63

Closed isaachinman closed 6 years ago

isaachinman commented 6 years ago

Hi there, and thanks for a really cool component.

I just installed this in my RN project, and played around with it both on an iOS simulator and a physical Android device. In both cases I noticed serious laggy-ness, gestures being missed, etc. Panning animation is nowhere near 60fps and is choppy.

How much of this is due to this component vs gl-react? I was using 2.x versions of both gl-react and gl-react-native, could that be a contributing factor? Is anyone else noticing performance issues like this?

Sorry to open such a vague issue, just trying to decide if this component is production-viable.

st0ffern commented 6 years ago

Sounds like you have a bug in your code. It is 99% shure that it is related to state changing while you move or use features in the gl-react component, making a multiple reload of the component every time

isaachinman commented 6 years ago

Nope:

import React from 'react'
import PropTypes from 'prop-types'

import { screenUtils } from 'utils/nav'

import { ImageCrop } from 'react-native-image-cropper'

@screenUtils
export default class EditImage extends React.Component {

  static screenTitle = 'Edit image'

  render() {

    return (
      <ImageCrop
        ref={x => this.cropper = x}
        image={this.props.uri}
        cropHeight={300}
        cropWidth={300}
        zoom={50}
        maxZoom={80}
        minZoom={20}
        panToMove
        pinchToZoom
        type='jpg'
      />
    )
  }
}

As soon as I noticed issues I made sure to strip my use case back as far as possible to match the most basic examples. Perhaps it would be useful after all if I can reproduce this in a simple repo.

st0ffern commented 6 years ago

what does screenUtils do?

Shure, make a repo if you can reproduce the problem

isaachinman commented 6 years ago

Ah, it's just a HOC that adds a screen title in construction:

export default (() => WrappedComponent =>
  class extends WrappedComponent {
    constructor(props) {
      super(props)

      // Add `static screenTitle` functionality
      if (WrappedComponent.screenTitle) {
        props.navigator.setTitle({
          title: WrappedComponent.screenTitle,
        })
      }
    }
  })()
st0ffern commented 6 years ago

Try setting this.props.uri to a state in the EditImage component

  constructor(props) {
    super(props);

    this.state = {
      uri: this.props.uri
    };
  }

and pass state.uri to the component. see if that helps.

st0ffern commented 6 years ago

...?

isaachinman commented 6 years ago

Hi @stoffern, I've decided to move away from this component in my current project and just build a custom solution. I'm not sure what difference passing the uri from state vs props would make.