store borderColor in a non lossy way
Summary: Fix for issue #3652 - Converting from `int` to `float` is lossy for very large numbers, so storing `borderColor` as a single `Spacing` object (which uses `float`) was not working for certain colors. So, this pull request splits `borderColor` into alpha and RGB components, and stores each of these as their own respective `Spacing` objects. *Test Plan* Check out cosmith sample code here that triggers the bug: https://rnplay.org/apps/l1bw2A What currently looks like this: <img width="548" alt="screen shot 2016-08-13 at 6 22 28 pm" src="https://cloud.githubusercontent.com/assets/1630466/17645965/9346f05e-6183-11e6-8d40-3e458b08fd9a.png"> Should look like this (with my fix applied): <img width="543" alt="screen shot 2016-08-13 at 6 20 08 pm" src="https://cloud.githubusercontent.com/assets/1630466/17645968/9c26d1d0-6183-11e6-8759-75a5e99f498a.png"> Closes https://github.com/facebook/react-native/pull/9380 Differential Revision: D3716707 Pulled By: foghina fbshipit-source-id: 1164378112e2a58d43c8f5fc671c2efdb64b412b
This commit is contained in:
parent
8240339dca
commit
8095707938
|
@ -46,6 +46,8 @@ import com.facebook.csslayout.Spacing;
|
|||
/* package */ class ReactViewBackgroundDrawable extends Drawable {
|
||||
|
||||
private static final int DEFAULT_BORDER_COLOR = Color.BLACK;
|
||||
private static final int DEFAULT_BORDER_RGB = 0x00FFFFFF & DEFAULT_BORDER_COLOR;
|
||||
private static final int DEFAULT_BORDER_ALPHA = (0xFF000000 & DEFAULT_BORDER_COLOR) >>> 24;
|
||||
|
||||
private static enum BorderStyle {
|
||||
SOLID,
|
||||
|
@ -73,7 +75,8 @@ import com.facebook.csslayout.Spacing;
|
|||
|
||||
/* Value at Spacing.ALL index used for rounded borders, whole array used by rectangular borders */
|
||||
private @Nullable Spacing mBorderWidth;
|
||||
private @Nullable Spacing mBorderColor;
|
||||
private @Nullable Spacing mBorderRGB;
|
||||
private @Nullable Spacing mBorderAlpha;
|
||||
private @Nullable BorderStyle mBorderStyle;
|
||||
|
||||
/* Used for rounded border and rounded background */
|
||||
|
@ -164,16 +167,37 @@ import com.facebook.csslayout.Spacing;
|
|||
}
|
||||
}
|
||||
|
||||
public void setBorderColor(int position, float color) {
|
||||
if (mBorderColor == null) {
|
||||
mBorderColor = new Spacing();
|
||||
mBorderColor.setDefault(Spacing.LEFT, DEFAULT_BORDER_COLOR);
|
||||
mBorderColor.setDefault(Spacing.TOP, DEFAULT_BORDER_COLOR);
|
||||
mBorderColor.setDefault(Spacing.RIGHT, DEFAULT_BORDER_COLOR);
|
||||
mBorderColor.setDefault(Spacing.BOTTOM, DEFAULT_BORDER_COLOR);
|
||||
public void setBorderColor(int position, float rgb, float alpha) {
|
||||
this.setBorderRGB(position, rgb);
|
||||
this.setBorderAlpha(position, alpha);
|
||||
}
|
||||
|
||||
private void setBorderRGB(int position, float rgb) {
|
||||
// set RGB component
|
||||
if (mBorderRGB == null) {
|
||||
mBorderRGB = new Spacing();
|
||||
mBorderRGB.setDefault(Spacing.LEFT, DEFAULT_BORDER_RGB);
|
||||
mBorderRGB.setDefault(Spacing.TOP, DEFAULT_BORDER_RGB);
|
||||
mBorderRGB.setDefault(Spacing.RIGHT, DEFAULT_BORDER_RGB);
|
||||
mBorderRGB.setDefault(Spacing.BOTTOM, DEFAULT_BORDER_RGB);
|
||||
}
|
||||
if (!FloatUtil.floatsEqual(mBorderColor.getRaw(position), color)) {
|
||||
mBorderColor.set(position, color);
|
||||
if (!FloatUtil.floatsEqual(mBorderRGB.getRaw(position), rgb)) {
|
||||
mBorderRGB.set(position, rgb);
|
||||
invalidateSelf();
|
||||
}
|
||||
}
|
||||
|
||||
private void setBorderAlpha(int position, float alpha) {
|
||||
// set Alpha component
|
||||
if (mBorderAlpha == null) {
|
||||
mBorderAlpha = new Spacing();
|
||||
mBorderAlpha.setDefault(Spacing.LEFT, DEFAULT_BORDER_ALPHA);
|
||||
mBorderAlpha.setDefault(Spacing.TOP, DEFAULT_BORDER_ALPHA);
|
||||
mBorderAlpha.setDefault(Spacing.RIGHT, DEFAULT_BORDER_ALPHA);
|
||||
mBorderAlpha.setDefault(Spacing.BOTTOM, DEFAULT_BORDER_ALPHA);
|
||||
}
|
||||
if (!FloatUtil.floatsEqual(mBorderAlpha.getRaw(position), alpha)) {
|
||||
mBorderAlpha.set(position, alpha);
|
||||
invalidateSelf();
|
||||
}
|
||||
}
|
||||
|
@ -327,8 +351,11 @@ import com.facebook.csslayout.Spacing;
|
|||
* {@link #getFullBorderWidth}.
|
||||
*/
|
||||
private int getFullBorderColor() {
|
||||
return (mBorderColor != null && !CSSConstants.isUndefined(mBorderColor.getRaw(Spacing.ALL))) ?
|
||||
(int) (long) mBorderColor.getRaw(Spacing.ALL) : DEFAULT_BORDER_COLOR;
|
||||
float rgb = (mBorderRGB != null && !CSSConstants.isUndefined(mBorderRGB.getRaw(Spacing.ALL))) ?
|
||||
mBorderRGB.getRaw(Spacing.ALL) : DEFAULT_BORDER_RGB;
|
||||
float alpha = (mBorderAlpha != null && !CSSConstants.isUndefined(mBorderAlpha.getRaw(Spacing.ALL))) ?
|
||||
mBorderAlpha.getRaw(Spacing.ALL) : DEFAULT_BORDER_ALPHA;
|
||||
return ReactViewBackgroundDrawable.colorFromAlphaAndRGBComponents(alpha, rgb);
|
||||
}
|
||||
|
||||
private void drawRectangularBackgroundWithBorders(Canvas canvas) {
|
||||
|
@ -419,8 +446,17 @@ import com.facebook.csslayout.Spacing;
|
|||
return mBorderWidth != null ? Math.round(mBorderWidth.get(position)) : 0;
|
||||
}
|
||||
|
||||
private static int colorFromAlphaAndRGBComponents(float alpha, float rgb) {
|
||||
int rgbComponent = 0x00FFFFFF & (int)rgb;
|
||||
int alphaComponent = 0xFF000000 & ((int)alpha) << 24;
|
||||
|
||||
return rgbComponent | alphaComponent;
|
||||
}
|
||||
|
||||
private int getBorderColor(int position) {
|
||||
// Check ReactStylesDiffMap#getColorInt() to see why this is needed
|
||||
return mBorderColor != null ? (int) (long) mBorderColor.get(position) : DEFAULT_BORDER_COLOR;
|
||||
float rgb = mBorderRGB != null ? mBorderRGB.get(position) : DEFAULT_BORDER_RGB;
|
||||
float alpha = mBorderAlpha != null ? mBorderAlpha.get(position) : DEFAULT_BORDER_ALPHA;
|
||||
|
||||
return ReactViewBackgroundDrawable.colorFromAlphaAndRGBComponents(alpha, rgb);
|
||||
}
|
||||
}
|
||||
|
|
|
@ -201,8 +201,8 @@ public class ReactViewGroup extends ViewGroup implements
|
|||
getOrCreateReactViewBackground().setBorderWidth(position, width);
|
||||
}
|
||||
|
||||
public void setBorderColor(int position, float color) {
|
||||
getOrCreateReactViewBackground().setBorderColor(position, color);
|
||||
public void setBorderColor(int position, float rgb, float alpha) {
|
||||
getOrCreateReactViewBackground().setBorderColor(position, rgb, alpha);
|
||||
}
|
||||
|
||||
public void setBorderRadius(float borderRadius) {
|
||||
|
|
|
@ -135,9 +135,9 @@ public class ReactViewManager extends ViewGroupManager<ReactViewGroup> {
|
|||
"borderColor", "borderLeftColor", "borderRightColor", "borderTopColor", "borderBottomColor"
|
||||
}, customType = "Color")
|
||||
public void setBorderColor(ReactViewGroup view, int index, Integer color) {
|
||||
view.setBorderColor(
|
||||
SPACING_TYPES[index],
|
||||
color == null ? CSSConstants.UNDEFINED : (float) color);
|
||||
float rgbComponent = color == null ? CSSConstants.UNDEFINED : (float) ((int)color & 0x00FFFFFF);
|
||||
float alphaComponent = color == null ? CSSConstants.UNDEFINED : (float) ((int)color >>> 24);
|
||||
view.setBorderColor(SPACING_TYPES[index], rgbComponent, alphaComponent);
|
||||
}
|
||||
|
||||
@ReactProp(name = ViewProps.COLLAPSABLE)
|
||||
|
|
Loading…
Reference in New Issue