stephan-tolksdorf / STULabel

A faster and more flexible label view for iOS
Other
121 stars 28 forks source link

macOS Catalyst Support #8

Closed b3ll closed 4 years ago

b3ll commented 4 years ago

So this diff is kinda massive, but it does work. I split up the type collision and the Catalyst gating separately.

This also just cuts out the STU_USE_SAFARI_SERVICES gating and what not since TARGET_OS_MACCATALYST is better

tbh I totally understand if you don't want to do the type collision changes since they add namespaces everywhere and it's kinda gigantic of a refactor

I tried running the tests but there were a bunch of failures (on iOS simulator) for like font references and stuff but didn't see anything really these changes would've caused

stephan-tolksdorf commented 4 years ago

Thanks for the pull request! I'll try to take a look this weekend.

I've never received any reply to the Radar I filed, by the way -- as expected.

stephan-tolksdorf commented 4 years ago

I'll work around the UInt type ambiguity using the preprocessor instead of explicitly qualifying the type everywhere. Since this won't affect the public headers, I suppose that's ok with you.

I have a question regarding the first commit: Why did you disable the main thread asserts in display and drawInContext: in STULabelLayer?

b3ll commented 4 years ago

That's totally fine with me! Wasn't sure how you'd go about nicely doing it with preprocessor stuff tho

As for the main thread asserts… that's actually an interesting Catalyst side effect. macOS will render layers on different threads than main, which is why those asserts were being tripped

stephan-tolksdorf commented 4 years ago

Thanks for the explanation! Is this documented somewhere? Can there be concurrent calls to display or drawInContext:? Is the main thread blocked during calls to these methods from other threads?

b3ll commented 4 years ago

Insofar as documented I have no idea, I just noticed during the tile rendering it was being called from a bunch of different threads (other than main) which confused me… I think it might be doing offscreen CGContext drawing and then blipping them back to main when ready

stephan-tolksdorf commented 4 years ago

The STULabelLayer display method is not thread-safe and will make delegate calls, which normally must only happen on the main thread. Are you using the layer in some kind of non-standard container layer or view? Is there a way to force drawing on the main thread?

stephan-tolksdorf commented 4 years ago

The Demo app seems to work fine on the Mac with enabled main thread asserts. (See the updated master branch version.)

b3ll commented 4 years ago

huh, I guess it does work. Perhaps it was an AsyncDisplayKit issue that only repros on Catalyst… I can update it to drop that, although it looks like you already merged in Catalyst-ready changes into master?

b3ll commented 4 years ago

Actually, it seems to only assert when it's highlighting text under Catalyst… the same assert doesn't happen on iOS. To be honest the text doesn't even render as highlighted anyways under Catalyst

Thread 38 is where it's asserting, still trying to figure out what's causing it to draw though

