Skip to content

Commit

Permalink
Restore Cocoa's whacky layout in the Mac app launcher since it has le…
Browse files Browse the repository at this point in the history
…ss bad.

This CL reverts r267186 and r243075 which tried to work around Cocoa's
buggy layout in an NSCollectionView when it is first drawn during a
scroll. On balance, the workarounds make things worse - we will just
have to put up with jiggling apps and crappy text anti-aliasing until
the toolkit-views launcher is fully ported to mac.

The jiggling and bad text AA result because NSCollectionView incorrectly
gives a nonintegral frameForItemAtIndex when its enclosing scroll view
is mid-scroll.

r243075 doesn't properly handle removals when the launcher isn't visible.
r267186 tried to fix that but broke the Cocoa-provided animations.

via git:
Revert "Ensure the Mac app_list's initial NSCollectionView layouts obey frameForItemAtIndex:"
Revert "Ensure stale values are not used in Mac App Launcher layout."

BUG=364495,329454

Review URL: https://codereview.chromium.org/273563002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@268888 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
tapted@chromium.org committed May 7, 2014
1 parent 69a0a37 commit 3754d59
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 63 deletions.
49 changes: 22 additions & 27 deletions ui/app_list/cocoa/apps_grid_controller.mm
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,8 @@ - (NSView*)pagesContainerView;
// Create any new pages after updating |items_|.
- (void)updatePages:(size_t)startItemIndex;

- (void)updatePageContent:(size_t)pageIndex;
- (void)updatePageContent:(size_t)pageIndex
resetModel:(BOOL)resetModel;

// Bridged methods for AppListItemListObserver.
- (void)listItemAdded:(size_t)index
Expand All @@ -87,11 +88,6 @@ - (void)listItemMovedFromIndex:(size_t)fromIndex
// Moves the selection by |indexDelta| items.
- (BOOL)moveSelectionByDelta:(int)indexDelta;

// -[NSCollectionView frameForItemAtIndex:] misreports the frame origin of an
// item when the method is called during a scroll animation provided by the
// NSScrollView. This returns the correct value.
- (NSRect)trueFrameForItemAtIndex:(size_t)itemIndex;

@end

