14

I have a UICollectionView with a grid of images. When you tap on one, it opens up the grid and shows a subview with some details. Like this:

Supposed to look like this

I open up the grid in my UICollectionViewLayout by adjusting the UICollectionViewLayoutAttributes and setting a translation on the transform3D property for all cells below the current row of the selected item. This works really nicely, and is a much better animation and a simpler approach than my first attempt at inserting another cell into the grid which is a different size to the others.

Anyway... it works most of the time, but then after continued use I see old images on the collection view. They are like ghost cells. I can't click them, it's like they haven't been removed from the collection view properly, and they sit on top of the cells preventing taps and just being a nuisance. Like this:

Problem looks like this

Any ideas why these cells are doing this?

EDIT: I'd like to add, I think it only happens when i scroll the collection view really fast. I've written my own UICollectionViewFlowLayout replacement to test if it still happens. It does.

EDIT 2: The 3d transforms or layout have nothing to do with this. It must be a bug in UICollectionView. I can exploit by just scrolling really fast, letting come to a standstill and then querying the views that are on screen. There are often double the number of cells, but they are hidden as they are stacked on top of each other. My implementation above reveals them because of the translation i do.

This can really hurt performance too.

See my answer for a solution.

bandejapaisa
  • 24,838
  • 13
  • 86
  • 110
  • Could you post an screenshot? – Marcelo Fabri Jul 18 '13 at 05:07
  • Bizarre random moment, saw this a couple hours ago. Looked at your profile page. Had an idea for an app. Went searching for Flickr API. Saw FlickrKit on Github. Recognised the name. LOL! Netception! – Fogmeister Jul 18 '13 at 16:44
  • yeah, I get around..... and you're in Leeds, and I'm from Leeds... actually, just about to drive there. – bandejapaisa Jul 18 '13 at 16:46
  • With some added perspective from here in the future: I seem to get this issue under iOS 6 but not under iOS 7. When cells are supposed to move by animation under 6 they often leave copies behind; when scrolling if I add code to check for removal in that collection view cell's `removeFromSuperview` or in a creative subclass of the collection view's `willRemoveSubview:` I see that views are never removed. Under iOS 7 I see no such problems. So I think Apple has fixed this bug. Based on the comments so far I feel I should also add: I lived in York for five years way back when. – Tommy Mar 06 '14 at 13:07
  • You should also have a look at this post. http://stackoverflow.com/questions/12927027/uicollectionview-flowlayout-not-wrapping-cells-correctly-ios#comment33106935_12927027 which I think is the same thing, but a more elegant solution. I haven't tested fully. – bandejapaisa Mar 19 '14 at 09:42

3 Answers3

14

My second edit of my question details why this is happenening, and here is my workaround. It's not bullet proof, but it works in my case, and if you experience somethign similar you could tweak my solution:

- (void) removeNaughtyLingeringCells {

    // 1. Find the visible cells
    NSArray *visibleCells = self.collectionView.visibleCells;
    //NSLog(@"We have %i visible cells", visibleCells.count);

    // 2. Find the visible rect of the collection view on screen now
    CGRect visibleRect;
    visibleRect.origin = self.collectionView.contentOffset;
    visibleRect.size = self.collectionView.bounds.size;
    //NSLog(@"Rect %@", NSStringFromCGRect(visibleRect));


    // 3. Find the subviews that shouldn't be there and remove them
    //NSLog(@"We have %i subviews", self.collectionView.subviews.count);
    for (UIView *aView in [self.collectionView subviews]) {
        if ([aView isKindOfClass:UICollectionViewCell.class]) {
            CGPoint origin = aView.frame.origin;
            if(CGRectContainsPoint(visibleRect, origin)) {
                if (![visibleCells containsObject:aView]) {
                    [aView removeFromSuperview];
                }
            }
        }
    }
    //NSLog(@"%i views shouldn't be there", viewsShouldntBeThere.count);

    // 4. Refresh the collection view display
    [self.collectionView setNeedsDisplay];    
}

and

- (void) scrollViewDidEndDragging:(UIScrollView *)scrollView willDecelerate:(BOOL)decelerate {
    if (!decelerate) {
        [self removeNaughtyLingeringCells];
    }
}

- (void) scrollViewDidEndDecelerating:(UIScrollView *)scrollView {
    [self removeNaughtyLingeringCells];
}
bandejapaisa
  • 24,838
  • 13
  • 86
  • 110
  • 1
    Thank you a lot! In my case, removing `Hidden` cells in the same situation works just fine. Also, my way to reproduce this problem is: 1. Scroll very fast. 2. Wait for deceleration to stop. 3. Delete a cell. 4. Wait for delete animation to finish. – Dan Abramov Aug 22 '13 at 11:01
2

A quick further comment to bandejapaisa's: under iOS 6 only, I found that UICollectionView also had a habit of bungling animated transitions. The original cells would remain where they were, copies would be made and then the copies would be animated. Usually on top of the originals but not always. So a simple bounds test wasn't sufficient.

I therefore wrote a custom subclass of UICollectionView that does the following:

- (void)didAddSubview:(UIView *)subview
{
    [super didAddSubview:subview];

    //
    // iOS 6 contains a bug whereby it fails to remove subviews, ever as far as I can make out.
    // This is a workaround for that. So, if this is iOS 6...
    //
    if(![UIViewController instancesRespondToSelector:@selector(automaticallyAdjustsScrollViewInsets)])
    {
        // ... then we'll want to wait until visibleCells has definitely been updated ...
        dispatch_async(dispatch_get_main_queue(),
        ^{
            // ... then we'll manually remove anything that's a sub of UICollectionViewCell
            // and isn't currently listed as a visible cell
            NSArray *visibleCells = self.visibleCells;
            for(UIView *view in self.subviews)
            {
                if([view isKindOfClass:[UICollectionViewCell class]] && ![visibleCells containsObject:view])
                    [view removeFromSuperview];
            }
        });
    }
}

Obviously it's a shame that 'is this iOS 6' test can't be a little more direct but it's hidden off in a category in my actual code.

Tommy
  • 97,164
  • 12
  • 174
  • 193
  • I've been seeing this strange "ghost" effect during fast scrolling calling performBatchUpdate: animations and had *no* idea what it was. It never would have occurred to me that the collection view is making a new cell for a currently on-screen cell - that seems counter-productive. Good catch, thanks – SG1 Aug 08 '14 at 14:01
1

A Swift UICollectionView extension version of bandejapaisa's answer:

extension UICollectionView {

    func removeNaughtyLingeringCells() {

        // 1. Find the visible cells
        let visibleCells = self.visibleCells()
        //NSLog("We have %i visible cells", visibleCells.count)


        // 2. Find the visible rect of the collection view on screen now
        let visibleRect = CGRectOffset(bounds, contentOffset.x, contentOffset.y)
        //NSLog("Rect %@", NSStringFromCGRect(visibleRect))


        // 3. Find the subviews that shouldn't be there and remove them
        //NSLog("We have %i subviews", subviews.count)
        for aView in subviews {
            if let aCollectionViewCell = aView as? UICollectionViewCell {

                let origin = aView.frame.origin
                if (CGRectContainsPoint(visibleRect, origin)) {
                    if (!visibleCells.contains(aCollectionViewCell)) {
                        aView.removeFromSuperview()
                    }
                }

            }
        }

        // 4. Refresh the collection view display
        setNeedsDisplay()
    }
}
user2067021
  • 3,919
  • 35
  • 43