Fix Xcode 15.3 concurrency warnings when using Screen.scale

[jszumski]

Fixes #765. The viral nature of concurrency unfortunately means that the blast radius is pretty high for just 2 warning fixes:

• Adds @MainActor to direct and downstream callers of Screen.scale
• Adds @MainActor to downstream callers in tests
• Moves @MainActor from XCTestCases to individual test functions instead
• Switches the image pipeline thread safety test from resizing to blurring. This avoids marking the test as @MainActor which would destroy its ability to test threading.

[kean]

Hey, @jszumski. Thanks for raising the issue and taking a stab at it.

means that the blast radius is pretty high for just 2 warning

Right, I also would prefer to avoid limiting most of these APIs to the main-thread. What I would suggest trying instead is updating ImageTargetSize so that it no longer pre-computes the size in pixels.

[jszumski]

What I would suggest trying instead is updating ImageTargetSize so that it no longer pre-computes the size in pixels.

@kean Any pointers to where an async call to the MainActor might fit best? I don't know much about the internals of Nuke, but I assume the entire pipeline should be able to run in a background thread, so it would have to be done in an initial step somewhere and fed down into these other call sites?

[kean]

Any pointers to where an async call to the MainActor might fit best?

I briefly looked into it, and I'm really not sure. None of these potential solutions seem ideal. You need the pixel size during resizing and resizing happens in the background. I'm also not sure how pre-computing it would look like. After considering these options, restricting certain, but not all, initializers to the main thread doesn't seem like the worst option.

[kean]

If you remove the @MainActor attribute from Nuke.Screen, the warning goes away. It was added in 2022 to fix a warning in one of the previous versions of Xcode.

UIScreen.main is also deprecated, so it's probably worth just removing ImageProcessingOptions.Unit and only allowing to use the size in pixels.

[jszumski]

Updated with that fix. I'm honestly surprised it works.

[kean]

Yeah, it seems like a compiler bug. I gave up trying to find any official documentation on how it's supposed to work. The rules for Sendable will now probably require a separate book.

Maybe it works because it's static and the compiler knows to init it on the main actor.

[kean]

Updated with that fix. I'm honestly surprised it works.

I think it was just a bug with "minimum" concurrency checking, which I use, and now it's been fixed. I wouldn't expect a warning here with "minimum" checking.

Btw, if you enable "complete" checking, it becomes an error:

[jszumski]

I wonder if UITraitCollection.current.displayScale is a better replacement that could side step the main actor requirement?

[kean]

I wonder if UITraitCollection.current.displayScale is a better replacement that could side step the main actor requirement?

Great suggestion! It looks like it is and it is not confined to the main thread:

UIKit stores the value of the currentTraitCollection property as a thread-local variable, so access is lightweight and free of side effects. 

I'm going to go with UITraitCollection.current.displayScale in the upcoming version.