facebookarchive/AsyncDisplayKit

ASTableView Section Header Cell does not render subNode in App Store build

ulfie22 opened this issue · 6 comments

I have implemented a sectioned ASTableView. It renders as expected in the simulator, but when I build the app for testing in the AppStore, the section header view does not render its single ASTextNode subnode.

screen shot from simulator:

image

screen shot from iPhone running TestFlight build:

image

In the ASTableView delegate, I have implemented

- (UIView *)tableView:(UITableView *)tableView viewForHeaderInSection:(NSInteger)section
{
    SKMenuSection *sect = [_menu.sections objectAtIndex:section];
    TableHeaderNode *node = [[TableHeaderNode alloc] initWithTitle:sect.title];

    return node.view;
}

The TableHeaderNode class is pretty simple:

-(instancetype)initWithTitle:(NSString *)title {

    if (!(self = [super init]))
        return nil;

    self.backgroundColor = [UIColor colorWithRed:0.8 green:0.8 blue:0.8 alpha:1];

    _textNode = [[ASTextNode alloc] init];
    NSMutableAttributedString *myString = [[NSMutableAttributedString alloc] initWithString:title];
    _textNode.attributedString = [[NSAttributedString alloc] initWithAttributedString:myString];

    [self addSubnode:_textNode];

    return self;
}

- (CGSize)calculateSizeThatFits:(CGSize)constrainedSize
{
    DDLogCInfo(@"TableHeaderNode calculateSizeThatFits constrained size w: %f h: %f", constrainedSize.width, constrainedSize.height);
    CGSize textSize = [_textNode measure:CGSizeMake(constrainedSize.width, 44.0)];
    CGSize forcedSize = CGSizeMake(constrainedSize.width, 44.0);

    return forcedSize;
}

- (void)layout
{
    CGSize textSize1 = [_textNode measure:CGSizeMake(self.frame.size.width, self.frame.size.height)];
    CGSize textSize = _textNode.calculatedSize;
    DDLogCInfo(@"TableHeaderNode layout calculated size w: %f h: %f", textSize.width, textSize.height);

    _textNode.frame = CGRectMake(kOuterPadding, kOuterPadding, textSize.width, textSize.height);

}

You'll note that I call measure: on the ASTextNode in layout: - I did this because calculateSizeThatFits: is never called for this node.

So - what is different about how this is rendered in an Archive build versus running in the simulator? Any ideas how I can fix this?

Thanks for slogging through this...

I created a repo:

https://github.com/ulfie22/SectionHeaderTest

with a project that demonstrates this issue. I will be happy to add you as an internal tester in TestFlight if you want to see the App Store version of the app.

Thank you for your help.

Hm, interesting! Let's take a look.

You'll note that I call measure: on the ASTextNode in layout: - I did this because calculateSizeThatFits: is never called for this node.

This is because you never call -[TableHeaderNode measure:]. When you supply nodes to ASTableView or ASCollectionView, the underlying working-range machinery is responsible for measuring them; if you're using nodes directly, you have to size them yourself, ideally on a background queue if possible.

I'd start by modifying your delegate method to look more like this:

- (UIView *)tableView:(UITableView *)tableView viewForHeaderInSection:(NSInteger)section
{
    SKMenuSection *sect = [_menu.sections objectAtIndex:section];
    TableHeaderNode *node = [[TableHeaderNode alloc] initWithTitle:sect.title];
    [node measure:CGSizeMake(tableView.bounds.size.width, FLT_MAX)];

    return node.view;
}

You can then remove the duplicate work from TableHeaderNode's layout implementation:

- (void)layout
{
    CGPoint textOrigin = CGPointMake(kOuterPadding, kOuterPadding);
    _textNode.frame = (CGRect){ .origin = textOrigin, .size = _textNode.calculatedSize };
}

What does your -tableView:heightForHeaderInSection: delegate method look like? ASTableView currently doesn't support header / footer / supplementary nodes, which is why you have to use the UIView-based API; you may want to create and measure your TableHeaderNodes in advance, stash them somewhere, and return cachedNode.view and cachedNode.calculatedSize.height for the two methods. (This is, to a first approximation, how ASRangeController works.)

(Quick postscript on creating these in advance: you could, at the time you populate _menu.sections, create / measure / store TableHeaderNodes instead of just NSStrings. If you do this on a background queue, you shouldn't block your app's UI and everything should work out nicely.)

Secretive -

Thank you very much for your help, but unfortunately, I am getting the same result. (I pushed the changes you suggested to https://github.com/ulfie22/SectionHeaderTest ). When I run the app, calculateSizeThatFits: is indeed now called. In the simulator, the table headers look like I expect them to; however, in the app store build, the table headers have the background color, but not the text node. (This is the same result as before).

You had asked about the -tableView:heightForHeaderInSection: method: I have it just returning a constant - which is the same value as I force the height to be in calculateSizeThatFits: in the header node class. (As an aside - in that method, I call measure: on the text node in order to cache its computed size, and it ends up as an "unused variable" to Xcode. Obviously there's no damage done here, but I'm wondering if that's the right way to do it).

I would be happy to invite you as a tester for the TestFlight version of this simple app.

Thanks again for your help.

Thanks for the report @ulfie22! This one was kind of a doozy because its really bizarre that it works in Debug but not in Release. In theory, it shouldn't work in Debug at all!

When you are returning the node's view in -tableView:viewForHeaderInSection:, the view that is returned does not have a strong reference to the node, so it's reference count gets decreased and it's deallocated. If you override dealloc for TableHeaderNode you will see this.

@secretiverhyme's suggestion should do the trick. I created a really simple pull request to demonstrate this.

As to why it works in Debug... it's got to do with some Core Animation magic. We create a reference to the node here but let CALayer create its own dynamic properties.

Again, great issue! This was a lot of fun to look into 😃

Thanks @rnystrom !!!!!!

I can confirm that you pull request performs as expected on the release build. I might have guessed that TableView magic would retain that header view, but even with that caching them will definitely save time on table changes and reloads.

I really appreciate your help with this!!