zepojo / UPCarouselFlowLayout

A fancy carousel flow layout for UICollectionView on iOS.
MIT License
1.67k stars 235 forks source link

Deceleration is instantaneous at times #18

Open iwasrobbed opened 7 years ago

iwasrobbed commented 7 years ago

Have you come across this before?

I tested both on device and simulator and see it both places. I also tested with different deceleration rates and it doesn't seem to change anything

I'm using a vanilla UICollectionView w/ vertical scrolling and this as the layout:

final class SwipeableLessonLayout: UPCarouselFlowLayout {

    // MARK: - Instantiation

    override init() {
        super.init()

        itemSize = CGSize(width: UIScreen.main.bounds.size.width, height: LessonSectionCell.cellHeight)
        scrollDirection = .vertical
        minimumLineSpacing = 0
        minimumInteritemSpacing = 0

        sideItemScale = 1
        sideItemAlpha = 0.4
        spacingMode = UPCarouselFlowLayoutSpacingMode.fixed(spacing: 0)
    }

    // MARK: - Unsupported Initializers

    required init?(coder aDecoder: NSCoder) { fatalError("init(coder:) has not been implemented") }

}
dangthaison91 commented 7 years ago

@iwasrobbed You meant you can only swipe one by one not able to swipe multiple items at once like this:

https://dribbble.com/shots/3132598-Carousel

iwasrobbed commented 7 years ago

@dangthaison91 No; the paging behavior is fine. I mean the scroll view underlying the collection view should always bounce nicely and come to a slow stop.

Unfortunately, the way this is implemented is causing it to sometimes come to an instantaneous stop and it skips the deceleration animation all together. I haven't been able to debug it yet so wanted to check if this was known behavior

dangthaison91 commented 7 years ago

Will changing ScrollDecelerate help?

iwasrobbed commented 7 years ago

@dangthaison91 In the original issue I wrote:

I also tested with different deceleration rates and it doesn't seem to change anything

matthew-reilly commented 7 years ago

Seeing this as well. I think the issue is in the targetContentOffset function.

matthew-reilly commented 7 years ago

Yup, this only happens when withScrollingVelocity is non-nil.

matthew-reilly commented 7 years ago

@iwasrobbed I've been playing around with this and found something that may help. Manually calling: collectionView.setContentOffset(targetContentOffset, animated: true) at the end of the targetContentOffset if the velocity > 0 seems to make things smooth.

I think setting offsets + velocity mucks up collectionviews, but I could be wrong.

Playing around with the velocity/offset could provide a way to make this work flawlessly. Right now with this fix, if I flick it - it advances to the next page smoothly.

iwasrobbed commented 7 years ago

@matthew-reilly That's unfortunately jittery for me when I don't flick it hard enough to advance to the next page. It does seem to be an issue w/ velocity and offset though.

The thing I don't understand about this function is that it's only returning a point, but does nothing with the velocity. I'm guessing they recalculate the velocity based on what point you return.

By calling setContentOffset, I think you're just overriding whatever effect targetContentOffset would otherwise have so that would point more towards using the regular UIScrollViewDelegate methods instead of this one.

Still haven't had time to dig into this, but I appreciate the help

matthew-reilly commented 7 years ago

Yeah, it's not perfect. I think the solution may lie the CV itself. Per: http://stackoverflow.com/questions/15800111/uiscrollview-revert-to-starting-position-without-snaps

kafejo commented 7 years ago

Any update on this?

kafejo commented 7 years ago

I have been able to improve the scrolling behaviour by taking the velocity into account when calculating targetContentOffset

let proposedContentOffsetCenterOrigin = (isHorizontal ? proposedContentOffset.x + (proposedContentOffset.x * velocity.x) : proposedContentOffset.y + (proposedContentOffset.y * velocity.y)) + midSide

It's much better but sometimes it still jumps.

punithbm commented 7 years ago

@iwasrobbed did you get any solution.. i'm also facing this issue.. can you help me

punithbm commented 7 years ago

@kafejo did you find any better solution for this

kafejo commented 7 years ago

I've ended up with the solution I described above. It works much better.

AntonTheDev commented 6 years ago

So...... based on default behavior of scrollviews, if I flick my finger on a scrollview that is already scrolling, it will then accelerates, and goes faster.

What I noticed is that the proposedTargetOffset appears to think it's supposed to overshoot by a lot, probably based on the velocity it is going! The the scrollview appears to be compounding the velocity for every flick, by adding it to the velocity it already thinks the scrollview is going, and then overshoots into a monster bounce at the end when it reaches the boundary of it's content size.

Here are some interesting readings I got while flicking faster and faster.

Index -- proposedOffset ------------ nextPageOffset 1 ------ (0.0, 2542.0) ----------------- (0.0, 812.0) ------- Flick 1 2 ------ (0.0, 4292.0) ---------------- (0.0, 1624.0) ------ Flick 2 3 ------ (0.0, 4735.0) ---------------- (0.0, 2436.0) ------ Flick 3 4 ------ (0.0, 4872.0) ---------------- (0.0, 3248.0)
5 ------ (0.0, 4872.0) ---------------- (0.0, 4060.0) 6 ------ (0.0, 4872.0) ---------------- (0.0, 4872.0) 6 ------ (0.0, 4872.0) ---------------- (0.0, 4872.0)

With each flick, the proposed content offset kept growing, and by index 3, it already thought it was supposed to scroll nearly to the end, even though I was modifying the proposed content offset. This action should have set off some flag to start decelerating as soon as possible, but it doesn't. I sense this is a bug in the scrollview, or this was never intended to be used this way.

Banck commented 6 years ago

Have anyone found solution for this issue? Except let proposedContentOffsetCenterOrigin = (isHorizontal ? proposedContentOffset.x + (proposedContentOffset.x * velocity.x) : proposedContentOffset.y + (proposedContentOffset.y * velocity.y)) + midSide it's not full solution

chika-kasymov commented 6 years ago

Another improvement can be adding a threshold value to detect if it's a flickering or not:

var proposedContentOffsetCenterOrigin = (isHorizontal ? proposedContentOffset.x : proposedContentOffset.y) + midSide
let flickVelocity: CGFloat = 0.3 // threshold
if (isHorizontal && abs(velocity.x) > flickVelocity) || (!isHorizontal && abs(velocity.y) > flickVelocity) {
    proposedContentOffsetCenterOrigin = (isHorizontal
        ? proposedContentOffset.x + (proposedContentOffset.x * velocity.x)
        : proposedContentOffset.y + (proposedContentOffset.y * velocity.y)
    ) + midSide
}

This kind of solution for flickering I found on this SO thread.

romanfurman6 commented 6 years ago

https://gist.github.com/romanfurman6/e907ccaed89976f8591b9cf736108619