Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 25.2k
feat(iOS/fabric): percentage support in translate#43192
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
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
14a4dde6829aa2887993050fd7fb411c84d5a6f86e9ac710698a597f1ed7559371a8165e63c8ab84a003ea3b0af783056a3d9b569776d67de677dd7File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -401,19 +401,33 @@ Transform BaseViewProps::resolveTransform( | ||
| const LayoutMetrics& layoutMetrics) const { | ||
| float viewWidth = layoutMetrics.frame.size.width; | ||
| float viewHeight = layoutMetrics.frame.size.height; | ||
| if (!transformOrigin.isSet() || (viewWidth == 0 && viewHeight == 0)) { | ||
| return transform; | ||
| auto transformMatrix = Transform{}; | ||
| if (viewWidth == 0 && viewHeight == 0){ | ||
| return transformMatrix; | ||
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Different from previous behaviour which returned the plain transform. Not sure it matters.
| ||
| } | ||
| std::array<float, 3> translateOffsets = | ||
| getTranslateForTransformOrigin(viewWidth, viewHeight, transformOrigin); | ||
| auto newTransform = Transform::Translate( | ||
| translateOffsets[0], translateOffsets[1], translateOffsets[2]); | ||
| newTransform = newTransform * transform; | ||
| newTransform = | ||
| newTransform * | ||
| // transform is matrix | ||
| if (transform.operations.size() == 1 && transform.operations[0].type == TransformOperationType::Arbitrary) { | ||
| transformMatrix = transform; | ||
| } else { | ||
| auto viewSize = Size{viewWidth, viewHeight}; | ||
| for (size_t i = 0; i < transform.operations.size(); i++) { | ||
| transformMatrix = transformMatrix * transform.FromTransformOperation(transform.operations[i], viewSize); | ||
| } | ||
| } | ||
| if (transformOrigin.isSet()) { | ||
| std::array<float, 3> translateOffsets = | ||
| getTranslateForTransformOrigin(viewWidth, viewHeight, transformOrigin); | ||
| transformMatrix = | ||
| Transform::Translate( | ||
| -translateOffsets[0], -translateOffsets[1], -translateOffsets[2]); | ||
| return newTransform; | ||
| translateOffsets[0], translateOffsets[1], translateOffsets[2]) | ||
| * transformMatrix | ||
| * Transform::Translate(-translateOffsets[0], -translateOffsets[1], -translateOffsets[2]); | ||
| } | ||
Comment on lines
+420
to
+428
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not do this before the transform processing loop? That way we can avoid less intermediate objects. ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can do | ||
| return transformMatrix; | ||
| } | ||
| bool BaseViewProps::getClipsContentToBounds() const { | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -503,6 +503,8 @@ inline void fromRawValue( | ||
| auto pair = configurationPair.begin(); | ||
| auto operation = pair->first; | ||
| auto& parameters = pair->second; | ||
| auto Zero = ValueUnit(0, UnitType::Point); | ||
| auto One = ValueUnit(1, UnitType::Point); | ||
| if (operation == "matrix") { | ||
| react_native_expect(parameters.hasType<std::vector<Float>>()); | ||
| @@ -513,48 +515,52 @@ inline void fromRawValue( | ||
| transformMatrix.matrix[i++] = number; | ||
| } | ||
| transformMatrix.operations.push_back( | ||
| TransformOperation{TransformOperationType::Arbitrary, 0, 0, 0}); | ||
| TransformOperation{TransformOperationType::Arbitrary, Zero, Zero, Zero}); | ||
| } else if (operation == "perspective") { | ||
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I missed on my initial look-through that we are using Don't want to make this PR too big, but we would replace ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
You mean operations like this right? Yeah, I considered
For sure, i can see it is in | ||
| transformMatrix = | ||
| transformMatrix * Transform::Perspective((Float)parameters); | ||
| transformMatrix.operations.push_back(TransformOperation{ | ||
| TransformOperationType::Perspective, ValueUnit((Float)parameters, UnitType::Point), Zero, Zero}); | ||
| } else if (operation == "rotateX") { | ||
| transformMatrix = transformMatrix * | ||
| Transform::Rotate(toRadians(parameters, 0.0f), 0, 0); | ||
| transformMatrix.operations.push_back( | ||
| TransformOperation{TransformOperationType::Rotate, ValueUnit(toRadians(parameters, 0.0f), UnitType::Point), Zero, Zero}); | ||
| } else if (operation == "rotateY") { | ||
| transformMatrix = transformMatrix * | ||
| Transform::Rotate(0, toRadians(parameters, 0.0f), 0); | ||
| transformMatrix.operations.push_back( | ||
| TransformOperation{TransformOperationType::Rotate, Zero, ValueUnit(toRadians(parameters, 0.0f), UnitType::Point), Zero}); | ||
| } else if (operation == "rotateZ" || operation == "rotate") { | ||
| transformMatrix = transformMatrix * | ||
| Transform::Rotate(0, 0, toRadians(parameters, 0.0f)); | ||
| transformMatrix.operations.push_back( | ||
| TransformOperation{TransformOperationType::Rotate, Zero, Zero, ValueUnit(toRadians(parameters, 0.0f), UnitType::Point)}); | ||
| } else if (operation == "scale") { | ||
| auto number = (Float)parameters; | ||
| transformMatrix = | ||
| transformMatrix * Transform::Scale(number, number, number); | ||
| auto number = ValueUnit((Float)parameters, UnitType::Point); | ||
| transformMatrix.operations.push_back(TransformOperation{ | ||
| TransformOperationType::Scale, number, number, number}); | ||
| } else if (operation == "scaleX") { | ||
| transformMatrix = | ||
| transformMatrix * Transform::Scale((Float)parameters, 1, 1); | ||
| transformMatrix.operations.push_back(TransformOperation{ | ||
| TransformOperationType::Scale, ValueUnit((Float)parameters, UnitType::Point), One, One}); | ||
| } else if (operation == "scaleY") { | ||
| transformMatrix = | ||
| transformMatrix * Transform::Scale(1, (Float)parameters, 1); | ||
| transformMatrix.operations.push_back(TransformOperation{ | ||
| TransformOperationType::Scale, One, ValueUnit((Float)parameters, UnitType::Point), One}); | ||
| } else if (operation == "scaleZ") { | ||
| transformMatrix = | ||
| transformMatrix * Transform::Scale(1, 1, (Float)parameters); | ||
| transformMatrix.operations.push_back(TransformOperation{ | ||
| TransformOperationType::Scale, One, One, ValueUnit((Float)parameters, UnitType::Point)}); | ||
| } else if (operation == "translate") { | ||
| auto numbers = (std::vector<Float>)parameters; | ||
| transformMatrix = transformMatrix * | ||
| Transform::Translate(numbers.at(0), numbers.at(1), 0); | ||
| auto numbers = (std::vector<RawValue>)parameters; | ||
| auto valueX = ValueUnit::getValueUnitFromRawValue(numbers.at(0)); | ||
| auto valueY = ValueUnit::getValueUnitFromRawValue(numbers.at(1)); | ||
| transformMatrix.operations.push_back(TransformOperation{ | ||
| TransformOperationType::Translate, valueX, valueY, Zero}); | ||
| } else if (operation == "translateX") { | ||
| transformMatrix = | ||
| transformMatrix * Transform::Translate((Float)parameters, 0, 0); | ||
| auto value = ValueUnit::getValueUnitFromRawValue(parameters); | ||
| transformMatrix.operations.push_back(TransformOperation{ | ||
| TransformOperationType::Translate, value, Zero, Zero}); | ||
| } else if (operation == "translateY") { | ||
| transformMatrix = | ||
| transformMatrix * Transform::Translate(0, (Float)parameters, 0); | ||
| auto value = ValueUnit::getValueUnitFromRawValue(parameters); | ||
| transformMatrix.operations.push_back(TransformOperation{ | ||
| TransformOperationType::Translate, Zero, value, Zero}); | ||
| } else if (operation == "skewX") { | ||
| transformMatrix = | ||
| transformMatrix * Transform::Skew(toRadians(parameters, 0.0f), 0); | ||
| transformMatrix.operations.push_back( | ||
| TransformOperation{TransformOperationType::Skew, ValueUnit(toRadians(parameters, 0.0f), UnitType::Point), Zero, Zero}); | ||
| } else if (operation == "skewY") { | ||
| transformMatrix = | ||
| transformMatrix * Transform::Skew(0, toRadians(parameters, 0.0f)); | ||
| transformMatrix.operations.push_back( | ||
| TransformOperation{TransformOperationType::Skew, Zero, ValueUnit(toRadians(parameters, 0.0f), UnitType::Point), Zero}); | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this is an existing pattern, but for the future, we've been trying to move towards no-oping on invalid values, instead of creating a redbox, or something more disruptive.
Even longer into the future (not worth thinking about yet), on Fabric side, we want to just do all this parsing in native.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of this parsing does happen in native.
validateTransformis just an additional DEV-time only validation of the input to get better signal to the developer.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, but we have though redbox during development on invalid style is too aggressive/disruptive.