[iOS] Fabric: Add missing source properties of Image #46460
Conversation
|
@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
packages/react-native/ReactCommon/react/renderer/components/image/conversions.h
Outdated
Show resolved
Hide resolved
| method = [[NSString alloc] initWithBytesNoCopy:(void *)imageSource.method.c_str() | ||
| length:imageSource.method.size() | ||
| encoding:NSUTF8StringEncoding | ||
| freeWhenDone:NO] | ||
| .uppercaseString |
There was a problem hiding this comment.
why can't we just do?
| method = [[NSString alloc] initWithBytesNoCopy:(void *)imageSource.method.c_str() | |
| length:imageSource.method.size() | |
| encoding:NSUTF8StringEncoding | |
| freeWhenDone:NO] | |
| .uppercaseString | |
| method = [[NSString alloc] initWithUTF8String:imageSource.method.c_str()].uppercaseString; |
also, no need for the ?: fallback.
There was a problem hiding this comment.
Emm, I used no copy method because I think it can reduce one copy, if we use initWithUTF8String, it would produces twice copy operations from c_str to nsadata.
...nderer/imagemanager/platform/ios/react/renderer/imagemanager/RCTImagePrimitivesConversions.h
Outdated
Show resolved
Hide resolved
| length:imageSource.body.size() | ||
| encoding:NSUTF8StringEncoding | ||
| freeWhenDone:NO]; | ||
| body = [RCTConvert NSData:bodyString]; |
There was a problem hiding this comment.
can't we just initialize it with some NSData initializer?
There was a problem hiding this comment.
Seems no suitable initializer...
There was a problem hiding this comment.
the initializer should be:
[NSData dataWithBytes:imageSource.body.c_str() length:imageSource.body.size()]We use this also in other locations, like here
There was a problem hiding this comment.
@cipolleschi I updated. But they are maybe different when bytes are not a valid UTF-8 bytes?
...nderer/imagemanager/platform/ios/react/renderer/imagemanager/RCTImagePrimitivesConversions.h
Outdated
Show resolved
Hide resolved
packages/react-native/ReactCommon/react/renderer/imagemanager/primitives.h
Outdated
Show resolved
Hide resolved
|
@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@cipolleschi merged this pull request in 052def0. |
|
This pull request was successfully merged by @zhongwuzw in 052def0 When will my fix make it into a release? | How to file a pick request? |
Summary:
Fabric: Add missing source properties of Image
Changelog:
[IOS] [FIXED] - Fabric: Add missing source properties of Image
Test Plan:
Missing props should worked.