size of backgroundView is too small depending on the number of items #177

Open
karmeye opened this Issue Nov 5, 2012 · 14 comments

Comments

Projects
None yet
3 participants

karmeye commented Nov 5, 2012

When the number of items in the grid does not fill the last row exactly the backgroundView's height is to small (960 in my case).

fixed it by changing

self.backgroundView.frame = backgroundRect;

in layoutSubviews to

self.backgroundView.frame = CGRectMake(backgroundRect.origin.x, backgroundRect.origin.y, self.contentSize.width, self.contentSize.height);

This was just a fast fix, I do not know if its a good one.
This is the develop branch.

Collaborator

evadne commented Nov 5, 2012

Thanks! That sounds like a bug, and the line of code you mentioned is probably good. We should fix the underlying issue that causes backgroundRect to contain an invalid value.

On Nov 5, 2012, at 10:29 AM, rassol notifications@github.com wrote:

When the number of items in the grid does not fill the last row exactly the backgroundView's height is to small (960 in my case).

fixed it by changing

self.backgroundView.frame = backgroundRect;

to

self.backgroundView.frame = CGRectMake(backgroundRect.origin.x, backgroundRect.origin.y, self.contentSize.width, self.contentSize.height);

This was just a fast fix, I do not know if its a good one.
This is the develop branch.


Reply to this email directly or view it on GitHub:
#177

karmeye commented Nov 6, 2012

In the future it would be gread to implement a recycling model for the background view. Now if there are several hundred items the scrolling becomes hacky, even more in landscape mode.

thanks for good work.

Collaborator

evadne commented Nov 6, 2012

View tiling / recycling is something that can be trivially done from the calling class. You’ll observe -scrollViewDidScroll: and tile your background views, using contentOffset and the bounds to determine how much to tile.

We are not tiling the background view in AQGridView, at least as of the version I know. :)

Slow is not good. Post a project somewhere under a new issue, slap PERFORMANCE on the title and we’ll work a way out.

On Nov 6, 2012, at 4:46 AM, rassol notifications@github.com wrote:

In the future it would be gread to implement a recycling model for the background view. Now if there are several hundred items the scrolling becomes hacky, even more in landscape mode.

thanks for good work.


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

Owner

AlanQuatermain commented Nov 6, 2012

I've got code for a tiling background view that I've been meaning to clean up & commit — we've been doing that at Kobo for a while now to avoid the memory footprint of the layer backing store in a large background view.

On 2012-11-06, at 10:35 AM, Evadne Wu notifications@github.com wrote:

View tiling / recycling is something that can be trivially done from the calling class. You’ll observe -scrollViewDidScroll: and tile your background views, using contentOffset and the bounds to determine how much to tile.

We are not tiling the background view in AQGridView, at least as of the version I know. :)

Slow is not good. Post a project somewhere under a new issue, slap PERFORMANCE on the title and we’ll work a way out.

On Nov 6, 2012, at 4:46 AM, rassol notifications@github.com wrote:

In the future it would be gread to implement a recycling model for the background view. Now if there are several hundred items the scrolling becomes hacky, even more in landscape mode.

thanks for good work.


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

Reply to this email directly or view it on GitHub.

Cheers,
-Jim

jimdovey@(mac|gmail).com
http://twitter.com/alanQuatermain
http://github.com/alanQuatermain
http://stackoverflow.com/users/103900/jim-dovey
http://ca.linkedin.com/pub/jim-dovey/2/24b/883

Collaborator

evadne commented Nov 6, 2012

Can it be made into a magic view that automatically works when put into any UIScrollView? I’d push for that, on a separate repository.

On Nov 6, 2012, at 10:37 AM, Jim Dovey notifications@github.com wrote:

I've got code for a tiling background view that I've been meaning to clean up & commit — we've been doing that at Kobo for a while now to avoid the memory footprint of the layer backing store in a large background view.

On 2012-11-06, at 10:35 AM, Evadne Wu notifications@github.com wrote:

View tiling / recycling is something that can be trivially done from the calling class. You’ll observe -scrollViewDidScroll: and tile your background views, using contentOffset and the bounds to determine how much to tile.

We are not tiling the background view in AQGridView, at least as of the version I know. :)

Slow is not good. Post a project somewhere under a new issue, slap PERFORMANCE on the title and we’ll work a way out.

On Nov 6, 2012, at 4:46 AM, rassol notifications@github.com wrote:

In the future it would be gread to implement a recycling model for the background view. Now if there are several hundred items the scrolling becomes hacky, even more in landscape mode.

thanks for good work.


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

Reply to this email directly or view it on GitHub.

Cheers,
-Jim

jimdovey@(mac|gmail).com
http://twitter.com/alanQuatermain
http://github.com/alanQuatermain
http://stackoverflow.com/users/103900/jim-dovey
http://ca.linkedin.com/pub/jim-dovey/2/24b/883


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

Owner

AlanQuatermain commented Nov 6, 2012

Probably. Maybe. I'd have to check & see if there's any glue in there— we wouldn't want to eat the scroll view delegate property by requiring this background always be the delegate…

On 2012-11-06, at 10:43 AM, Evadne Wu notifications@github.com wrote:

Can it be made into a magic view that automatically works when put into any UIScrollView? I’d push for that, on a separate repository.

On Nov 6, 2012, at 10:37 AM, Jim Dovey notifications@github.com wrote:

I've got code for a tiling background view that I've been meaning to clean up & commit — we've been doing that at Kobo for a while now to avoid the memory footprint of the layer backing store in a large background view.

On 2012-11-06, at 10:35 AM, Evadne Wu notifications@github.com wrote:

View tiling / recycling is something that can be trivially done from the calling class. You’ll observe -scrollViewDidScroll: and tile your background views, using contentOffset and the bounds to determine how much to tile.

We are not tiling the background view in AQGridView, at least as of the version I know. :)

Slow is not good. Post a project somewhere under a new issue, slap PERFORMANCE on the title and we’ll work a way out.

On Nov 6, 2012, at 4:46 AM, rassol notifications@github.com wrote:

In the future it would be gread to implement a recycling model for the background view. Now if there are several hundred items the scrolling becomes hacky, even more in landscape mode.

thanks for good work.


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

Reply to this email directly or view it on GitHub.

Cheers,
-Jim

jimdovey@(mac|gmail).com
http://twitter.com/alanQuatermain
http://github.com/alanQuatermain
http://stackoverflow.com/users/103900/jim-dovey
http://ca.linkedin.com/pub/jim-dovey/2/24b/883


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

Reply to this email directly or view it on GitHub.

Cheers,
-Jim

jimdovey@(mac|gmail).com
http://twitter.com/alanQuatermain
http://github.com/alanQuatermain
http://stackoverflow.com/users/103900/jim-dovey
http://ca.linkedin.com/pub/jim-dovey/2/24b/883

Collaborator

evadne commented Nov 6, 2012

Guess we can use -didMoveToSuperview:, then KVO the contentOffset and bounds or worse use CADisplayLink and poll the presentationLayer.

On Nov 6, 2012, at 10:45 AM, Jim Dovey notifications@github.com wrote:

Probably. Maybe. I'd have to check & see if there's any glue in there— we wouldn't want to eat the scroll view delegate property by requiring this background always be the delegate…

On 2012-11-06, at 10:43 AM, Evadne Wu notifications@github.com wrote:

Can it be made into a magic view that automatically works when put into any UIScrollView? I’d push for that, on a separate repository.

On Nov 6, 2012, at 10:37 AM, Jim Dovey notifications@github.com wrote:

I've got code for a tiling background view that I've been meaning to clean up & commit — we've been doing that at Kobo for a while now to avoid the memory footprint of the layer backing store in a large background view.

On 2012-11-06, at 10:35 AM, Evadne Wu notifications@github.com wrote:

View tiling / recycling is something that can be trivially done from the calling class. You’ll observe -scrollViewDidScroll: and tile your background views, using contentOffset and the bounds to determine how much to tile.

We are not tiling the background view in AQGridView, at least as of the version I know. :)

Slow is not good. Post a project somewhere under a new issue, slap PERFORMANCE on the title and we’ll work a way out.

On Nov 6, 2012, at 4:46 AM, rassol notifications@github.com wrote:

In the future it would be gread to implement a recycling model for the background view. Now if there are several hundred items the scrolling becomes hacky, even more in landscape mode.

thanks for good work.


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

Reply to this email directly or view it on GitHub.

Cheers,
-Jim

jimdovey@(mac|gmail).com
http://twitter.com/alanQuatermain
http://github.com/alanQuatermain
http://stackoverflow.com/users/103900/jim-dovey
http://ca.linkedin.com/pub/jim-dovey/2/24b/883


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

Reply to this email directly or view it on GitHub.

Cheers,
-Jim

jimdovey@(mac|gmail).com
http://twitter.com/alanQuatermain
http://github.com/alanQuatermain
http://stackoverflow.com/users/103900/jim-dovey
http://ca.linkedin.com/pub/jim-dovey/2/24b/883


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

Collaborator

evadne commented Nov 6, 2012

I wrote it. Tested a few combinations, view dequeueing, reusing, and stretching all seem to work well.

Code: https://github.com/evadne/RATilingBackgroundView
Sample app: https://github.com/evadne/RATilingBackgroundView-Sample

karmeye commented Nov 12, 2012

Cool. I've added it to the gridView:

In my ViewController viewDidLoad

    _gridView.backgroundView = [[UIView alloc] initWithFrame:CGRectZero];    
    [_gridView insertSubview:self.tilingBackgroundView belowSubview:_gridView.backgroundView];

In AQGridView layoutSubviews

self.backgroundView.frame = CGRectZero;

Changed the property

- (RATilingBackgroundView *) tilingBackgroundView {

    if (_tilingBackgroundView)
        return _tilingBackgroundView;

    //if ([self isViewLoaded]) {
        RATilingBackgroundView *tilingBackgroundView = [RATilingBackgroundView new];
        tilingBackgroundView.delegate = self;
        tilingBackgroundView.layer.borderColor = [[UIColor blueColor] colorWithAlphaComponent:0.5f].CGColor;
        tilingBackgroundView.layer.borderWidth = 8.0f;
        //[_gridView addSubview:tilingBackgroundView];
        _tilingBackgroundView = tilingBackgroundView;
    //}

    return _tilingBackgroundView;
}

Works fine now with 3000 grid items which did not work at all before.

I am looking into adding the headerView now and also being able to drag the gridview beyond its contentSize to see the contents of its superview (now the tiles continue even tough contentoffset is negative).

Collaborator

evadne commented Nov 12, 2012

Probably need a way to tell the tiling background view if it should
continue tiling outside the bounds! It should be a single flag…. Up for
grabs?

On Nov 12, 2012, at 6:49, rassol notifications@github.com wrote:

Cool. I've added it to the gridView:

In my ViewController viewDidLoad

_gridView.backgroundView = [[UIView alloc] initWithFrame:CGRectZero];
[_gridView insertSubview:self.tilingBackgroundView

belowSubview:_gridView.backgroundView];

In AQGridView layoutSubviews

self.backgroundView.frame = CGRectZero;

Changed the property

  • (RATilingBackgroundView *) tilingBackgroundView {

    if (_tilingBackgroundView)
    return _tilingBackgroundView;

    //if ([self isViewLoaded]) {
    RATilingBackgroundView *tilingBackgroundView =
    [RATilingBackgroundView new];
    tilingBackgroundView.delegate = self;
    tilingBackgroundView.layer.borderColor = [[UIColor blueColor]
    colorWithAlphaComponent:0.5f].CGColor;
    tilingBackgroundView.layer.borderWidth = 8.0f;
    //[_gridView addSubview:tilingBackgroundView];
    _tilingBackgroundView = tilingBackgroundView;
    //}

    return _tilingBackgroundView;}

Works fine now with 3000 grid items which did not work at all before.

I am looking into adding the headerView now and also being able to drag the
gridview beound its contentSize to see the contents of its superview, now
the tiles continue even tough contentoffset is negative.


Reply to this email directly or view it on
GitHubhttps://github.com/AlanQuatermain/AQGridView/issues/177#issuecomment-10284646.

karmeye commented Nov 12, 2012

Added a local variable to keep track of the contentOffset. Also added a public UIEdgeInsets property which top value I set to the same height as the gridViews header.

if ([keyPath isEqualToString:contentOffsetKey])
        {            
            _curContentOffset = [change[NSKeyValueChangeNewKey] CGPointValue];
            const CGSize tileSize = [self tileSize];

            self.offset = (CGPoint)
            {
                fmodf(-1 * _curContentOffset.x, tileSize.width) - (self.horizontalStretchingEnabled ? 0.0f : ceilf(CGRectGetWidth(self.bounds) / tileSize.width) * tileSize.width),
                fmodf(-1 * _curContentOffset.y, tileSize.height) - (self.verticalStretchingEnabled ? 0.0f : ceilf(CGRectGetHeight(self.bounds) / tileSize.height) * tileSize.height)
            };

        }
        else if ([keyPath isEqualToString:boundsKey])
        {
            self.frame = CGRectOffset(self.superview.bounds, 0.f, _edgeInsets.top);// self.superview.bounds;
        }

In layoutSubviews added

tile.frame = rect;

        if (_curContentOffset.y < _edgeInsets.top && tile.frame.origin.y < _curContentOffset.y * -1.f) {
            tile.hidden = YES;
        }
        else {
            tile.hidden = NO;
        }

Also added a if (self.superview) in didMoveToSuperView.

Collaborator

evadne commented Nov 12, 2012

Thanks a bunch. That is interesting: I expected that overriding the method
which gives you tile rects might suffice — if you hide something, there is
still a small performance penalty for keeping it around. Will pull into
project. Thanks again!

On Nov 12, 2012, at 10:20, rassol notifications@github.com wrote:

Added a local variable to keep track of the contentOffset. Also added a
public UIEdgeInsets property which top value I set to the same height as
the gridViews header.

if ([keyPath isEqualToString:contentOffsetKey])
{
_curContentOffset = [change[NSKeyValueChangeNewKey] CGPointValue];
const CGSize tileSize = [self tileSize];

        self.offset = (CGPoint)
        {
            fmodf(-1 * _curContentOffset.x, tileSize.width) -

(self.horizontalStretchingEnabled ? 0.0f :
ceilf(CGRectGetWidth(self.bounds) / tileSize.width) * tileSize.width),
fmodf(-1 * _curContentOffset.y, tileSize.height) -
(self.verticalStretchingEnabled ? 0.0f :
ceilf(CGRectGetHeight(self.bounds) / tileSize.height) *
tileSize.height)
};

    }
    else if ([keyPath isEqualToString:boundsKey])
    {
        self.frame = CGRectOffset(self.superview.bounds, 0.f,

_edgeInsets.top);// self.superview.bounds;
}

In layoutSubviews added

tile.frame = rect;

    if (_curContentOffset.y < _edgeInsets.top &&

tile.frame.origin.y < _curContentOffset.y * -1.f) {
tile.hidden = YES;
}
else {
tile.hidden = NO;
}

Also added a if (self.superview) in didMoveToSuperView.


Reply to this email directly or view it on
GitHubhttps://github.com/AlanQuatermain/AQGridView/issues/177#issuecomment-10291246.

karmeye commented Nov 12, 2012

I hid it only because I was lazy. But I do not think it's a performance penalty. It ought to be at least as slow as when all tiles are visible.

karmeye commented Nov 13, 2012

How come you call setNeedsLayout first in observeValueForKeyPath and not last?
I call them last in each if case. Seems to work.

Hmm, maybe it doesn't matter since setNeedsLayout is async.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment