diff --git a/AppKit/CPArrayController.j b/AppKit/CPArrayController.j index d98f159d7..42ad4acc3 100644 --- a/AppKit/CPArrayController.j +++ b/AppKit/CPArrayController.j @@ -70,7 +70,8 @@ + (CPSet)keyPathsForValuesAffectingArrangedObjects { - return [CPSet setWithObjects:"content", "filterPredicate", "sortDescriptors"]; + // Also depends on "filterPredicate" but we'll handle that manually. + return [CPSet setWithObjects:"content", "sortDescriptors"]; } + (CPSet)keyPathsForValuesAffectingSelection @@ -488,7 +489,14 @@ */ - (void)setFilterPredicate:(CPPredicate)value { + if (_filterPredicate === value) + return; + + // __setFilterPredicate will call _rearrangeObjects without + // sending notifications, so we must send them instead. + [self willChangeValueForKey:@"arrangedObjects"]; [self __setFilterPredicate:value]; + [self didChangeValueForKey:@"arrangedObjects"]; } /* @@ -501,8 +509,7 @@ return; _filterPredicate = value; - // Use the non-notification version since arrangedObjects already depends - // on filterPredicate. + // Use the non-notification version. [self _rearrangeObjects]; } @@ -712,8 +719,12 @@ if (![self canAdd]) return; - if (_clearsFilterPredicateOnInsertion) + var willClearPredicate = NO; + if (_clearsFilterPredicateOnInsertion && _filterPredicate) + { [self willChangeValueForKey:@"filterPredicate"]; + willClearPredicate = YES; + } [self willChangeValueForKey:@"content"]; @@ -728,11 +739,15 @@ [_contentObject addObject:object]; _disableSetContent = NO; - if (_clearsFilterPredicateOnInsertion) - [self __setFilterPredicate:nil]; - - if (_filterPredicate === nil || [_filterPredicate evaluateWithObject:object]) + if (willClearPredicate) { + // Full rearrange needed due to changed filter. + _filterPredicate = nil; + [self _rearrangeObjects]; + } + else if (_filterPredicate === nil || [_filterPredicate evaluateWithObject:object]) + { + // Insert directly into the array. var pos = [_arrangedObjects insertObject:object inArraySortedByDescriptors:_sortDescriptors]; // selectionIndexes change notification will be fired as a result of the @@ -742,11 +757,16 @@ else [_selectionIndexes shiftIndexesStartingAtIndex:pos by:1]; } - else - [self _rearrangeObjects]; + /* + else if (_filterPredicate !== nil) + ... + // Implies _filterPredicate && ![_filterPredicate evaluateWithObject:object], so the new object does + // not appear in arrangedObjects and we do not have to update at all. + */ + // This will also send notificaitons for arrangedObjects. [self didChangeValueForKey:@"content"]; - if (_clearsFilterPredicateOnInsertion) + if (willClearPredicate) [self didChangeValueForKey:@"filterPredicate"]; } diff --git a/AppKit/CPButton.j b/AppKit/CPButton.j index 911a56d5b..095d3d0a3 100644 --- a/AppKit/CPButton.j +++ b/AppKit/CPButton.j @@ -560,14 +560,9 @@ CPButtonImageOffset = 3.0; return bounds; } -/*! - Adjust the size of the button to fit the title and surrounding button image. -*/ -- (void)sizeToFit +- (CGSize)_minimumFrameSize { - [self layoutSubviews]; - - var size, + var size = CGSizeMakeZero(), contentView = [self ephemeralSubviewNamed:@"content-view"]; if (contentView) @@ -591,9 +586,19 @@ CPButtonImageOffset = 3.0; if (maxSize.height >= 0.0) size.height = MIN(size.height, maxSize.height); - [self setFrameSize:size]; + return size; +} - if (contentView) +/*! + Adjust the size of the button to fit the title and surrounding button image. +*/ +- (void)sizeToFit +{ + [self layoutSubviews]; + + [self setFrameSize:[self _minimumFrameSize]]; + + if ([self ephemeralSubviewNamed:@"content-view"]) [self layoutSubviews]; } diff --git a/Foundation/CPArray+KVO.j b/Foundation/CPArray+KVO.j index ad05ef8c6..d8e404f31 100644 --- a/Foundation/CPArray+KVO.j +++ b/Foundation/CPArray+KVO.j @@ -127,7 +127,7 @@ _removeManySEL = sel_getName(@"remove" + capitalizedKey + "AtIndexes:"); if ([_proxyObject respondsToSelector:_removeManySEL]) - _remove = [_proxyObject methodForSelector:_removeManySEL]; + _removeMany = [_proxyObject methodForSelector:_removeManySEL]; _replaceManySEL = sel_getName(@"replace" + capitalizedKey + "AtIndexes:with" + capitalizedKey + ":"); if ([_proxyObject respondsToSelector:_replaceManySEL]) diff --git a/Tests/AppKit/CPArrayControllerPerformance.j b/Tests/AppKit/CPArrayControllerPerformance.j index 083f9b343..98137b1ee 100644 --- a/Tests/AppKit/CPArrayControllerPerformance.j +++ b/Tests/AppKit/CPArrayControllerPerformance.j @@ -2,16 +2,17 @@ @import @import +var ELEMENTS = 200, + REPEATS = 25; + @implementation CPArrayControllerPerformance : OJTestCase -- (void)testRearrangeObjects +- (CPArrayController)setupWithElements:(int)aCount { - var ELEMENTS = 200, - REPEATS = 25, - ac = [CPArrayController new], + var ac = [CPArrayController new], array = []; - for (var i = 0; i < ELEMENTS; i++) + for (var i = 0; i < aCount; i++) { var s = [Sortable new]; [s setA:i]; @@ -19,15 +20,17 @@ array.push(s); } - var descriptors = [ - [CPSortDescriptor sortDescriptorWithKey:"a" ascending:NO], - ]; - [ac setContent:array]; - [ac setFilterPredicate:[CPPredicate predicateWithFormat:@"(b != %@)", 0]]; + return ac; +} + +- (void)testRearrangeObjects +{ + var ac = [self setupWithElements:ELEMENTS]; + // Filter alone var start = (new Date).getTime(); for (var i = 0; i < REPEATS; i++) @@ -41,7 +44,7 @@ for (var j = 0, count = [sorted count]; j < count; j++) { if (sorted[j].b == 0) - [self fail:"b == 0 should be filtered out (position: "+j+")"]; + [self fail:"b == 0 should be filtered out (position: " + j + ")"]; last = sorted[j]; } } @@ -49,7 +52,9 @@ CPLog.warn("testRearrangeObjects, filter: "+(end-start)+"ms"); - [ac setSortDescriptors:descriptors]; + [ac setSortDescriptors:[ + [CPSortDescriptor sortDescriptorWithKey:"a" ascending:NO], + ]]; // Filter and sort. start = (new Date).getTime(); @@ -64,9 +69,9 @@ for (var j = 0, count = [sorted count]; j < count; j++) { if (sorted[j].b == 0) - [self fail:"b == 0 should be filtered out (position: "+j+")"]; + [self fail:"b == 0 should be filtered out (position: " + j + ")"]; if (sorted[j].a >= last) - [self fail:"array values should be descending (position: "+j+")"]; + [self fail:"array values should be descending (position: " + j + ")"]; last = sorted[j]; } } @@ -75,6 +80,64 @@ CPLog.warn("testRearrangeObjects, filter and sort: "+(end-start)+"ms"); } +- (void)testAddObject_ +{ + var ac = [self setupWithElements:ELEMENTS], + predicate = [ac filterPredicate], + content = [[ac content] copy]; + + // Add object while clearing the predicate. + [ac setClearsFilterPredicateOnInsertion:YES]; + + [ac setSortDescriptors:[ + [CPSortDescriptor sortDescriptorWithKey:"a" ascending:NO], + ]]; + + var start = (new Date).getTime(); + for (var i = 0; i < REPEATS / 2; i++) + { + [ac setFilterPredicate:predicate]; + [ac addObject:[Sortable sortableWithA:i B:i * 2]]; + + var sorted = [ac arrangedObjects], + last = ELEMENTS; + + // Verify that all is well. + for (var j = 0, count = [sorted count]; j < count; j++) + { + if (sorted[j].a >= last) + [self fail:"array values should be descending (position: " + j + ")"]; + last = sorted[j]; + } + } + var end = (new Date).getTime(); + CPLog.warn("testAddObject_, sorted, clear filter on insert: " + (end - start) + "ms"); + + [ac setClearsFilterPredicateOnInsertion:NO]; + [ac setFilterPredicate:predicate]; + + var start = (new Date).getTime(); + for (var i = 0; i < REPEATS; i++) + { + [ac addObject:[Sortable sortableWithA:i B:i % 3]]; + + var sorted = [ac arrangedObjects], + last = ELEMENTS; + + // Verify that all is well. + for (var j = 0, count = [sorted count]; j < count; j++) + { + if (sorted[j].b == 0) + [self fail:"b == 0 should be filtered out (position: " + j + ")"]; + if (sorted[j].a >= last) + [self fail:"array values should be descending (position: " + j + ")"]; + last = sorted[j]; + } + } + var end = (new Date).getTime(); + CPLog.warn("testAddObject_, sorted, filtered: " + (end - start) + "ms"); +} + @end @implementation Sortable : CPObject @@ -83,4 +146,12 @@ int b @accessors; } ++ (id)sortableWithA:(int)anA B:(int)aB +{ + var r = [Sortable new]; + r.a = anA; + r.b = aB; + return r; +} + @end \ No newline at end of file diff --git a/Tests/AppKit/CPArrayControllerTest.j b/Tests/AppKit/CPArrayControllerTest.j index 5d5a85157..992852b9f 100644 --- a/Tests/AppKit/CPArrayControllerTest.j +++ b/Tests/AppKit/CPArrayControllerTest.j @@ -412,6 +412,32 @@ [self assert:newSelection equals:[arrayController selectionIndexes] message:@"selection was not set properly"]; } +- (void)testObservationDuringAddObject_ +{ + var arrayController = [self arrayController]; + + [arrayController addObserver:self forKeyPath:@"arrangedObjects" options:CPKeyValueObservingOptionOld | CPKeyValueObservingOptionNew context:nil]; + + // Add something to clear. + [arrayController setFilterPredicate:[CPPredicate predicateWithFormat:@"(name != %@)", "Francisco"]]; + observations = []; + var aPerson = [Employee employeeWithName:@"Alexander" department:[Department departmentWithName:@"Cosmic Path Finding"]]; + + [arrayController setClearsFilterPredicateOnInsertion:NO]; + [self assert:0 equals:[observations count] message:@"no observations before addObject test"]; + [arrayController addObject:aPerson]; + [self assert:1 equals:[observations count] message:@"exactly 1 notification for addObject (clearsFilterPredicate NO)"]; + + observations = []; + + // Even that this is on, adding an object should only result in one notification. + [arrayController setClearsFilterPredicateOnInsertion:YES]; + + [self assert:0 equals:[observations count] message:@"no observations before addObject test"]; + [arrayController addObject:aPerson]; + [self assert:1 equals:[observations count] message:@"exactly 1 notification for addObject (clearsFilterPredicate YES)"]; +} + - (void)testCompoundKeyPaths { var departmentNameField = [[CPTextField alloc] init];