(lldb) bt all
  thread #1, queue = 'com.apple.main-thread'
    frame #0: 0x00007fff63a1425a libsystem_kernel.dylib`mach_msg_trap + 10
    frame #1: 0x00007fff63a145d0 libsystem_kernel.dylib`mach_msg + 60
    frame #2: 0x00007fff2c22fd0b CoreFoundation`__CFRunLoopServiceMachPort + 322
    frame #3: 0x00007fff2c22e8e7 CoreFoundation`__CFRunLoopRun + 1695
    frame #4: 0x00007fff2c22dbd3 CoreFoundation`CFRunLoopRunSpecific + 499
    frame #5: 0x00007fff2ad8365d HIToolbox`RunCurrentEventLoopInMode + 292
    frame #6: 0x00007fff2ad8339d HIToolbox`ReceiveNextEventCommon + 600
    frame #7: 0x00007fff2ad83127 HIToolbox`_BlockUntilNextEventMatchingListInModeWithFilter + 64
    frame #8: 0x00007fff293f3ba4 AppKit`_DPSNextEvent + 990
    frame #9: 0x00007fff293f2380 AppKit`-[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 1352
    frame #10: 0x00007fff293e409e AppKit`-[NSApplication run] + 658
    frame #11: 0x00007fff293b6465 AppKit`NSApplicationMain + 777
    frame #12: 0x00007fff296d969c AppKit`_NSApplicationMainWithInfoDictionary + 16
    frame #13: 0x00007fff5d2048f1 UIKitMacHelper`UINSApplicationMain + 322
    frame #14: 0x00007fff6a0ac273 UIKitCore`UIApplicationMain + 2105
    frame #15: 0x000000010003014b Artemis`main at AppDelegate.swift:13:7
    frame #16: 0x00007fff638d37fd libdyld.dylib`start + 1
    frame #17: 0x00007fff638d37fd libdyld.dylib`start + 1
  thread #8, name = 'com.apple.uikit.eventfetch-thread'
    frame #0: 0x00007fff63a1425a libsystem_kernel.dylib`mach_msg_trap + 10
    frame #1: 0x00007fff63a145d0 libsystem_kernel.dylib`mach_msg + 60
    frame #2: 0x00007fff2c22fd0b CoreFoundation`__CFRunLoopServiceMachPort + 322
    frame #3: 0x00007fff2c22e8e7 CoreFoundation`__CFRunLoopRun + 1695
    frame #4: 0x00007fff2c22dbd3 CoreFoundation`CFRunLoopRunSpecific + 499
    frame #5: 0x00007fff2e8d11a8 Foundation`-[NSRunLoop(NSRunLoop) runMode:beforeDate:] + 212
    frame #6: 0x00007fff2e97fdf7 Foundation`-[NSRunLoop(NSRunLoop) runUntilDate:] + 103
    frame #7: 0x00007fff6a0ad7e6 UIKitCore`-[UIEventFetcher threadMain] + 138
    frame #8: 0x00007fff2e8c97a8 Foundation`__NSThread__start__ + 1064
    frame #9: 0x00000001034dc9bd libsystem_pthread.dylib`_pthread_start + 148
    frame #10: 0x00000001034d816b libsystem_pthread.dylib`thread_start + 15
  thread #9, name = 'com.apple.NSEventThread'
    frame #0: 0x00007fff63a1425a libsystem_kernel.dylib`mach_msg_trap + 10
    frame #1: 0x00007fff63a145d0 libsystem_kernel.dylib`mach_msg + 60
    frame #2: 0x00007fff2c22fd0b CoreFoundation`__CFRunLoopServiceMachPort + 322
    frame #3: 0x00007fff2c22e8e7 CoreFoundation`__CFRunLoopRun + 1695
    frame #4: 0x00007fff2c22dbd3 CoreFoundation`CFRunLoopRunSpecific + 499
    frame #5: 0x00007fff29596792 AppKit`_NSEventThread + 132
    frame #6: 0x00000001034dc9bd libsystem_pthread.dylib`_pthread_start + 148
    frame #7: 0x00000001034d816b libsystem_pthread.dylib`thread_start + 15
  thread #10, name = 'Realm notification listener'
    frame #0: 0x00007fff63a18bce libsystem_kernel.dylib`kevent + 10
    frame #1: 0x000000010004ff33 Artemis`realm::_impl::ExternalCommitHelper::listen(this=0x0000600000cda040) at external_commit_helper.cpp:216:15
    frame #2: 0x0000000100054458 Artemis`realm::_impl::ExternalCommitHelper::ExternalCommitHelper(this=0x0000600000008148)::$_0::operator()() const at external_commit_helper.cpp:173:13
    frame #3: 0x00000001000543fd Artemis`decltype(__f=0x0000600000008148)::$_0>(fp)()) std::__1::__invoke<realm::_impl::ExternalCommitHelper::ExternalCommitHelper(realm::_impl::RealmCoordinator&)::$_0>(realm::_impl::ExternalCommitHelper::ExternalCommitHelper(realm::_impl::RealmCoordinator&)::$_0&&) at type_traits:4361:1
    frame #4: 0x0000000100054365 Artemis`void std::__1::__thread_execute<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, realm::_impl::ExternalCommitHelper::ExternalCommitHelper(realm::_impl::RealmCoordinator&)::$_0>(__t=size=2, (null)=__tuple_indices<> @ 0x0000700006cf6f58)::$_0>&, std::__1::__tuple_indices<>) at thread:342:5
    frame #5: 0x0000000100053c06 Artemis`void* std::__1::__thread_proxy<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, realm::_impl::ExternalCommitHelper::ExternalCommitHelper(realm::_impl::RealmCoordinator&)::$_0> >(__vp=0x0000600000008140) at thread:352:5
    frame #6: 0x00000001034dc9bd libsystem_pthread.dylib`_pthread_start + 148
    frame #7: 0x00000001034d816b libsystem_pthread.dylib`thread_start + 15
