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

Reimplement ASRectTable using unordered_map to avoid obscure NSMapTable exception. #719

Merged
merged 3 commits into from
Dec 22, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 12 additions & 12 deletions AsyncDisplayKit.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,6 @@
C78F7E2B1BF7809800CDEAFC /* ASTableNode.h in Headers */ = {isa = PBXBuildFile; fileRef = B0F880581BEAEC7500D17647 /* ASTableNode.h */; settings = {ATTRIBUTES = (Public, ); }; };
CC034A091E60BEB400626263 /* ASDisplayNode+Convenience.h in Headers */ = {isa = PBXBuildFile; fileRef = CC034A071E60BEB400626263 /* ASDisplayNode+Convenience.h */; settings = {ATTRIBUTES = (Public, ); }; };
CC034A0A1E60BEB400626263 /* ASDisplayNode+Convenience.m in Sources */ = {isa = PBXBuildFile; fileRef = CC034A081E60BEB400626263 /* ASDisplayNode+Convenience.m */; };
CC034A101E60C9BF00626263 /* ASRectTableTests.m in Sources */ = {isa = PBXBuildFile; fileRef = CC034A0F1E60C9BF00626263 /* ASRectTableTests.m */; };
CC034A131E649F1300626263 /* AsyncDisplayKit+IGListKitMethods.h in Headers */ = {isa = PBXBuildFile; fileRef = CC034A111E649F1300626263 /* AsyncDisplayKit+IGListKitMethods.h */; settings = {ATTRIBUTES = (Public, ); }; };
CC034A141E649F1300626263 /* AsyncDisplayKit+IGListKitMethods.m in Sources */ = {isa = PBXBuildFile; fileRef = CC034A121E649F1300626263 /* AsyncDisplayKit+IGListKitMethods.m */; };
CC051F1F1D7A286A006434CB /* ASCALayerTests.m in Sources */ = {isa = PBXBuildFile; fileRef = CC051F1E1D7A286A006434CB /* ASCALayerTests.m */; };
Expand Down Expand Up @@ -431,6 +430,9 @@
DECBD6EA1BE56E1900CF4905 /* ASButtonNode.mm in Sources */ = {isa = PBXBuildFile; fileRef = DECBD6E61BE56E1900CF4905 /* ASButtonNode.mm */; };
DEFAD8131CC48914000527C4 /* ASVideoNode.mm in Sources */ = {isa = PBXBuildFile; fileRef = AEEC47E01C20C2DD00EC1693 /* ASVideoNode.mm */; };
E51B78BF1F028ABF00E32604 /* ASLayoutFlatteningTests.m in Sources */ = {isa = PBXBuildFile; fileRef = E51B78BD1F01A0EE00E32604 /* ASLayoutFlatteningTests.m */; };
E52AC9BA1FEA90EB00AA4040 /* ASRectMap.mm in Sources */ = {isa = PBXBuildFile; fileRef = E52AC9B81FEA90EB00AA4040 /* ASRectMap.mm */; };
E52AC9BB1FEA90EB00AA4040 /* ASRectMap.h in Headers */ = {isa = PBXBuildFile; fileRef = E52AC9B91FEA90EB00AA4040 /* ASRectMap.h */; settings = {ATTRIBUTES = (Private, ); }; };
E52AC9C01FEA916C00AA4040 /* ASRectMapTests.m in Sources */ = {isa = PBXBuildFile; fileRef = E52AC9BE1FEA915D00AA4040 /* ASRectMapTests.m */; };
E54E00721F1D3828000B30D7 /* ASPagerNode+Beta.h in Headers */ = {isa = PBXBuildFile; fileRef = E54E00711F1D3828000B30D7 /* ASPagerNode+Beta.h */; settings = {ATTRIBUTES = (Public, ); }; };
E54E81FC1EB357BD00FFE8E1 /* ASPageTable.h in Headers */ = {isa = PBXBuildFile; fileRef = E54E81FA1EB357BD00FFE8E1 /* ASPageTable.h */; };
E54E81FD1EB357BD00FFE8E1 /* ASPageTable.m in Sources */ = {isa = PBXBuildFile; fileRef = E54E81FB1EB357BD00FFE8E1 /* ASPageTable.m */; };
Expand All @@ -454,8 +456,6 @@
E58E9E461E941D74004CFC59 /* ASCollectionLayoutDelegate.h in Headers */ = {isa = PBXBuildFile; fileRef = E58E9E411E941D74004CFC59 /* ASCollectionLayoutDelegate.h */; settings = {ATTRIBUTES = (Public, ); }; };
E58E9E491E941DA5004CFC59 /* ASCollectionLayout.h in Headers */ = {isa = PBXBuildFile; fileRef = E58E9E471E941DA5004CFC59 /* ASCollectionLayout.h */; settings = {ATTRIBUTES = (Private, ); }; };
E58E9E4A1E941DA5004CFC59 /* ASCollectionLayout.mm in Sources */ = {isa = PBXBuildFile; fileRef = E58E9E481E941DA5004CFC59 /* ASCollectionLayout.mm */; };
E5ABAC7B1E8564EE007AC15C /* ASRectTable.h in Headers */ = {isa = PBXBuildFile; fileRef = E5ABAC791E8564EE007AC15C /* ASRectTable.h */; };
E5ABAC7C1E8564EE007AC15C /* ASRectTable.m in Sources */ = {isa = PBXBuildFile; fileRef = E5ABAC7A1E8564EE007AC15C /* ASRectTable.m */; };
E5B077FF1E69F4EB00C24B5B /* ASElementMap.h in Headers */ = {isa = PBXBuildFile; fileRef = E5B077FD1E69F4EB00C24B5B /* ASElementMap.h */; settings = {ATTRIBUTES = (Public, ); }; };
E5B078001E69F4EB00C24B5B /* ASElementMap.m in Sources */ = {isa = PBXBuildFile; fileRef = E5B077FE1E69F4EB00C24B5B /* ASElementMap.m */; };
E5B225281F1790D6001E1431 /* ASHashing.h in Headers */ = {isa = PBXBuildFile; fileRef = E5B225271F1790B5001E1431 /* ASHashing.h */; settings = {ATTRIBUTES = (Public, ); }; };
Expand Down Expand Up @@ -800,7 +800,6 @@
BDC2D162BD55A807C1475DA5 /* Pods-AsyncDisplayKitTests.profile.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-AsyncDisplayKitTests.profile.xcconfig"; path = "Pods/Target Support Files/Pods-AsyncDisplayKitTests/Pods-AsyncDisplayKitTests.profile.xcconfig"; sourceTree = "<group>"; };
CC034A071E60BEB400626263 /* ASDisplayNode+Convenience.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = "ASDisplayNode+Convenience.h"; sourceTree = "<group>"; };
CC034A081E60BEB400626263 /* ASDisplayNode+Convenience.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = "ASDisplayNode+Convenience.m"; sourceTree = "<group>"; };
CC034A0F1E60C9BF00626263 /* ASRectTableTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ASRectTableTests.m; sourceTree = "<group>"; };
CC034A111E649F1300626263 /* AsyncDisplayKit+IGListKitMethods.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = "AsyncDisplayKit+IGListKitMethods.h"; sourceTree = "<group>"; };
CC034A121E649F1300626263 /* AsyncDisplayKit+IGListKitMethods.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = "AsyncDisplayKit+IGListKitMethods.m"; sourceTree = "<group>"; };
CC051F1E1D7A286A006434CB /* ASCALayerTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ASCALayerTests.m; sourceTree = "<group>"; };
Expand Down Expand Up @@ -931,6 +930,9 @@
E51B78BD1F01A0EE00E32604 /* ASLayoutFlatteningTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ASLayoutFlatteningTests.m; sourceTree = "<group>"; };
E52405B21C8FEF03004DC8E7 /* ASLayoutTransition.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = ASLayoutTransition.mm; sourceTree = "<group>"; };
E52405B41C8FEF16004DC8E7 /* ASLayoutTransition.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ASLayoutTransition.h; sourceTree = "<group>"; };
E52AC9B81FEA90EB00AA4040 /* ASRectMap.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = ASRectMap.mm; sourceTree = "<group>"; };
E52AC9B91FEA90EB00AA4040 /* ASRectMap.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ASRectMap.h; sourceTree = "<group>"; };
E52AC9BE1FEA915D00AA4040 /* ASRectMapTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ASRectMapTests.m; sourceTree = "<group>"; };
E54E00711F1D3828000B30D7 /* ASPagerNode+Beta.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = "ASPagerNode+Beta.h"; sourceTree = "<group>"; };
E54E81FA1EB357BD00FFE8E1 /* ASPageTable.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ASPageTable.h; sourceTree = "<group>"; };
E54E81FB1EB357BD00FFE8E1 /* ASPageTable.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ASPageTable.m; sourceTree = "<group>"; };
Expand All @@ -954,8 +956,6 @@
E58E9E411E941D74004CFC59 /* ASCollectionLayoutDelegate.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ASCollectionLayoutDelegate.h; sourceTree = "<group>"; };
E58E9E471E941DA5004CFC59 /* ASCollectionLayout.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ASCollectionLayout.h; sourceTree = "<group>"; };
E58E9E481E941DA5004CFC59 /* ASCollectionLayout.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = ASCollectionLayout.mm; sourceTree = "<group>"; };
E5ABAC791E8564EE007AC15C /* ASRectTable.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ASRectTable.h; sourceTree = "<group>"; };
E5ABAC7A1E8564EE007AC15C /* ASRectTable.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ASRectTable.m; sourceTree = "<group>"; };
E5B077FD1E69F4EB00C24B5B /* ASElementMap.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ASElementMap.h; sourceTree = "<group>"; };
E5B077FE1E69F4EB00C24B5B /* ASElementMap.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ASElementMap.m; sourceTree = "<group>"; };
E5B225261F1790B5001E1431 /* ASHashing.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = ASHashing.m; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1179,7 +1179,6 @@
CC583ABF1EF9BAB400134156 /* Common */,
CCDD148A1EEDCD9D0020834E /* ASCollectionModernDataSourceTests.m */,
BB5FC3CD1F9BA688007F191E /* ASNavigationControllerTests.m */,
CC034A0F1E60C9BF00626263 /* ASRectTableTests.m */,
BB5FC3D01F9C9389007F191E /* ASTabBarControllerTests.m */,
CC11F9791DB181180024D77B /* ASNetworkImageNodeTests.m */,
CC051F1E1D7A286A006434CB /* ASCALayerTests.m */,
Expand All @@ -1196,6 +1195,7 @@
CCA221D21D6FA7EF00AF6A0F /* ASViewControllerTests.m */,
CC0AEEA31D66316E005D1C78 /* ASUICollectionViewTests.m */,
CCB2F34C1D63CCC6004E6DE9 /* ASDisplayNodeSnapshotTests.m */,
E52AC9BE1FEA915D00AA4040 /* ASRectMapTests.m */,
83A7D95D1D446A6E00BF333E /* ASWeakMapTests.m */,
DBC453211C5FD97200B16017 /* ASDisplayNodeImplicitHierarchyTests.m */,
DBC452DD1C5C6A6A00B16017 /* ArrayDiffingTests.m */,
Expand Down Expand Up @@ -1383,8 +1383,8 @@
CCA282B31E9EA7310037E8B7 /* ASTipsController.m */,
CC2F65EC1E5FFB1600DA57C9 /* ASMutableElementMap.h */,
CC2F65ED1E5FFB1600DA57C9 /* ASMutableElementMap.m */,
E5ABAC791E8564EE007AC15C /* ASRectTable.h */,
E5ABAC7A1E8564EE007AC15C /* ASRectTable.m */,
E52AC9B91FEA90EB00AA4040 /* ASRectMap.h */,
E52AC9B81FEA90EB00AA4040 /* ASRectMap.mm */,
CC55A70F1E52A0F200594372 /* ASResponderChainEnumerator.h */,
CC55A7101E52A0F200594372 /* ASResponderChainEnumerator.m */,
6947B0BB1E36B4E30007C478 /* Layout */,
Expand Down Expand Up @@ -1857,6 +1857,7 @@
E5775B021F16759300CAC9BC /* ASCollectionLayoutCache.h in Headers */,
E5775B001F13D25400CAC9BC /* ASCollectionLayoutState+Private.h in Headers */,
E5667E8C1F33871300FA6FC0 /* _ASCollectionGalleryLayoutInfo.h in Headers */,
E52AC9BB1FEA90EB00AA4040 /* ASRectMap.h in Headers */,
E5775AFC1F13CE9F00CAC9BC /* _ASCollectionGalleryLayoutItem.h in Headers */,
E5855DF01EBB4D83003639AE /* ASCollectionLayoutDefines.h in Headers */,
E5B5B9D11E9BAD9800A6B726 /* ASCollectionLayoutContext+Private.h in Headers */,
Expand Down Expand Up @@ -1918,7 +1919,6 @@
B35062061B010EFD0018CF92 /* ASNetworkImageNode.h in Headers */,
CCA282C81E9EB64B0037E8B7 /* ASDisplayNodeTipState.h in Headers */,
34EFC76C1B701CED00AD841F /* ASOverlayLayoutSpec.h in Headers */,
E5ABAC7B1E8564EE007AC15C /* ASRectTable.h in Headers */,
B35062261B010EFD0018CF92 /* ASRangeController.h in Headers */,
34EFC76E1B701CF400AD841F /* ASRatioLayoutSpec.h in Headers */,
DB55C2671C641AE4004EDCF5 /* ASContextTransitioning.h in Headers */,
Expand Down Expand Up @@ -2165,7 +2165,6 @@
9F06E5CD1B4CAF4200F015D8 /* ASCollectionViewTests.mm in Sources */,
2911485C1A77147A005D0878 /* ASControlNodeTests.m in Sources */,
CC3B208E1C3F7D0A00798563 /* ASWeakSetTests.m in Sources */,
CC034A101E60C9BF00626263 /* ASRectTableTests.m in Sources */,
F711994E1D20C21100568860 /* ASDisplayNodeExtrasTests.m in Sources */,
BB5FC3CE1F9BA689007F191E /* ASNavigationControllerTests.m in Sources */,
ACF6ED5D1B178DC700DA7C62 /* ASDimensionTests.mm in Sources */,
Expand Down Expand Up @@ -2211,6 +2210,7 @@
254C6B541BF8FF2A003EC431 /* ASTextKitTests.mm in Sources */,
05EA6FE71AC0966E00E35788 /* ASSnapshotTestCase.m in Sources */,
ACF6ED631B178DC700DA7C62 /* ASStackLayoutSpecSnapshotTests.mm in Sources */,
E52AC9C01FEA916C00AA4040 /* ASRectMapTests.m in Sources */,
CCE4F9BA1F0DBB5000062E4E /* ASLayoutTestNode.mm in Sources */,
81E95C141D62639600336598 /* ASTextNodeSnapshotTests.m in Sources */,
3C9C128519E616EF00E942A0 /* ASTableViewTests.mm in Sources */,
Expand Down Expand Up @@ -2239,6 +2239,7 @@
3917EBD51E9C2FC400D04A01 /* _ASCollectionReusableView.m in Sources */,
CCA282D11E9EBF6C0037E8B7 /* ASTipsWindow.m in Sources */,
CCCCCCE41EC3EF060087FE10 /* NSParagraphStyle+ASText.m in Sources */,
E52AC9BA1FEA90EB00AA4040 /* ASRectMap.mm in Sources */,
8BBBAB8D1CEBAF1E00107FC6 /* ASDefaultPlaybackButton.m in Sources */,
B30BF6541C59D889004FCD53 /* ASLayoutManager.m in Sources */,
92DD2FE71BF4D0850074C9DD /* ASMapNode.mm in Sources */,
Expand Down Expand Up @@ -2351,7 +2352,6 @@
254C6B8B1BF94F8A003EC431 /* ASTextKitShadower.mm in Sources */,
254C6B851BF94F8A003EC431 /* ASTextKitAttributes.mm in Sources */,
90FC784F1E4BFE1B00383C5A /* ASDisplayNode+Yoga.mm in Sources */,
E5ABAC7C1E8564EE007AC15C /* ASRectTable.m in Sources */,
CCA282C91E9EB64B0037E8B7 /* ASDisplayNodeTipState.m in Sources */,
509E68601B3AED8E009B9150 /* ASScrollDirection.m in Sources */,
B35062091B010EFD0018CF92 /* ASScrollNode.mm in Sources */,
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
## master
* Add your own contributions to the next release on the line below this with your name.
- [ASRectMap] Replace implementation of ASRectTable with a simpler one based on unordered_map.[Scott Goodson](https://github.com/appleguy) [#719](https://github.com/TextureGroup/Texture/pull/719)
- [ASCollectionView] Add missing flags for ASCollectionDelegate [Ilya Zheleznikov](https://github.com/ilyailya) [#718](https://github.com/TextureGroup/Texture/pull/718)
- [ASNetworkImageNode] Deprecates .URLs in favor of .URL [Garrett Moon](https://github.com/garrettmoon) [#699](https://github.com/TextureGroup/Texture/pull/699)
- [iOS11] Update project settings and fix errors [Eke](https://github.com/Eke) [#676](https://github.com/TextureGroup/Texture/pull/676)
Expand Down
10 changes: 5 additions & 5 deletions Source/Layout/ASLayout.mm
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
#import <AsyncDisplayKit/ASEqualityHelpers.h>
#import <AsyncDisplayKit/ASInternalHelpers.h>
#import <AsyncDisplayKit/ASObjectDescriptionHelpers.h>
#import <AsyncDisplayKit/ASRectTable.h>
#import <AsyncDisplayKit/ASRectMap.h>

CGPoint const ASPointNull = {NAN, NAN};

Expand Down Expand Up @@ -86,7 +86,7 @@ @interface ASLayout () <ASDescriptionProvider>
*/
@property (nonatomic, strong) NSMutableArray<id<ASLayoutElement>> *sublayoutLayoutElements;

@property (nonatomic, strong, readonly) ASRectTable<id<ASLayoutElement>, id> *elementToRectTable;
@property (nonatomic, strong, readonly) ASRectMap *elementToRectMap;

@end

Expand Down Expand Up @@ -143,9 +143,9 @@ - (instancetype)initWithLayoutElement:(id<ASLayoutElement>)layoutElement
_sublayouts = sublayouts != nil ? [sublayouts copy] : @[];

if (_sublayouts.count > 0) {
_elementToRectTable = [ASRectTable rectTableForWeakObjectPointers];
_elementToRectMap = [ASRectMap rectMapForWeakObjectPointers];
for (ASLayout *layout in sublayouts) {
[_elementToRectTable setRect:layout.frame forKey:layout.layoutElement];
[_elementToRectMap setRect:layout.frame forKey:layout.layoutElement];
}
}

Expand Down Expand Up @@ -303,7 +303,7 @@ - (ASLayoutElementType)type

- (CGRect)frameForElement:(id<ASLayoutElement>)layoutElement
{
return _elementToRectTable ? [_elementToRectTable rectForKey:layoutElement] : CGRectNull;
return _elementToRectMap ? [_elementToRectMap rectForKey:layoutElement] : CGRectNull;
}

- (CGRect)frame
Expand Down
52 changes: 52 additions & 0 deletions Source/Private/ASRectMap.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
//
// ASRectMap.h
// Texture
//
// Copyright (c) 2017-present, Pinterest, Inc. All rights reserved.
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//

#import <Foundation/Foundation.h>
#import <CoreGraphics/CGGeometry.h>

NS_ASSUME_NONNULL_BEGIN

/**
* A category for indexing weak pointers to CGRects. Similar to ASIntegerMap.
*/
@interface ASRectMap : NSObject

/**
* Creates a new rect map. The keys are never retained.
*/
+ (ASRectMap *)rectMapForWeakObjectPointers;

/**
* Retrieves the rect for a given key, or CGRectNull if the key is not found.
*
* @param key An object to lookup the rect for.
*/
- (CGRect)rectForKey:(id)key;

/**
* Sets the given rect for the associated key. Key *will not be retained!*
*
* @param rect The rect to store as value.
* @param key The key to use for the rect.
*/
- (void)setRect:(CGRect)rect forKey:(id)key;

/**
* Removes the rect for the given key, if one exists.
*
* @param key The key to remove.
*/
- (void)removeRectForKey:(id)key;

@end

NS_ASSUME_NONNULL_END
78 changes: 78 additions & 0 deletions Source/Private/ASRectMap.mm
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
//
// ASRectMap.mm
// Texture
//
// Copyright (c) 2017-present, Pinterest, Inc. All rights reserved.
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update this file license per Danger's comment.

//
//  ASRectMap.mm
//  Texture
//
//  Copyright (c) 2017-present, Pinterest, Inc.  All rights reserved.
//  Licensed under the Apache License, Version 2.0 (the "License");
//  you may not use this file except in compliance with the License.
//  You may obtain a copy of the License at
//
//      http://www.apache.org/licenses/LICENSE-2.0
//


#import "ASRectMap.h"
#import "ASObjectDescriptionHelpers.h"
#import <UIKit/UIGeometry.h>
#import <unordered_map>

@implementation ASRectMap {
std::unordered_map<void *, CGRect> _map;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may should add a comment that we use a void * key here as arc would not be able to determine the ownership qualification.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we not use __unsafe_unretained id as the key type here?

Copy link
Contributor

@maicki maicki Dec 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately I don't think we can use __unsafe_unretained id as key. The unordered_map template cannot figure out the hash for the __unsafe_unretained id key. We would have to write our own key like:

struct ASRectMapMapKey {
    __unsafe_unretained id obj;
    
    bool operator==(const ASRectMapMapKey &other) const
    {
        return [obj isEqual:other.obj];
    }
};

namespace std {
    template<> struct hash<ASRectMapMapKey>
    {
        size_t operator()(const ASRectMapMapKey &k) const
        {
            return [k.obj hash];
        }
    };
}

And we could use it via:

std::unordered_map<ASRectMapMapKey, CGRect> map;
map.insert({{@"key"}, CGRectMake(0, 0, 100, 100)});
NSLog(@"%@", NSStringFromRect(map.at({@"key"})));

}

+ (ASRectMap *)rectMapForWeakObjectPointers
{
return [[self alloc] init];
}

- (CGRect)rectForKey:(id)key
{
auto result = _map.find((__bridge void *)key);
if (result != _map.end()) {
// result->first is the key; result->second is the value, a CGRect.
return result->second;
} else {
return CGRectNull;
}
}

- (void)setRect:(CGRect)rect forKey:(id)key
{
if (key) {
_map[(__bridge void *)key] = rect;
}
}

- (void)removeRectForKey:(id)key
{
if (key) {
_map.erase((__bridge void *)key);
}
}

- (id)copyWithZone:(NSZone *)zone
{
ASRectMap *copy = [ASRectMap rectMapForWeakObjectPointers];
copy->_map = _map;
return copy;
}

- (NSMutableArray<NSDictionary *> *)propertiesForDescription
{
NSMutableArray *result = [NSMutableArray array];

// { ptr1->rect1 ptr2->rect2 ptr3->rect3 }
NSMutableString *str = [NSMutableString string];
for (auto it = _map.begin(); it != _map.end(); it++) {
Copy link
Contributor

@maicki maicki Dec 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could also use of a bit of a smaller code:

for ( auto const& entry : _map ) {
    [str appendFormat:@" %@->%@", entry->first, NSStringFromCGRect(entry->second)];
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this method dangerous, now that the keys are stored unretained? Could we be calling -description on a garbage pointer? We could fall back to %p.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with @Adlai-Holler. Let's pay extra attention here.

[str appendFormat:@" %@->%@", it->first, NSStringFromCGRect(it->second)];
}
[result addObject:@{ @"ASRectMap": str }];

return result;
Copy link
Member

@Adlai-Holler Adlai-Holler Dec 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The class name will already be in the description by default, so I think what we want here is @{ (id)kCFNull : str } which will just dump the list of pointers inside e.g.: <ASRectMap: 0xFFFFFFFF; "a->b, c->d">

Edit: NM I see you used MakeWithoutObject so we exclude the pointer from the description a la other collection classes 👍

}

- (NSString *)description
{
return ASObjectDescriptionMakeWithoutObject([self propertiesForDescription]);
}

@end
Loading