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

Distortion When Animating Height of Auto Layout LineChartView #3811

Closed
staykids opened this issue Jan 9, 2019 · 7 comments
Closed

Distortion When Animating Height of Auto Layout LineChartView #3811

staykids opened this issue Jan 9, 2019 · 7 comments

Comments

@staykids
Copy link

staykids commented Jan 9, 2019

What did you do?

I have a simple LineChartView constrained to a superview using Auto Layout constraints for its top, left, and right anchors and its height. I tried to animate a LineChartView's height constraint.

What did you expect to happen?

When I try to animate its NSLayoutConstraint height constant, I expected all the data points in the chart to animate without distortion into the LineChartView's new dimensions.

What happened instead?

When I animate its NSLayoutConstraint height constant, all the data points within the line chart are instead distorted while it animates to the new height. Animating a line chart's dimensions results in visual distortion, as seen in the included GIF. This visual distortion can be reproduced with any LineChartView.

It seems like instead of redrawing the points during animation, it's simply image snapshotting and stretching the after state resulting in the distortion.

linechart_animation

Charts Environment

Charts version/Branch/Commit Number: 3.2.1
Xcode version: 10.1
Swift version: 4.2
Platform(s) running Charts: iOS 12.1
macOS version running Xcode: 10.14.2

Demo Project

This visual distortion can be reproduced with any LineChartView. Here's the code used to animate the height constraint change:

func animate(expanded: Bool) {
	self.layoutIfNeeded()
	/// Height Constraint is the `LineChartView`'s height constraint
	heightConstraint?.constant = expanded ? 400 : 200
	UIView.animate(withDuration: 1.0, animations: {
		self.layoutIfNeeded()
	})
}

I'm happy to provide any additional details. Thanks in advance!

@liuxuan30
Copy link
Member

liuxuan30 commented Jan 14, 2019

I'm not sure if it's a bug or by nature and we never thought about your scenario..

The library has an observer on the chart view's frame and bounds and will update the chart properties:

        if keyPath == "bounds" || keyPath == "frame"
        {
            let bounds = self.bounds
            
            if (_viewPortHandler !== nil &&
                (bounds.size.width != _viewPortHandler.chartWidth ||
                bounds.size.height != _viewPortHandler.chartHeight))
            {
                _viewPortHandler.setChartDimens(width: bounds.size.width, height: bounds.size.height)
                
                // This may cause the chart view to mutate properties affecting the view port -- lets do this
                // before we try to run any pending jobs on the view port itself
                notifyDataSetChanged()

                // Finish any pending viewport changes
                while (!_viewportJobs.isEmpty)
                {
                    let job = _viewportJobs.remove(at: 0)
                    job.doJob()
                }
            }
        }

so basically it will redraw the chart itself by calling notifyDataSetChanged(). But in your GIF, it seems the chart view already got the final frame and being rendered already, and then started the stretching from distortion to normal.

Can you try to mark out notifyDataSetChanged() or call it after the view's frame is settled? Or check this observer's frame update if it's smooth.

@jjatie any suggestion?

@staykids
Copy link
Author

staykids commented Jan 14, 2019

@liuxuan30 Thanks for the reply, Xuan.

Can you try to mark out notifyDataSetChanged() or call it after the view's frame is settled?

Commenting out this notifyDataSetChanged() call in func observeValue(...) in ChartViewBase doesn't seem to resolve the issue – it produces the same distorted animation as in my initial comment.

Or check this observer's frame update if it's smooth.

Inspecting ChartViewBase's frame's values in func observeValue(...) shows just the "new" final frame/bounds, not a smooth update of all the values in-between the old and new frame/bounds. Is that what you were asking?

self.addObserver(self, forKeyPath: "bounds", options: .new, context: nil)
self.addObserver(self, forKeyPath: "frame", options: .new, context: nil)

@liuxuan30
Copy link
Member

liuxuan30 commented Jan 16, 2019

shows just the "new" final frame/bounds, not a smooth update of all the values in-between the old and new frame/bounds

I guess this is it. Only have the old and new value seems consistent with your GIF. I guess you have to find a way to animate the frame change.

Currently I don't think we could fix it, mostly due to no time and I think we could only fix it if we were using CoreAnimation. Or you could try figure out a new animator job on your side?

The animation we did is based on CoreGraphics, which is limited I guess because we have to implement everything by ourselves. You have to find a way if you could write your own animation based on CG. But I would suggest you reconsider if you really need the animation, or if any workarounds.

@staykids
Copy link
Author

@liuxuan30 I see. I'm happy to try my hand at creating a custom AnimatedViewPortJob. This could leverage the animationUpdate() ticks to get the in-between frame values that are needed, then incrementally per tick update the ChartViewBase's frame towards the desired dimensions.

So maybe something like:

  1. Create ViewSizeAnimatedViewPortJob
  2. On each animationUpdate()tick:
    1. Update the view's (ChartViewBase) frame, incrementing it as a factor of its current value, the desired value, and the current phase
    2. Call view.notifyDataSetChanged()

I'm still relatively new to the library, but does that make sense? Any other guidance, recommendations, or gotchas if I were to go this route?

@liuxuan30
Copy link
Member

liuxuan30 commented Jan 22, 2019

I'm concerned the performance with your (i) and (ii). Keep changing the frame and redrawing sounds like a heavy burden.

I'm thinking if you can first capture a snapshot of your view and start scaling it and then replace a new real chart. I saw similar implementation in open source UICollectionViewCell dragging and reordering feature before Apple publish it.

Animating the frame is no easy job.

@staykids
Copy link
Author

staykids commented Jan 23, 2019

@liuxuan30 Thanks for the guidance. If the aspect ratio of the snapshots (initial and destination) differ though (as they do in my use case), this snapshot approach would also produce a distorted look during animation, so I don't believe I can use that approach here.

@liuxuan30
Copy link
Member

as long as you can know the ratio before the animation, you can use a offline render to get the image snapshot. otherwise you have to think about another one :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants