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

Improve no-copy rendering experiment, remove +load method #771

Merged
merged 5 commits into from
Jan 30, 2018

Conversation

Adlai-Holler
Copy link
Member

This PR achieves the following:

  • Critical Fixes an issue with no-copy rendering where images with bytes-per-row not aligned to 32 have to be copied by CoreAnimation.
  • Removes our +load method by changing ASScreenScale to use UIGraphics which is thread-safe.
  • Prevents leaks if users fail to close their ASGraphics contexts.
  • ASGraphics creates images in the device-specific (wide color) space, same as plain UIGraphics. This doesn't seem to matter as far as I can tell – CA will not copy the image in either case, but still best to copy UIGraphics.
  • Always read fields from "reference" UIGraphics contexts so that if they change we are covered.

@@ -143,8 +143,9 @@ CGFloat ASScreenScale()
static CGFloat __scale = 0.0;
static dispatch_once_t onceToken;
dispatch_once(&onceToken, ^{
ASDisplayNodeCAssertMainThread();
__scale = [[UIScreen mainScreen] scale];
UIGraphicsBeginImageContextWithOptions(CGSizeMake(1, 1), YES, 0);
Copy link
Member

Choose a reason for hiding this comment

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

@Adlai-Holler as an extra measure of safety, could you add a unit test that verifies the equality of [[UIScreen mainScreen] scale] with ASScreenScale()?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good call, done.

Copy link
Contributor

@maicki maicki left a comment

Choose a reason for hiding this comment

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

LGTM

@Adlai-Holler Adlai-Holler merged commit 2e94bb8 into master Jan 30, 2018
@Adlai-Holler Adlai-Holler deleted the AHImproveGraphicsCtx branch January 30, 2018 19:18
Adlai-Holler added a commit that referenced this pull request Jan 30, 2018
* Improve graphics contexts experiment

* Update changelog

* Remove extra space

* Add a unit test for screen scale

* Fix typo and use unique value
bernieperez pushed a commit to AtomTickets/Texture that referenced this pull request Apr 25, 2018
…up#771)

* Improve graphics contexts experiment

* Update changelog

* Remove extra space

* Add a unit test for screen scale

* Fix typo and use unique value
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants