[WIP] Move ImageEditor native module from react-native. #1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Summary: public Added JS wrappers for ImageStore(Manager) and ImageEditor(Manager) so they can be required in the normal way instead of accessed directly via NativeModules. Reviewed By: dmmiller Differential Revision: D2773822 fb-gh-sync-id: 6eeafd3f80a87b1b91a04a2aebad6e2fd31b0e98
Summary: public Standardises the image decoding logic for all image sources, meaning we get the benefits of efficient downscaling of images from all sources, not just ALAssets. Reviewed By: javache Differential Revision: D2647083 fb-gh-sync-id: e41456f838e4c6ab709b1c1523f651a86ff6e623
Reviewed By: mkonicek Differential Revision: D2869751 fb-gh-sync-id: 862c266601dd83ca3bf9c9bcbf107f7b17b8bdfd
Summary:Diff D2647083 cleaned up image editing related logics and introduced an image cropping bug. The bug is that the result of the image cropping will be wrong if displaySize is specified. In particular, in Ads Manager App, we generate thumbnail by calling the image cropping function with displaySize set. With this bug, the thumbnail we get is not correct. This diff fixed the bug by replacing `image` with `croppedImage`. It should be a typo from D2647083 Reviewed By: zjj010104 Differential Revision: D2947730 fb-gh-sync-id: df7c7f3ddac5b053425db884f808e27b8418116e shipit-source-id: df7c7f3ddac5b053425db884f808e27b8418116e
Summary:Previously images that have exif orientation data would be cropped and the exif data would be lost leading to us displaying pictures rotated in error. Reviewed By: foghina Differential Revision: D2971596 fb-gh-sync-id: 40f92e27089455259a7d8b83c92d0cf36367e5df shipit-source-id: 40f92e27089455259a7d8b83c92d0cf36367e5df
Reviewed By: zjj010104 Differential Revision: D3012969 fb-gh-sync-id: 8997a6167398715f8f78724a43a449da4079cc7e shipit-source-id: 8997a6167398715f8f78724a43a449da4079cc7e
Differential Revision: D3019087 fb-gh-sync-id: 8f137351d1fd89b71260b73b2f8daefb6f00127a shipit-source-id: 8f137351d1fd89b71260b73b2f8daefb6f00127a
Reviewed By: zjj010104 Differential Revision: D3073724 fb-gh-sync-id: cd97477c9405f1ec048ba3dcdfd4280f77b631d2 shipit-source-id: cd97477c9405f1ec048ba3dcdfd4280f77b631d2
Reviewed By: zjj010104 Differential Revision: D3076377 fb-gh-sync-id: da9ca730904c2d520c5c0619b68f3ebee2c74f02 shipit-source-id: da9ca730904c2d520c5c0619b68f3ebee2c74f02
Summary:Remove Trailing Spaces. Why: Sometimes there are conflicts with trailing spaces Saves space Those whose tools automatically delete them will have their pr watered down with trailing space removal Closes facebook/react-native#6787 Differential Revision: D3144704 fb-gh-sync-id: d8a62f115a3f8a8a49d5b07f56c540a02af38cf8 fbshipit-source-id: d8a62f115a3f8a8a49d5b07f56c540a02af38cf8
Summary:`externalCacheDir == null && externalCacheDir == null` is obviously a typo in existing code. Closes facebook/react-native#6429 Differential Revision: D3184362 fb-gh-sync-id: 1cd966ed96414348c4319d44679d2c912df6cc93 fbshipit-source-id: 1cd966ed96414348c4319d44679d2c912df6cc93
Summary:
Allows developers to specify headers to include in the HTTP request
when fetching a remote image. For example, one might leverage this
when fetching an image from an endpoint that requires authentication:
```
<Image
style={styles.logo}
source={{
uri: 'http://facebook.github.io/react/img/logo_og.png',
headers: {
Authorization: 'someAuthToken'
}
}}
/>
```
Note that the header values must be strings.
Works on iOS and Android.
**Test plan (required)**
- Ran a small example like the one above on iOS and Android and ensured the headers were sent to the server.
- Ran a small example to ensure that \<Image\> components without headers still work.
- Currently using this code in our app.
Adam Comella
Microsoft Corp.
Closes facebook/react-native#7338
Reviewed By: javache
Differential Revision: D3371458
Pulled By: nicklockwood
fbshipit-source-id: cdb24fe2572c3ae3ba82c86ad383af6d85157e20
…ries/FBReactKit/BUCK Reviewed By: javache Differential Revision: D3442470 fbshipit-source-id: 584a2bb3df5f7122166778b8fd44fae45560491e
Reviewed By: bestander Differential Revision: D3683952 fbshipit-source-id: 9484d0b0e86859e8edaca0da1aa13a667f200905
Reviewed By: astreet Differential Revision: D3334273 fbshipit-source-id: a33bf72c5c184844885ef3ef610a05d9c102c8ea
Reviewed By: astreet Differential Revision: D3334273 fbshipit-source-id: a3849604ea89db74900850c294685e7da9aeeacc
…oduleList Reviewed By: lexs Differential Revision: D3796860 fbshipit-source-id: d4b5f3635754ef28277b79cb1ea9bab07ba3ea6e
Summary: To make React Native play nicely with our internal build infrastructure we need to properly namespace all of our header includes. Where previously you could do `#import "RCTBridge.h"`, you must now write this as `#import <React/RCTBridge.h>`. If your xcode project still has a custom header include path, both variants will likely continue to work, but for new projects, we're defaulting the header include path to `$(BUILT_PRODUCTS_DIR)/usr/local/include`, where the React and CSSLayout targets will copy a subset of headers too. To make Xcode copy headers phase work properly, you may need to add React as an explicit dependency to your app's scheme and disable "parallelize build". Reviewed By: mmmulani Differential Revision: D4213120 fbshipit-source-id: 84a32a4b250c27699e6795f43584f13d594a9a82
Reviewed By: javache Differential Revision: D4487530 fbshipit-source-id: ea16720dc15e490267ad244c43ea9d237f81e353
Summary:
On Android, using `ImageEditor.cropImage` with `displaySize` option may causes crash with exception below:
```
FATAL EXCEPTION: mqt_native_modules
Process: me.sohobloo.test, PID: 11308
com.facebook.react.bridge.UnexpectedNativeTypeException: TypeError: expected dynamic type `int64', but had type `double'
at com.facebook.react.bridge.ReadableNativeMap.getInt(Native Method)
at com.facebook.react.modules.camera.ImageEditingManager.cropImage(ImageEditingManager.java:196)
at java.lang.reflect.Method.invoke(Native Method)
at java.lang.reflect.Method.invoke(Method.java:372)
at com.facebook.react.bridge.BaseJavaModule$JavaMethod.invoke(BaseJavaModule.java:345)
at com.facebook.react.cxxbridge.JavaModuleWrapper.invoke(JavaModuleWrapper.java:141)
at com.facebook.react.bridge.queue.NativeRunnable.run(Native Method)
at android.os.Handler.handleCallback(Handler.java:815)
at android.os.Handler.dispatchMessage(Handler.java:104)
at com.facebook.react.bridge.queue.MessageQueueThreadHandler.dispatchMessage(MessageQueueThreadHandler.java:31)
at android.os.Looper.loop(Looper.java:194)
at com.facebook.react.bridge.queue.MessageQueueThreadImpl$3.run(MessageQueueThreadImpl.java:196)
at java.lang.Thread.run(Thread.java:818)
```
This is caused by getInt from `number` type of JS.
```javascript
ImageEditor.cropImage(
uri,
{
offset: {x: 0, y: 0},
size: {width: 320, height: 240},
displaySize: {width: 320.5, height: 240.5}
}
);
```
Closes facebook/react-native#13312
Differential Revision: D5462709
Pulled By: shergin
fbshipit-source-id: 42cb853b533769b6969b8ac9ad50f3dd3c764055
Summary: The Android ImageEditingManager is inefficient and slow when cropping images. It loads the full resolution image into memory and then crops it. This leads to slow performance and occasional OutOfMemory Exceptions. [BitmapRegionDecoder](https://developer.android.com/reference/android/graphics/BitmapRegionDecoder.html) can be used to crop without needing to load the full resolution image into memory. Using it is much more efficient and much faster. Relevant issue: facebook/react-native#10470 Attempt to crop a very large image (2000x2000) on Android. With this change, the crop should happen almost instantly. On the master branch, it should take 2-3 seconds (and might run out of memory). Please let me know if there's anything else I can provide. Closes facebook/react-native#15439 Differential Revision: D5628223 Pulled By: shergin fbshipit-source-id: bf314e76134cd015380968ec4533225e724c4b26
Summary: Trivial beauty. Reviewed By: sahrens Differential Revision: D6715955 fbshipit-source-id: 3632750591f53d4673a2ce76309a0cc62946524d
Summary:
Includes React Native and its dependencies Fresco, Metro, and Yoga. Excludes samples/examples/docs.
find: ^(?:( *)|( *(?:[\*~#]|::))( )? *)?Copyright (?:\(c\) )?(\d{4})\b.+Facebook[\s\S]+?BSD[\s\S]+?(?:this source tree|the same directory)\.$
replace: $1$2$3Copyright (c) $4-present, Facebook, Inc.\n$2\n$1$2$3This source code is licensed under the MIT license found in the\n$1$2$3LICENSE file in the root directory of this source tree.
Reviewed By: TheSavior, yungsters
Differential Revision: D7007050
fbshipit-source-id: 37dd6bf0ffec0923bfc99c260bb330683f35553e
Summary: This PR removes the need for having the `providesModule` tags in all the modules in the repository. It configures Flow, Jest and Metro to get the module names from the filenames (`Libraries/Animated/src/nodes/AnimatedInterpolation.js` => `AnimatedInterpolation`) * Checked the Flow configuration by running flow on the project root (no errors): ``` yarn flow ``` * Checked the Jest configuration by running the tests with a clean cache: ``` yarn jest --clearCache && yarn test ``` * Checked the Metro configuration by starting the server with a clean cache and requesting some bundles: ``` yarn run start --reset-cache curl 'localhost:8081/IntegrationTests/AccessibilityManagerTest.bundle?platform=android' curl 'localhost:8081/Libraries/Alert/Alert.bundle?platform=ios' ``` [INTERNAL] [FEATURE] [All] - Removed providesModule from all modules and configured tools. Closes facebook/react-native#18995 Reviewed By: mjesun Differential Revision: D7729509 Pulled By: rubennorte fbshipit-source-id: 892f760a05ce1fddb088ff0cd2e97e521fb8e825
Summary: ImageEditor.cropImage creates a temporary file when downloading images https://fburl.com/07r68w9s This temporary file can be stored on external storage on android. External storage is accessible to any other application on the device, which could possibly leak images. Using external storage may be unavoidable. I've voiced my opinion and solicited others on T31548988. Once a good policy is agreed upon, we can implement it. For now, I'm adding this comment to make it explicit how images are cached. Reviewed By: achen1 Differential Revision: D8837808 fbshipit-source-id: 02341bc94a1c95340390a713b76fe85603fd8f1b
Summary: Locking down view style so that invalid styles can't be passed into View. Reviewed By: yungsters Differential Revision: D9309097 fbshipit-source-id: 69e7e3c5626609cfd47c167027a55470c42228c8
Summary: This change drops the year from the copyright headers and the LICENSE file. Reviewed By: yungsters Differential Revision: D9727774 fbshipit-source-id: df4fc1e4390733fe774b1a160dd41b4a3d83302a
|
This is awesome! I'm gonna give you admin access to this repo and would like to give you push access for npm – what's your npm username? |
|
Awesome! I added you to npm. As next steps, could you publish a 1.0.0 release, send a PR with a deprecation message to RN, and set up CI for this repo? |
|
Actually, first, let me know once this PR specifically is ready to be merge and reviewing :D Sorry I am thinking too far ahead already. |
|
Sure, I'll let you know, it should be ready tomorrow. |
|
@cpojer I accidentialy pushed changes directly to master branch of this repo instead of my fork (it worked cause you gave me permissions :D) and it automatically merged this PR. I hope it's not a big problem? Let me know if I have to change anything. |
cpojer
reviewed
Feb 13, 2019
| const RNCImageEditor = require('NativeModules').RNCImageEditor; | ||
| import { NativeModules } from 'react-native'; | ||
|
|
||
| const { RNCImageEditor } = NativeModules; |
There was a problem hiding this comment.
can you remove the spaces around the curly braces? :D
|
@Trancever I'm sure we'll survive :) Is it ready to be published? If yes, go for it, if no, please send a PR with fixes necessary to make that happen. Please do publish the identical version to RN as 1.0.0. |
|
@cpojer It's live https://www.npmjs.com/package/@react-native-community/image-editor 🎉 I am not sure if I'll have enough time to submit PR to react-native this week cause I have short holidays, but will try to do that asap. |
|
Awesome work! Yeah take your time, looking forward to seeing your future PRs :) |
64 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is still work in progress, description will be updated once it's ready for review.