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

Repeated views using FXImageView as viewForItemAtIndex #179

Closed
MobileVet opened this issue Jul 15, 2012 · 23 comments
Closed

Repeated views using FXImageView as viewForItemAtIndex #179

MobileVet opened this issue Jul 15, 2012 · 23 comments

Comments

@MobileVet
Copy link

After installing the latest version of FXImageView (which is great by the way), I have noticed that occasionally I am seeing repeated images displayed in my carousel. This can happen while scrolling and when I come to a stop, an image appears to not update from it's previous version. I have also seen it when loading a brand new carousel, with the first item having the wrong image.

I am curious if this is indicative of a cache problem or a background processing issue? Any thoughts on how best to determine the root cause? I haven't been able to obtain repeatability yet unfortunately.
Cheers,

@nicklockwood
Copy link
Owner

Are you using it with AsyncImageView, or on it's own?

@nicklockwood
Copy link
Owner

It may be indicative of a caching issue, especially if you weren't seeing it with the previous FXImageView problem. How are you loading your images - can you show me your viewForItemAtIndex code?

@MobileVet
Copy link
Author

My custom Block is kind of large, which clutters things up, but here
it is. Now that I am using FXImageView, I might now do that scaling
on the ref... probably redundant. Good old legacy code...

Thanks for all your insight.

  • (UIView *)carousel:(iCarousel *)carousel
    viewForItemAtIndex:(NSUInteger)index reusingView:(UIView *)view {
    #define radians(degrees) (degrees * M_PI/180)

    if (view == nil) {
    FXImageView* fxiv = [[FXImageView alloc]
    initWithImage:[UIImage imageNamed:@"Loading.png"]];
    //[fxiv setImage:[UIImage imageNamed:@"Loading.png"]];

    UIImage* (^applyGroup)(UIImage* image);
    applyGroup = ^(UIImage* image){
        UIGraphicsBeginImageContextWithOptions(image.size, NO, 0.0f);
        CGContextRef c = UIGraphicsGetCurrentContext();
    
        // drop inside and add the white edges
        float inset = MIN(image.size.width, image.size.height) *
    

    EDGE_PERCENT;
    CGRect insetRect = CGRectInset(CGRectMake(0, 0,
    image.size.width, image.size.height), inset, inset);
    CGContextSetRGBFillColor(c,1,1,1,1);

        // draw the tilted tiles
        CGContextSaveGState(c);
        CGContextTranslateCTM(c, 0.5f * insetRect.size.width, 0.5f
    
  • insetRect.size.height);
    CGContextRotateCTM (c, radians((rand() % 5 + 2)));
    CGContextTranslateCTM(c, -0.5f * insetRect.size.width,
    -0.5f * insetRect.size.height);
    CGContextFillRect(c,insetRect);
    CGContextRestoreGState(c);

        CGContextSaveGState(c);
        CGContextTranslateCTM(c, 0.5f * insetRect.size.width, 0.5f
    
  • insetRect.size.height);
    CGContextRotateCTM (c, radians(-(rand() % 5 + 2)));
    CGContextTranslateCTM(c, -0.5f * insetRect.size.width,
    -0.5f * insetRect.size.height);
    CGContextSetShadow(c, CGSizeMake(0, 0), 10);
    CGContextFillRect(c,insetRect);
    CGContextRestoreGState(c);

        // draw the main image background
        CGContextSaveGState(c);
        CGContextSetShadow(c, CGSizeMake(0, 0), 10);
        CGContextFillRect(c,insetRect);
        CGContextRestoreGState(c);
    
        // drop in again and draw the image
        insetRect = CGRectInset(insetRect, inset/2, inset/2);
        [image drawInRect:insetRect];
    
        //capture resultant image
        UIImage *comp = UIGraphicsGetImageFromCurrentImageContext();
        UIGraphicsEndImageContext();
    
        //return composite image
        return comp;
    };
    fxiv.customEffectsBlock = applyGroup;
    fxiv.asynchronous = YES;
    view = fxiv;
    

    }
    ALAssetsGroup* group = [groups objectAtIndex:index];
    CGFloat height = coverFlowView.bounds.size.height * COVER_HEIGHT_PERCENT;

    // grab our low resolution placeholder image
    CGImageRef ref = [group posterImage];
    CGFloat widthScale = coverFlowView.bounds.size.width * COVER_HEIGHT_PERCENT;
    widthScale = widthScale / CGImageGetWidth(ref);
    CGFloat heightScale = height;
    heightScale = heightScale / CGImageGetHeight(ref);
    CGFloat scaleFactor = MIN(widthScale, heightScale);
    UIImage* coverImage;
    if (ref) {
    coverImage = [UIImage imageWithCGImage:ref scale:1/scaleFactor
    orientation:UIImageOrientationUp];
    }
    else {
    coverImage = [UIImage imageNamed:@"Sunflower.png"];
    }

    //if ([hiResSwitch isOn]) {
    // send out a request for the high resolution version of the image
    // AFGetImageOperation* getImageOperation =
    [[AFGetImageOperation alloc] initWithIndex:index fromGroup:group
    ofDimension:height viewController:self];
    // [loadImageOpQueue addOperation:getImageOperation];
    // }

    [(FXImageView*)view setImage:coverImage];
    return view;

}