namespace app_list {
Expand Down Expand Up @@ -458,32 +454,38 @@ - (void)updatePages:(size_t)startItemIndex {

const size_t startPage = startItemIndex / kItemsPerPage;
// All pages on or after |startPage| may need items added or removed.
for (size_t pageIndex = startPage; pageIndex < targetPages; ++pageIndex)
[self updatePageContent:pageIndex];
for (size_t pageIndex = startPage; pageIndex < targetPages; ++pageIndex) {
[self updatePageContent:pageIndex
resetModel:YES];
}
}

- (void)updatePageContent:(size_t)pageIndex {
- (void)updatePageContent:(size_t)pageIndex
resetModel:(BOOL)resetModel {
NSCollectionView* pageView = [self collectionViewAtPageIndex:pageIndex];
// Clear the models first, otherwise removed items could be autoreleased at
// an unknown point in the future, when the model owner may have gone away.
for (size_t i = 0; i < [[pageView content] count]; ++i) {
AppsGridViewItem* gridItem =
base::mac::ObjCCastStrict<AppsGridViewItem>([pageView itemAtIndex:i]);
[gridItem setModel:NULL];
if (resetModel) {
// Clear the models first, otherwise removed items could be autoreleased at
// an unknown point in the future, when the model owner may have gone away.
for (size_t i = 0; i < [[pageView content] count]; ++i) {
AppsGridViewItem* gridItem = base::mac::ObjCCastStrict<AppsGridViewItem>(
[pageView itemAtIndex:i]);
[gridItem setModel:NULL];
}
}

NSRange inPageRange = NSIntersectionRange(
NSMakeRange(pageIndex * kItemsPerPage, kItemsPerPage),
NSMakeRange(0, [items_ count]));
NSArray* pageContent = [items_ subarrayWithRange:inPageRange];
[pageView setContent:pageContent];
if (!resetModel)
return;

for (size_t i = 0; i < [pageContent count]; ++i) {
AppsGridViewItem* gridItem = base::mac::ObjCCastStrict<AppsGridViewItem>(
[pageView itemAtIndex:i]);
[gridItem setModel:static_cast<app_list::AppListItem*>(
[[pageContent objectAtIndex:i] pointerValue])];
[gridItem setInitialFrameRect:[self trueFrameForItemAtIndex:i]];
}
}

Expand All @@ -498,15 +500,17 @@ - (void)moveItemInView:(size_t)fromIndex
size_t fromPageIndex = fromIndex / kItemsPerPage;
size_t toPageIndex = toIndex / kItemsPerPage;
if (fromPageIndex == toPageIndex) {
[self updatePageContent:fromPageIndex];
[self updatePageContent:fromPageIndex
resetModel:NO]; // Just reorder items.
return;
}

if (fromPageIndex > toPageIndex)
std::swap(fromPageIndex, toPageIndex);

for (size_t i = fromPageIndex; i <= toPageIndex; ++i) {
[self updatePageContent:i];
[self updatePageContent:i
resetModel:YES];
}
}

Expand Down Expand Up @@ -611,15 +615,6 @@ - (BOOL)moveSelectionByDelta:(int)indexDelta {
return YES;
}

- (NSRect)trueFrameForItemAtIndex:(size_t)itemIndex {
size_t column = itemIndex % kFixedColumns;
size_t row = itemIndex % kItemsPerPage / kFixedColumns;
return NSMakeRect(column * kPreferredTileWidth,
row * kPreferredTileHeight,
kPreferredTileWidth,
kPreferredTileHeight);
}

- (void)selectItemAtIndex:(NSUInteger)index {
if (index >= [items_ count])
return;
Expand Down
9 changes: 0 additions & 9 deletions ui/app_list/cocoa/apps_grid_view_item.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,6 @@ APP_LIST_EXPORT
// Set the represented model, updating views. Clears if |itemModel| is NULL.
- (void)setModel:(app_list::AppListItem*)itemModel;

// Set the frame that will be used the first time the NSCollectionView performs
// layout on the item.
// This is required because the first time an NSCollectionView becomes visible,
// it performs a layout, and it can attempt to set a frame that differs from the
// -[NSCollectionView frameForItemAtIndex:] reported when the cell was created.
// Worse, this frame can have a non-integral origin, leading to graphical
// glitches because the content is no longer pixel-aligned.
- (void)setInitialFrameRect:(NSRect)frameRect;

// Model accessor, via the |observerBridge_|.
- (app_list::AppListItem*)model;

Expand Down
27 changes: 0 additions & 27 deletions ui/app_list/cocoa/apps_grid_view_item.mm
Original file line number Diff line number Diff line change
Expand Up @@ -137,22 +137,13 @@ - (void)setPercentDownloaded:(int)percent;
// grid, and to draw with a highlight when selected.
@interface AppsGridItemBackgroundView : NSView {
@private
// Whether the item is selected, and draws a background.
BOOL selected_;

// Whether to intercept the next call to -[NSView setFrame:] and override it.
BOOL overrideNextSetFrame_;

// The frame given to -[super setFrame:], when |overrideNextSetFrame_| is set.
NSRect overrideFrame_;
}

- (NSButton*)button;

- (void)setSelected:(BOOL)flag;

- (void)setOneshotFrameRect:(NSRect)frameRect;

@end

@interface AppsGridItemButtonCell : NSButtonCell {
Expand Down Expand Up @@ -182,20 +173,6 @@ - (void)setSelected:(BOOL)flag {
[self setNeedsDisplay:YES];
}

- (void)setOneshotFrameRect:(NSRect)frameRect {
[super setFrame:frameRect];
overrideNextSetFrame_ = YES;
overrideFrame_ = frameRect;
}

- (void)setFrame:(NSRect)frameRect {
if (overrideNextSetFrame_) {
frameRect = overrideFrame_;
overrideNextSetFrame_ = NO;
}
[super setFrame:frameRect];
}

// Ignore all hit tests. The grid controller needs to be the owner of any drags.
- (NSView*)hitTest:(NSPoint)aPoint {
return nil;
Expand Down Expand Up @@ -334,10 +311,6 @@ - (void)setModel:(app_list::AppListItem*)itemModel {
[[self view] addTrackingArea:trackingArea_.get()];
}

- (void)setInitialFrameRect:(NSRect)frameRect {
[[self itemBackgroundView] setOneshotFrameRect:frameRect];
}

- (app_list::AppListItem*)model {
return observerBridge_->model();
}
Expand Down

0 comments on commit 3754d59

Please sign in to comment.