[tvOS] Fixes errors when building against tvOS SDK #728

alexhillc · 2017-12-26T00:44:47Z

Hello!

I decided to look into what needed fixing for tvOS support and discovered #653, which doesn't have any recent activity... so I thought that going through and fixing the build errors that the project has would be a good start.

UIViewAnimationOptions

ghost · 2017-12-28T10:14:52Z

🚫 CI failed with log

[ASMultiplexImageNode] Enables support for Photos framework on tvOS 10+ [ASMultiplexImageNode] Fixes comment depth [ASAvailability] Adjust logic in AS_AVAILABLE_IOS_TVOS to account for both versions Adjusts API_AVAILABLE to minimum deployment target

the built-in solution (more accurately target OS by checking target) Change AS_AVAILABLE_IOS -> AS_AVAILABLE_IOS_TVOS in places that shoud allow for both [ASAvailability] Simplify AS_AVAILABLE_IOS_TVOS

when targeting tvOS

ghost · 2017-12-29T07:13:00Z

🚫 CI failed with log

overrides. Removes methods already implemented in ASDisplayNode+UIViewBridge category. [ASControlNode] Moves tvOS category declaration to ASControlNode header [ASImageNode] Moves tvOS category declaration to ASImageNode header [ASControlNode+Private] Adds private category for ASControlNode to access private selectors

alexhillc · 2017-12-30T16:45:56Z

Source/Private/TextExperiment/Utility/NSParagraphStyle+ASText.m

@@ -25,13 +25,15 @@ + (NSParagraphStyle *)as_styleWithCTStyle:(CTParagraphStyleRef)CTStyle {

  NSMutableParagraphStyle *style = [[NSParagraphStyle defaultParagraphStyle] mutableCopy];

+#if TARGET_OS_IOS
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wdeprecated-declarations"
  CGFloat lineSpacing;
  if (CTParagraphStyleGetValueForSpecifier(CTStyle, kCTParagraphStyleSpecifierLineSpacing, sizeof(CGFloat), &lineSpacing)) {


Need to add back support for line spacing on tvOS since kCTParagraphStyleSpecifierLineSpacing is unavailable on tvOS.

alexhillc · 2017-12-30T16:47:20Z

Source/TextKit/ASTextKitCoreTextAdditions.m

@@ -236,6 +237,7 @@ + (instancetype)paragraphStyleWithCTParagraphStyle:(CTParagraphStyleRef)coreText
    if (CTParagraphStyleGetValueForSpecifier(coreTextParagraphStyle, kCTParagraphStyleSpecifierLineSpacing, sizeof(lineSpacing), &lineSpacing))


Need to add back support for line spacing on tvOS since kCTParagraphStyleSpecifierLineSpacing is unavailable on tvOS.

…fierMinimumLineSpacing, kCTParagraphStyleSpecifierMaximumLineSpacing, kCTParagraphStyleSpecifierLineSpacingAdjustment when mapping CTParagraphStyle onto NSParagraphStyle [ASTextNode] Uses CoreText-cleansed attributed string when assigning ascender/descender to avoid crash when a CTParagraphStyle is passed as an attribute

[ASImageNode+tvOS] Add missing Foundation import (whoops!)

evaluates to an NSParagraphStyle, pass through to cleansed attributes. This fixes a bug that would occur if a CTParagraphStyle was passed as an attribute _alone_ (would not be caught by unsupported attributes check)

nguyenhuy · 2018-01-16T14:24:26Z

@alexhillc Awesome to see you taking the lead here. I know @Eke has been working on tvOS support for a while. @Eke Do you mind reviewing this diff?

Eke · 2018-01-16T15:53:54Z

@alexhillc great job! I'm happy that someone managed to make this happen! I will compare it to my local changes, maybe we can collaborate on some parts?

@nguyenhuy i will try to review it tomorrow

UPD:
@alexhillc for example i replaced UITapGestureRecognizer for control taps with Low-Level Event Handling and etc.

alexhillc · 2018-01-16T17:48:51Z

@Eke, I'd be more than happy to collaborate on this. :) I'd also be interested to see your local changes as well. I didn't make any changes to the functionality of the existing tvOS code yet so I'd be interested to see how you're going about handling it.

I have also added ASTableNode + ASCollectionNode support for the focus API in my master branch, but wanted to see this PR through before opening another!

Eke · 2018-01-21T18:30:51Z

Hello @alexhillc @nguyenhuy !

Sorry for delay, I had very busy week and i only managed to try building my project today.

Both tvOS and iOS versions build good with this changes.

I think this PR is big step forward for tvOS support implementation.

Tomorrow morning i will review code changes as well.

We had small conversation with @alexhillc and we will collaborate for further changes.

appleguy · 2018-02-09T19:16:02Z

@alexhillc I'm really grateful that you've worked on this! I think tvOS is a great platform for Texture and will meaningfully improve the UX of apps on the platform. Especially preloading will work very well, since the home internet connections can easily handle the higher bandwidth requirements, and it's worth that user experience win of thumbnails being loaded before they come onscreen (which many tvOS apps don't do today).

appleguy · 2018-02-09T19:17:05Z

Source/ASControlNode.h

 #if TARGET_OS_TV
+@interface ASControlNode (tvOS)


Thanks for adding categories, this is a nice improvement for organization.

appleguy · 2018-02-09T19:20:11Z

Source/ASTextNode.mm

@@ -450,10 +450,10 @@ - (void)setAttributedText:(NSAttributedString *)attributedText
  // Since truncation text matches style of attributedText, invalidate it now.
  [self _invalidateTruncationText];

-  NSUInteger length = attributedText.length;
+  NSUInteger length = _attributedText.length;


Interesting fix, this could be important in the case of cleansing attributed text...

appleguy · 2018-02-09T19:20:59Z

Source/TextKit/ASTextKitCoreTextAdditions.m

+      if ([coreTextValue isKindOfClass:[NSParagraphStyle class]]) {
+        cleanAttributes[NSParagraphStyleAttributeName] = (NSParagraphStyle *)coreTextValue;
+      }
+      else {


Please adjust if-else style to match project; else { should be on the line with the }.

Hmm, I guess other parts of this file are not conforming either. So this is probably OK.

appleguy · 2018-02-09T19:22:17Z

Source/TextKit/ASTextKitCoreTextAdditions.m

+  //   - kCTParagraphStyleSpecifierLineSpacingAdjustment
+  if (fabs(lineSpacing) <= FLT_EPSILON &&
+      CTParagraphStyleGetValueForSpecifier(coreTextParagraphStyle, kCTParagraphStyleSpecifierMinimumLineSpacing, sizeof(lineSpacing), &lineSpacing))
+    newParagraphStyle.lineSpacing = lineSpacing;


All if statements should use braces, even for one line. I'd suggest doing that for these statements even if the rest of the file doesn't / did not previously, since the conditions are so long.

Maybe worth making a macro for these checks, or at least a local variable for the FLT_EPSILON check.

appleguy · 2018-02-09T19:23:31Z

Texture.podspec

-
-  # Uncomment when fixed: issues with tvOS build for release 2.0
-  # spec.tvos.deployment_target = '9.0'
+  spec.tvos.deployment_target = '9.0'


Adlai-Holler

This was a ton of work, but tvOS is exciting! Thanks @alexhillc

appleguy · 2018-02-09T19:26:56Z

Source/ASTableView.mm

    self.separatorInset = node.separatorInset;
+#endif
    self.selectionStyle = node.selectionStyle; 
    self.focusStyle = node.focusStyle;


@alexhillc I had to do a manual merge of this file. Does this code look correct now / still?

@appleguy, looks correct to me

alexhillc · 2018-02-14T17:20:26Z

I’ll get back to this soon, been busy lately but will hopefully find some time later this week

appleguy · 2018-02-19T23:51:40Z

@alexhillc let us know when you're ready to go and I'm happy to merge this. BTW I did try building it locally with a hacked version of ASDKgram to build for tvOS, and no build failures encountered :). I did hit a weird runtime error with an assertion about lock handling though, so I'm still looking at that (but I don't think it's caused by this PR).

maicki

Nice work!

alexhillc · 2018-02-24T02:03:09Z

@appleguy, just made some small changes to formatting as requested -- I'm ready 👍
Looking forward to adding more support for tvOS!

ghost · 2018-02-28T12:35:21Z

🚫 CI failed with log

ghost · 2018-03-04T01:32:11Z

	4 Warnings
⚠️	Please ensure license is correct for ASTextAttribute.m: // // ASTextAttribute.m // Texture // // Copyright (c) 2014-present, Facebook, Inc. All rights reserved. // This source code is licensed under the BSD-style license found in the // LICENSE file in the /ASDK-Licenses directory of this source tree. An additional // grant of patent rights can be found in the PATENTS file in the same directory. // // Modifications to this file made after 4/13/2017 are: Copyright (c) through the present, // Pinterest, Inc. 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 //
⚠️	Please ensure license is correct for NSAttributedString+ASText.h: // // NSAttributedString+ASText.h // Texture // // Copyright (c) 2014-present, Facebook, Inc. All rights reserved. // This source code is licensed under the BSD-style license found in the // LICENSE file in the /ASDK-Licenses directory of this source tree. An additional // grant of patent rights can be found in the PATENTS file in the same directory. // // Modifications to this file made after 4/13/2017 are: Copyright (c) through the present, // Pinterest, Inc. 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 //
⚠️	Please ensure license is correct for NSAttributedString+ASText.m: // // NSAttributedString+ASText.m // Texture // // Copyright (c) 2014-present, Facebook, Inc. All rights reserved. // This source code is licensed under the BSD-style license found in the // LICENSE file in the /ASDK-Licenses directory of this source tree. An additional // grant of patent rights can be found in the PATENTS file in the same directory. // // Modifications to this file made after 4/13/2017 are: Copyright (c) through the present, // Pinterest, Inc. 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 //
⚠️	Please ensure license is correct for NSParagraphStyle+ASText.m: // // NSParagraphStyle+ASText.m // Texture // // Copyright (c) 2014-present, Facebook, Inc. All rights reserved. // This source code is licensed under the BSD-style license found in the // LICENSE file in the /ASDK-Licenses directory of this source tree. An additional // grant of patent rights can be found in the PATENTS file in the same directory. // // Modifications to this file made after 4/13/2017 are: Copyright (c) through the present, // Pinterest, Inc. 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 //

Generated by 🚫 Danger

@available

* [tvOS] Fixes errors when building against tvOS SDK * Update CHANGELOG.md * [tvOS] Fixes implicit conversion between UIViewAnimationCurve + UIViewAnimationOptions * Enable tvOS deployment target in Texture.podspec (for CI) * [ASMultiplexImageNode] Fixes typo * [tvOS] Fixes warnings related to @available guards in Xcode 9 [ASMultiplexImageNode] Enables support for Photos framework on tvOS 10+ [ASMultiplexImageNode] Fixes comment depth [ASAvailability] Adjust logic in AS_AVAILABLE_IOS_TVOS to account for both versions Adjusts API_AVAILABLE to minimum deployment target * [ASAvailability] Update AS_AVAILABLE_XXX fallbacks to function more like the built-in solution (more accurately target OS by checking target) Change AS_AVAILABLE_IOS -> AS_AVAILABLE_IOS_TVOS in places that shoud allow for both [ASAvailability] Simplify AS_AVAILABLE_IOS_TVOS * [ASControlNode] Adds missing 'super' call in -[ASControlNode didLoad] when targeting tvOS * Fix API_AVAILABLE iOS requirement * [ASDisplayNode] Fixes last of the linker warnings related to category overrides. Removes methods already implemented in ASDisplayNode+UIViewBridge category. [ASControlNode] Moves tvOS category declaration to ASControlNode header [ASImageNode] Moves tvOS category declaration to ASImageNode header [ASControlNode+Private] Adds private category for ASControlNode to access private selectors * [NSParagraphStyle+ASText] Fixes typo related to testing * [ASControlNode] Re-add helpful comment * [ASTextKitCoreTextAdditions] Adds mappings for kCTParagraphStyleSpecifierMinimumLineSpacing, kCTParagraphStyleSpecifierMaximumLineSpacing, kCTParagraphStyleSpecifierLineSpacingAdjustment when mapping CTParagraphStyle onto NSParagraphStyle [ASTextNode] Uses CoreText-cleansed attributed string when assigning ascender/descender to avoid crash when a CTParagraphStyle is passed as an attribute * [AsyncDisplayKit] Update project file to include new/deleted files * [ASControlNode+tvOS] Add missing Foundation import (whoops!) [ASImageNode+tvOS] Add missing Foundation import (whoops!) * Update podspec to only link AssetsLibrary framework on iOS * [ASTextKitCoreTextAdditions] If kCTParagraphStyleAttributeName key-value evaluates to an NSParagraphStyle, pass through to cleansed attributes. This fixes a bug that would occur if a CTParagraphStyle was passed as an attribute _alone_ (would not be caught by unsupported attributes check) * [ASMultiplexImageNode] Bump availability check to support < Xcode 9 * [ASTraitCollection] Fixes typo that was causing build to fail * Clean up formatting to adhere to character/line limit + braces

alexhillc added 5 commits December 25, 2017 19:38

[tvOS] Fixes errors when building against tvOS SDK

7e8c3e2

Update CHANGELOG.md

bd9fd57

Merge branch 'master' of github.com:TextureGroup/Texture into tvos

8663491

[tvOS] Fixes implicit conversion between UIViewAnimationCurve +

46f5412

UIViewAnimationOptions

Enable tvOS deployment target in Texture.podspec (for CI)

6b90382

[ASMultiplexImageNode] Fixes typo

b1b582a

alexhillc force-pushed the tvos branch from 5171320 to 15362d8 Compare December 28, 2017 20:12

alexhillc force-pushed the tvos branch from 15362d8 to 7759cf7 Compare December 28, 2017 20:55

[ASAvailability] Update AS_AVAILABLE_XXX fallbacks to function more like

8dbcc51

the built-in solution (more accurately target OS by checking target) Change AS_AVAILABLE_IOS -> AS_AVAILABLE_IOS_TVOS in places that shoud allow for both [ASAvailability] Simplify AS_AVAILABLE_IOS_TVOS

alexhillc force-pushed the tvos branch from a135ad6 to 8dbcc51 Compare December 28, 2017 22:00

[ASControlNode] Adds missing 'super' call in -[ASControlNode didLoad]

6607bda

when targeting tvOS

alexhillc added 4 commits December 29, 2017 12:44

Fix API_AVAILABLE iOS requirement

14a8e91

[NSParagraphStyle+ASText] Fixes typo related to testing

9673a83

[ASControlNode] Re-add helpful comment

bf9aa78

alexhillc commented Dec 30, 2017

View reviewed changes

alexhillc force-pushed the tvos branch from ed9eb08 to 8eb2dbe Compare January 6, 2018 02:11

alexhillc added 6 commits January 12, 2018 19:12

Merge remote-tracking branch 'upstream/master' into tvos

cb0e735

[AsyncDisplayKit] Update project file to include new/deleted files

3da014a

[ASControlNode+tvOS] Add missing Foundation import (whoops!)

fdf22d6

[ASImageNode+tvOS] Add missing Foundation import (whoops!)

Update podspec to only link AssetsLibrary framework on iOS

2d6d0a7

Merge remote-tracking branch 'upstream/master' into tvos

12a06b3

[ASMultiplexImageNode] Bump availability check to support < Xcode 9

91dcb20

alexhillc and others added 3 commits January 16, 2018 10:17

Merge branch 'master' into tvos

ef16069

[ASTraitCollection] Fixes typo that was causing build to fail

305b443

Merge remote-tracking branch 'upstream/master' into tvos

2ec4233

TextureGroup deleted a comment Jan 31, 2018

Adlai-Holler mentioned this pull request Jan 31, 2018

Enable tvOS support #653

Closed

appleguy approved these changes Feb 9, 2018

View reviewed changes

Adlai-Holler approved these changes Feb 9, 2018

View reviewed changes

Merge branch 'master' into tvos

fd246ad

appleguy reviewed Feb 9, 2018

View reviewed changes

Merge branch 'tvos' of github.com:alexhillc/Texture into tvos

1f76d48

Merge branch 'master' into tvos

b9cb2fa

maicki approved these changes Feb 20, 2018

View reviewed changes

alexhillc added 2 commits February 20, 2018 17:29

Merge branch 'tvos' of github.com:alexhillc/Texture into tvos

917d3f3

Clean up formatting to adhere to character/line limit + braces

ebbf512

alexhillc mentioned this pull request Feb 24, 2018

[tvOS] Focus Improvements #807

Closed

Merge branch 'master' into tvos

1a5b195

Merge branch 'master' into tvos

873933e

Adlai-Holler merged commit d9d9a29 into TextureGroup:master Mar 11, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[tvOS] Fixes errors when building against tvOS SDK #728

[tvOS] Fixes errors when building against tvOS SDK #728

alexhillc commented Dec 26, 2017

ghost commented Dec 28, 2017

ghost commented Dec 29, 2017

alexhillc Dec 30, 2017

alexhillc Dec 30, 2017

nguyenhuy commented Jan 16, 2018

Eke commented Jan 16, 2018 •

edited

Loading

alexhillc commented Jan 16, 2018

Eke commented Jan 21, 2018

appleguy commented Feb 9, 2018

appleguy Feb 9, 2018

appleguy Feb 9, 2018

appleguy Feb 9, 2018

appleguy Feb 9, 2018

appleguy Feb 9, 2018

appleguy Feb 9, 2018

appleguy Feb 9, 2018

Adlai-Holler left a comment

appleguy Feb 9, 2018

alexhillc Feb 9, 2018

alexhillc commented Feb 14, 2018

appleguy commented Feb 19, 2018

maicki left a comment

alexhillc commented Feb 24, 2018

ghost commented Feb 28, 2018

ghost commented Mar 4, 2018

		@@ -236,6 +237,7 @@ + (instancetype)paragraphStyleWithCTParagraphStyle:(CTParagraphStyleRef)coreText
		if (CTParagraphStyleGetValueForSpecifier(coreTextParagraphStyle, kCTParagraphStyleSpecifierLineSpacing, sizeof(lineSpacing), &lineSpacing))

[tvOS] Fixes errors when building against tvOS SDK #728

[tvOS] Fixes errors when building against tvOS SDK #728

Conversation

alexhillc commented Dec 26, 2017

ghost commented Dec 28, 2017

ghost commented Dec 29, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nguyenhuy commented Jan 16, 2018

Eke commented Jan 16, 2018 • edited Loading

alexhillc commented Jan 16, 2018

Eke commented Jan 21, 2018

appleguy commented Feb 9, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Adlai-Holler left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexhillc commented Feb 14, 2018

appleguy commented Feb 19, 2018

maicki left a comment

Choose a reason for hiding this comment

alexhillc commented Feb 24, 2018

ghost commented Feb 28, 2018

ghost commented Mar 4, 2018

Eke commented Jan 16, 2018 •

edited

Loading