* thread #38, stop reason = breakpoint 1.1
    frame #0: 0x00007fff625657d5 libobjc.A.dylib`objc_exception_throw
    frame #1: 0x00007fff2c2d4d10 CoreFoundation`+[NSException raise:format:arguments:] + 88
    frame #2: 0x00007fff2ea0baf7 Foundation`-[NSAssertionHandler handleFailureInFunction:file:lineNumber:description:] + 166
    frame #3: 0x0000000102b06588 STULabel`stu_assertion_failed(fileName="/Users/adam/Dropbox/Dev/iOS/Projects/Artemis/Pods/STULabel/STULabel/STULabelLayer.mm", line=1987, functionName="-[STULabelLayer display]", condition="is_main_thread()") at Assert.m:36:3
  * frame #4: 0x0000000102bb8ba2 STULabel`::-[STULabelLayer display](self=0x000000010747e6a0, _cmd="display") at STULabelLayer.mm:1987:3
    frame #5: 0x00007fff37a582c3 QuartzCore`CA::Layer::display_if_needed(CA::Transaction*) + 739
    frame #6: 0x00007fff37a36ba6 QuartzCore`CA::Context::commit_transaction(CA::Transaction*, double) + 334
    frame #7: 0x00007fff37a357ce QuartzCore`CA::Transaction::commit() + 638
    frame #8: 0x00007fff37a55c12 QuartzCore`CA::Transaction::release_thread(void*) + 206
    frame #9: 0x00000001034da889 libsystem_pthread.dylib`_pthread_tsd_cleanup + 551
    frame #10: 0x00000001034dcdca libsystem_pthread.dylib`_pthread_exit + 70
    frame #11: 0x00000001034da610 libsystem_pthread.dylib`_pthread_wqthread_exit + 74
    frame #12: 0x00000001034d90ec libsystem_pthread.dylib`_pthread_wqthread + 482
    frame #13: 0x00000001034d8157 libsystem_pthread.dylib`start_wqthread + 15
  thread #40
    frame #0: 0x00000001034d8148 libsystem_pthread.dylib`start_wqthread
  thread #41
    frame #0: 0x00007fff63a1592e libsystem_kernel.dylib`__workq_kernreturn + 10
    frame #1: 0x00000001034d9090 libsystem_pthread.dylib`_pthread_wqthread + 390
    frame #2: 0x00000001034d8157 libsystem_pthread.dylib`start_wqthread + 15
  thread #42
    frame #0: 0x00007fff63a1592e libsystem_kernel.dylib`__workq_kernreturn + 10
    frame #1: 0x00000001034d9090 libsystem_pthread.dylib`_pthread_wqthread + 390
    frame #2: 0x00000001034d8157 libsystem_pthread.dylib`start_wqthread + 15
stephan-tolksdorf commented 4 years ago

I assume the threading issue you're seeing is caused by AsyncDisplayKit. STULabelLayer is only safe to use on the main thread, and that is unlikely to change.

Except removing the main thread asserts, I've adopted all your changes in commit f1408e1 and commit a9baed0a. Thanks again for your pull request!

b3ll commented 4 years ago

Huh weird, I'll check it out again when I get back to it! Thanks for keeping this project alive! :D

stephan-tolksdorf commented 4 years ago

Please let me know if you can reproduce the issue in a way that excludes AsyncDisplayKit as a cause.