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

[Upstream] Pull out CGContext early in UIImage+Diff #35940

Closed
wants to merge 2 commits into from

Conversation

Saadnajmi
Copy link
Contributor

@Saadnajmi Saadnajmi commented Jan 24, 2023

Summary

This is a small refactor designed to make future merges for React Native macOS easier. It was found while merging React Native 0.71 into React Native macOS.

In React Native macOS, we ifdef iOS out API's and replace them with macOS APIs, reusing as much code as possible. CGContext is a type that is available on both iOS and macOS, but UIGraphicsImageRendererContext is not. A simple refactor makes it easier to reuse this code in React Native macOS without affecting iOS :)

Resolves microsoft#1676

Changelog

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Microsoft Partner: Microsoft Partner labels Jan 24, 2023
@Saadnajmi Saadnajmi changed the title [React Native macOS upstream] Pull out CGContext early in UIImage+Diff [Upstream] Pull out CGContext early in UIImage+Diff Jan 24, 2023
@analysis-bot
Copy link

analysis-bot commented Jan 24, 2023

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,462,911 -19
android hermes armeabi-v7a 7,783,489 +208
android hermes x86 8,936,172 +272
android hermes x86_64 8,794,233 +425
android jsc arm64-v8a 9,648,873 -14
android jsc armeabi-v7a 8,383,370 +207
android jsc x86 9,711,124 +262
android jsc x86_64 10,188,137 +422

Base commit: 6f7428e
Branch: main

Copy link
Contributor

@matrush matrush left a comment

Choose a reason for hiding this comment

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

This is not compiling, please fix the issue

@facebook-github-bot
Copy link
Contributor

@philIip has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@Saadnajmi
Copy link
Contributor Author

Any update on this?

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Feb 5, 2023
@facebook-github-bot
Copy link
Contributor

@javache merged this pull request in 7f2dd1d.

@Saadnajmi Saadnajmi deleted the renderercontext branch April 6, 2023 23:28
OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this pull request May 22, 2023
Summary:
This is a small refactor designed to make future merges for React Native macOS easier. It was found while merging React Native 0.71 into React Native macOS.

In React Native macOS, we ifdef iOS  out API's and replace them with macOS APIs, reusing as much code as possible. `CGContext` is a type that is available on both iOS and macOS, but `UIGraphicsImageRendererContext` is not. A simple refactor makes it easier to reuse this code in React Native macOS without affecting iOS :)

Resolves microsoft#1676

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry.

Pick one each for the category and type tags:

[IOS] [CHANGED] - Pull out CGContext early in UIImage+Diff

Pull Request resolved: facebook#35940

Test Plan: Build should pass.

Reviewed By: matrush

Differential Revision: D42761424

Pulled By: javache

fbshipit-source-id: 5ce79a5e7c60484dd37d0b2c3f58ff0897d662df
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. p: Microsoft Partner: Microsoft Partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pull out context.CGContext early in UIImage+diff.m
4 participants