On Sat, Jul 14, 2012 at 9:04 PM, Nick Lockwood
reply@reply.github.com
wrote:

It may be indicative of a caching issue, especially if you weren't seeing it with the previous FXImageView problem. How are you loading your images - can you show me your viewForItemAtIndex code?


Reply to this email directly or view it on GitHub:
#179 (comment)

@nicklockwood
Copy link
Owner

Its probably not a good idea to use initWithImage to set the loading image, for a few reasons:

  1. At that point your FXImageView is operating in synchronous mode, so that will block the main thread and screw up performance
  2. The loading image will not be put back when the image is recycled, so it will be showing the previously generated image until such time as the new one has finished loading (may be related to the duplication issues you are seeing)
  3. You're applying your effects to your loading image multiple times, even though it never changes.

I suggest setting the loading.png image after of the if (view == nil) check, and you should set it using the FXImageView.processedImage property so that the effects are not applied each time you set it - if you want to apply the effects, apply them yourself manually to the image when the app loads and keep the processed image for re-use.

The Dynamic Image Effects (aka Reflections) example in iCarousel shows the correct way to set a loading image.

Also, as you already suggested, it's not necessary to apply any scaling to the image prior to setting it on the FXImageView because FXImageView already scales or crops it according to the contentMode, so just set the FXImageView contentMode to aspect fit or whatever it is your scaling logic is currently doing.

@MobileVet
Copy link
Author

Thanks, this is very helpful.

I adjusted the loading image to the appropriate place and that
definitely looks better as the fade in is from the 'loading' image,
not the previous image.

Sadly, I still see quite a bit of erroneous image placement. This
appears to occur regardless of whether I utilize asynchronous=YES or
NO. If I swap out FXImageView for my lame GroupImageView, I don't
have any erroneous image placement. This was what initially lead me
to believe there might be a caching issue or some threading issue.

My GroupImageView subclasses view and overrides drawRect with the
'block' code that i copied previously (adjusted to fit the context
appropriately).

Clearly though, my GroupImageView results in much less smooth
scrolling in the iCarousel... thus my interest in doing it properly
via UIImageView and FXImageView.

Any other thoughts as to why I am seeing improperly loaded images? Is
there anything I can do on my end to provide you with meaningful
debugging information? Your libraries are very solid and I would love
to be able to contribute if there is an issue and if I can help
resolve it.

Thanks again,
Rob

On Sat, Jul 14, 2012 at 9:36 PM, Nick Lockwood
reply@reply.github.com
wrote:

Its probably not a good idea to use initWithImage to set the loading image, for a few reasons:

  1. At that point your FXImageView is operating in synchronous mode, so that will block the main thread and screw up performance
  2. The loading image will not be put back when the image is recycled, so it will be showing the previously generated image until such time as the new one has finished loading (may be related to the duplication issues you are seeing)
  3. You're applying your effects to your loading image multiple times, even though it never changes.

I suggest setting the loading.png image after of the if (view == nil) check, and you should set it using the FXImageView.processedImage property so that the effects are not applied each time you set it - if you want to apply the effects, apply them yourself manually to the image when the app loads and keep the processed image for re-use.

The Dynamic Image Effects (aka Reflections) example in iCarousel shows the correct way to set a loading image.

Also, as you already suggested, it's not necessary to apply any scaling to the image prior to setting it on the FXImageView because FXImageView already scales or crops it according to the contentMode, so just set the FXImageView contentMode to aspect fit or whatever it is your scaling logic is currently doing.


Reply to this email directly or view it on GitHub:
#179 (comment)

@nicklockwood
Copy link
Owner

I think the key priority is to define some steps to reliably reproduce the issue. If you can reproduce it consistently it will be much easier to fix.

@nicklockwood
Copy link
Owner

Thinking about it a bit more, if this is still happening in synchronous mode, it's unlikely to be threading related, and the actual drawing logic for the setImage: method is extremely simple, so the chances are it's a caching problem. Try string the customEffectIdentifier to match the item index - that should act as a fairly effective cache buster. If that doesn't work, try disabling the cache altogether by modifying the FXImageView setImage: method.

@MobileVet
Copy link
Author

Finally got a chance to revisit the issue. Things I have found so far:

  1. custom block has no effect
  2. caching very suspect (as you say) - added log and each time I see the issue I see a subsequent 'Cache used'

Proceeding to follow your advice (didn't see it until I came to post this)

@MobileVet
Copy link
Author

More notes:

  1. synchronous has no effect
  2. fxiv.customEffectsIdentifier = [NSString stringWithFormat:@"%d",index]; has no effect
  3. setImage forced to avoid 'use cache version' has NO effect --- very interesting

FOUND IT - ((FXImageView *)view).processedImage = [UIImage imageNamed:@"Loading.png"];

Commenting out the 'loading' image resolves the issue.

@MobileVet
Copy link
Author

A suggested fix.

I am not sure if this is breaking other things, but I have found that if I comment out line 483 of FXImageView.m, I am able to use the processedImage as expected (loading up a placeholder image).

For clarity:

(void)setProcessedImage:(UIImage *)image
{
//self.originalImage = nil;
_imageView.image = image;
}

@nicklockwood
Copy link
Owner

Hmm... I don't think the fix makes sense in terms of how the method should be working. I'll think about it some more.

@MobileVet
Copy link
Author

Yea, I had a feeling it wasn't the right thing to do, that there was something happening that wasn't expected. I figured it would be helpful though as you clearly know the intent more than I.

@MobileVet
Copy link
Author

btw, I tried the demo program again and was not able to recreate the issue I am seeing in my project. As a result, I am assuming you haven't seen this yet live. I am wondering if the images weren't stored in the project, like they are in the demo, if that would help you recreate it?

My thought is that the issue has to do with the delay that is present in retrieving an ALAsset, though that seems a little far fetched. The group images typically return pretty quickly (they aren't asynchronous as full assets are) but it still could be an issue.

Haven't come up with any ideas on this that were solid, so I figured I would pass the above thoughts back to you and see if they spurred any more insight.
Cheers,

@MobileVet
Copy link
Author

Hey Nick,
Did you happen to have any follow up thoughts on this issue and how to appropriately address it?
Thanks,

@nicklockwood
Copy link
Owner

Unfortunately I've not had a chance to look into this in detail yet. I've identified some bugs in the library that I will need to address, but nothing that would account for the issue you are seeing. In particular the fact that it still happens when operating synchronously eliminates most of the potential explanations.

When you load your assets, are you doing the loading on a thread before passing the image to FXImageView? If so, what does that code look like?

@MobileVet
Copy link
Author

Nick,

No, right now we are loading the images directly in the viewForIndex
method as we are only loading the 'group' image, which is immediately
returned. I had thought about using FXImageView to also request the
full resolution image after the group image (prescaled by Apple) is
loaded. This would require a delay as the full asset is requested
asynchronously with a block call.

Cheers,

On Wed, Jul 25, 2012 at 8:11 PM, Nick Lockwood
reply@reply.github.com
wrote:

Unfortunately I've not had a chance to look into this in detail yet. I've identified some bugs in the library that I will need to address, but nothing that would account for the issue you are seeing. In particular the fact that it still happens when operating synchronously eliminates most of the potential explanations.

When you load your assets, are you doing the loading on a thread before passing the image to FXImageView? If so, what does that code look like?


Reply to this email directly or view it on GitHub:
#179 (comment)

@nicklockwood
Copy link
Owner

Hello again. I've still not been able to reproduce this problem, but I did release an update to FXImageView that fixes a few caching bugs, so it may be worth trying again with the new version.

@MobileVet
Copy link
Author

Thanks Nick,

I just checked it out and found the same results. If I comment out, now
line 462, everything works ok. If I leave it in, i get incorrect results
fairly commonly. After I swipe and run the cover flow over to a new group
of albums, one and sometimes two, of the covers are repeats from the last
group.

Considering what I am commenting out, my theory is that for some reason the
album is not marked as dirty and it just shows the prior results for that
particularly cached result versus what should be a new request at a new
index.

I will try and dig back into it again sometime soon and see if I can be
more helpful. Thanks for pinging me.
Cheers,

On Tue, Aug 21, 2012 at 8:34 AM, Nick Lockwood notifications@github.comwrote:

Hello again. I've still not been able to reproduce this problem, but I did
release an update to FXImageView that fixes a few caching bugs, so it may
be worth trying again with the new version.


Reply to this email directly or view it on GitHubhttps://github.com//issues/179#issuecomment-7899504.

@nicklockwood
Copy link
Owner

Are you setting your FXImageView.processedImage placeholder before or after you set the FXImageView.image for processing?

@MobileVet
Copy link
Author

Before

R

On Thu, Aug 23, 2012 at 3:58 AM, Nick Lockwood notifications@github.comwrote:

Are you setting your FXImageView.processedImage placeholder before or
after you set the FXImageView.image for processing?


Reply to this email directly or view it on GitHubhttps://github.com//issues/179#issuecomment-7962517.

@nicklockwood
Copy link
Owner

Hmm... Well that should be correct. It makes it even more strange that commenting the line you did would make any difference since you're immediately overwriting that value.

I appreciate you're probably getting bored of trying new versions by now, but if you get a chance please try version 1.2.2 which fixes some issues I spotted this morning that may conceivably relate to your bug.

And as always, if you have a project that demonstrates the problem in a consistently reproducible way, I'd love to see it.

@MobileVet
Copy link
Author

Nick

Still no good, although I am seeing a crash bug occasionally now... it
indicates my drawing routine for FXImageView has a bad reference in the
midst of the drawing...

Not sure why I didn't do this earlier, but I forked your repo and created a
new branch called alasset. It utilizes the core image lookup code I am
using in my project and thus suffers from the same issues I am seeing. I
was moving too quickly and submitted a pull request to you, but obviously I
don't want you to integrate my 'changes'. It is just an example.

Cheers,
Rob

On Thu, Aug 23, 2012 at 11:57 AM, Nick Lockwood notifications@github.comwrote:

Hmm... Well that should be correct. It makes it even more strange that
commenting the line you did would make any difference since you're
immediately overwriting that value.

I appreciate you're probably getting bored of trying new versions by now,
but if you get a chance please try version 1.2.2 which fixes some issues I
spotted this morning that may conceivably relate to your bug.

And as always, if you have a project that demonstrates the problem in a
consistently reproducible way, I'd love to see it.


Reply to this email directly or view it on GitHubhttps://github.com//issues/179#issuecomment-7974407.

@nicklockwood
Copy link
Owner

Now fixed.

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