Fabric: Using optional<int> instead of CGColorRef on iOS

Summary:
Finally, this diff changes the internal implementation of SharedColor to be `optional<int>`.

Initially, when we started working on the new renderer, it seemed like a good idea to allocated CGColor objects ahead of time and store them with Props. Now, this idea does not look so good, especially because:
* Having `SharedColor` as a `shared_ptr` is a quite big perf overhead for copying this thing. And the size of the object is not small.
* Having `SharedColor` as a `shared_ptr` creates huge interconnectedness among pieces of the core and rendering. E.g. improper releasing a pointer in some component view can cause a crash somewhere in the core (because both places might use the same shared `blackColor`.

On Android, we already use simple `int` as a representation of a color, and this works great. And this diff implements something very similar to Android, but a bit differently: here we use `optional<int>` instead of custom class with a single `int` field and some magic value that represents "empty value".

This approach should fix T75836417 because now we don't have allocation and deallocation when we simply assign color values.

If this solution works fine on iOS, I will do unify all implementations among all platforms.

Changelog: [Internal] Fabric-specific internal change.

Reviewed By: JoshuaGross

Differential Revision: D23753507

fbshipit-source-id: 42fd6cee6bf7b39c92c88536da06ba9e8cf4d4db
This commit is contained in:
Valentin Shergin
2020-09-17 11:12:28 -07:00
committed by Facebook GitHub Bot
parent 241795cbc4
commit e4877ed985
3 changed files with 19 additions and 24 deletions
+2
View File
@@ -7,6 +7,7 @@ load(
"fb_xplat_cxx_test",
"get_apple_compiler_flags",
"get_apple_inspector_flags",
"react_native_xplat_target",
"rn_xplat_cxx_library",
"subdir_glob",
)
@@ -95,6 +96,7 @@ rn_xplat_cxx_library(
tests = [":tests"],
visibility = ["PUBLIC"],
deps = [
react_native_xplat_target("better:better"),
"//third-party/glog:glog",
"//xplat/fbsystrace:fbsystrace",
"//xplat/folly:headers_only",
@@ -12,28 +12,21 @@ namespace facebook {
namespace react {
SharedColor colorFromComponents(ColorComponents components) {
const CGFloat componentsArray[] = {
components.red, components.green, components.blue, components.alpha};
auto color = CGColorCreate(CGColorSpaceCreateDeviceRGB(), componentsArray);
return SharedColor(color, CGColorRelease);
float ratio = 255.9999;
return SharedColor(
((int)(components.alpha * ratio) & 0xff) << 24 |
((int)(components.red * ratio) & 0xff) << 16 |
((int)(components.green * ratio) & 0xff) << 8 |
((int)(components.blue * ratio) & 0xff));
}
ColorComponents colorComponentsFromColor(SharedColor color) {
if (!color) {
// Empty color object can be considered as `clear` (black, fully
// transparent) color.
return ColorComponents{0, 0, 0, 0};
}
auto numberOfComponents __unused = CGColorGetNumberOfComponents(color.get());
assert(numberOfComponents == 4);
const CGFloat *components = CGColorGetComponents(color.get());
return ColorComponents{(float)components[0],
(float)components[1],
(float)components[2],
(float)components[3]};
ColorComponents colorComponentsFromColor(SharedColor sharedColor) {
float ratio = 256;
Color color = *sharedColor;
return ColorComponents{(float)((color >> 16) & 0xff) / ratio,
(float)((color >> 8) & 0xff) / ratio,
(float)((color >> 0) & 0xff) / ratio,
(float)((color >> 24) & 0xff) / ratio};
}
SharedColor clearColor() {
@@ -7,17 +7,17 @@
#pragma once
#include <memory>
#include <better/optional.h>
#include <CoreGraphics/CoreGraphics.h>
#include <react/renderer/graphics/ColorComponents.h>
#include <react/renderer/graphics/Float.h>
namespace facebook {
namespace react {
using Color = CGColor;
using SharedColor = std::shared_ptr<Color>;
using Color = int32_t;
using SharedColor = better::optional<Color>;
SharedColor colorFromComponents(ColorComponents components);
ColorComponents colorComponentsFromColor(SharedColor color);