[TIMOB-16596] Android: Animation anchorPoints not working #10007

ypbnv · 2018-04-19T13:58:00Z

JIRA: https://jira.appcelerator.org/browse/TIMOB-16596

Description:
Use Animation's anchorPoint property instead of Operation's.

Note: Currently no unit tests can be added for this. They will be included in an upcoming PR addressing other issues with animation that prevents this.

Test case:

In the JIRA ticket.

app.js

var t = Ti.UI.create2DMatrix({rotate:90, anchorPoint:{x: 0, y:0}});

var a = {
  transform: t,
  duration: 2000,
  // This property can be used to check the priority of anchorPoint used
  //anchorPoint: {x:1, y:1}
};

var win = Ti.UI.createWindow();
var view = Ti.UI.createView({
  backgroundColor:'#336699',
  width:100, height:100
});

win.add(view);

var button = Ti.UI.createButton({
  title:'Animate ',
  height: 80,
  width: 300,
  top:30
});

win.add(button);

button.addEventListener('click', function(){
  button.touchEnabled = false;
  view.animate(a, function(){
    button.touchEnabled = true;
  });
});

win.open();

build · 2018-04-19T14:18:08Z

	Messages
📖	👍 Hey!, You deleted more code than you added. That's awesome!
📖	💾 Here's the generated SDK zipfile.

Generated by 🚫 dangerJS

jquick-axway · 2018-04-19T18:40:10Z

android/titanium/src/java/org/appcelerator/titanium/util/TiAnimationBuilder.java

@@ -360,7 +360,7 @@ private AnimatorSet buildPropertyAnimators(int x, int y, int w, int h, int paren
 				for (Operation operation : operations) {
 					if (operation.anchorX != Ti2DMatrix.DEFAULT_ANCHOR_VALUE
 						|| operation.anchorY != Ti2DMatrix.DEFAULT_ANCHOR_VALUE) {
-						setAnchor(w, h, operation.anchorX, operation.anchorY);
+						setAnchor(w, h, anchorX, anchorY);


I think the if-condition should be written like the below.

final float EPSILON = Math.ulp(1.0f); boolean hasAnchorXChanged = (Math.abs(operation.anchorX - anchorX) >= EPSILON); boolean hasAnchorYChanged = (Math.abs(operation.anchorY - anchorY) >= EPSILON); if (hasAnchorXChanged || hasAnchorYChanged) { setAnchor(w, h, anchorX, anchorY); }

This way if the first time the operations were using DEFAULT_ANCHOR_VALUE and the anchor was changed to something else, then the operations would be updated instead of locking-in to using the previous values. The above effectively doing operation.anchorX != anchorX, but with floats.

(I've never played with this code before. So, hopefully I'm understanding things correctly. Am I right?)

I don't think that's what was intended. The test case has an animation with anchor point that is different by the default one, which is then followed by an animation with the default one. And this will result in playing the first animation twice, because the difference will be less than EPSILON.

I also found out that we can have anchorPoint as a parameter in the creation dictionary of Ti.UI.2DMatrix. And my changes totally ignore that, which is not OK. So I will revisit the code, check what is the priority on iOS and fix it.

ypbnv · 2018-05-02T15:32:49Z

@jquick-axway I have updated the PR. Now the animation will use anchorPoint in the following order of priority:

anchorPoint set as a property in the instance of Ti.UI.Animation;
anchorPoint set in the creation dictionary matrix of the transformation;
This is an Android only case, because anchorPoint as a part of the creation dictionary for 2DMatrix is not supported on iOS. To me it makes sense to have that priority. I have added a note for that in the docs as well. I have added another test case too.

lokeshchdhry · 2018-05-02T23:15:29Z

@ypbnv , I see that anchorPoints are not working on android 4.1. Works fine on Android 8.0 & Android 6.0.1 with the fix.

jquick-axway · 2018-05-02T23:19:36Z

android/titanium/src/java/org/appcelerator/titanium/util/TiAnimationBuilder.java

@@ -412,6 +412,10 @@ private AnimatorSet buildPropertyAnimators(int x, int y, int w, int h, int paren
 			}
 		}

+		if (anchorX != Ti2DMatrix.DEFAULT_ANCHOR_VALUE && anchorY != Ti2DMatrix.DEFAULT_ANCHOR_VALUE) {
+			setAnchor(w, h, anchorX, anchorY);
+		}


Your if-check should be doing an OR ||, not an AND &&.

jquick-axway · 2018-05-02T23:25:07Z

@ypbnv, can you also remove all of the PRE_HONEYCOMB parts please?
That's Android 3.x which we do not support anymore. Thanks.

Remove PRE_HONEYCOMB checks. Improve default anchorPoints check.

ypbnv · 2018-05-03T13:29:47Z

@jquick-axway , @lokeshchdhry Updated with fix for API 16-19. I don't like the way it is done, but couldn't figure out anything else. I assume this is the reason for it:
API 18:
https://android.googlesource.com/platform/frameworks/base.git/+/jb-release/core/java/android/view/View.java#9110
API 19:
https://android.googlesource.com/platform/frameworks/base.git/+/kitkat-release/core/java/android/view/View.java#9703

To me it looks like it was not accounting if the pivot point was explicitly set and since 0 matched with the default values in TransformationInfo class it just ignored it.

Also removed unused code and improved the current check for default anchorPoint coordinates.

jquick-axway · 2018-05-03T18:34:05Z

android/titanium/src/java/org/appcelerator/titanium/util/TiAnimationBuilder.java

+		// case.
+		if (PRE_LOLLIPOP && thisAnchorX == 0 && thisAnchorY == 0) {
+			pivotX++;
+			pivotY++;


Pivot is a normalized value between 0.0 and 1.0.

So, if given pivot is (0,0) which is the top-left corner, then adding 1 to both will make it pivot at the bottom-right corner, right?

Perhaps adding epsilon would be better?

On our end - yes, it is between 0.0 and 1.0. In the Android code it is passed as pixels.
view.setPivotX() leads to
https://android.googlesource.com/platform/frameworks/base.git/+/master/core/java/android/view/RenderNode.java#632

It is multiplied by the view's dimensions here:
https://github.com/appcelerator/titanium_mobile/blob/master/android/titanium/src/java/org/appcelerator/titanium/util/TiAnimationBuilder.java#L1484
and here:
https://github.com/appcelerator/titanium_mobile/blob/master/android/titanium/src/java/org/appcelerator/titanium/util/TiAnimationBuilder.java#L1488

So in the end it will set the pivot point in the pixel in position (1,1).
Increasing them with EPSILON will work, too. And it will be closer to the expected result.

Ahh... thanks man. I didn't know that. :)

Fix comment typo.

jquick-axway

CR: Pass

lokeshchdhry · 2018-05-07T18:47:45Z

FR Passed.

Animation anchor points works as expected.

Studio Ver: 5.1.0.201804230827
SDK Ver: 7.2.0 local build
OS Ver: 10.13.4
Xcode Ver: Xcode 9.3
Appc NPM: 4.2.13
Appc CLI: 7.0.3
Daemon Ver: 1.1.1
Ti CLI Ver: 5.1.0
Alloy Ver: 1.12.0
Node Ver: 8.9.1
NPM Ver: 5.5.1
Java Ver: 10
Devices: ⇨ google Nexus 5 --- Android 6.0.1
⇨ google Nexus 6P --- Android 8.0.0
Emulator: ⇨ Android 4.1.2

Fix anchorPoint for Ti.UI.Animation.

657e8cd

ypbnv added android bug no tests labels Apr 19, 2018

ypbnv added this to the 7.2.0 milestone Apr 19, 2018

ypbnv requested a review from jquick-axway April 19, 2018 13:58

build added the android label Apr 19, 2018

jquick-axway suggested changes Apr 19, 2018

View reviewed changes

ypbnv added 2 commits May 2, 2018 18:27

Apply anchorPoint in order.

88e7c40

Merge branch 'master' into TIMOB-16596

9b0c0cd

build added android docs labels May 2, 2018

jquick-axway suggested changes May 2, 2018

View reviewed changes

Fix pivot points for APIs 16-18.

7ea056b

Remove PRE_HONEYCOMB checks. Improve default anchorPoints check.

build added android docs labels May 3, 2018

Merge branch 'master' into TIMOB-16596

611f503

jquick-axway reviewed May 3, 2018

View reviewed changes

build added android docs labels May 3, 2018

ypbnv added 2 commits May 4, 2018 12:02

Use EPSILON instead of 1.

4d6ea31

Fix comment typo.

Merge branch 'master' into TIMOB-16596

993f9ba

build added android docs labels May 4, 2018

build added docs android labels May 4, 2018

jquick-axway approved these changes May 4, 2018

View reviewed changes

jquick-axway added the in-qe-testing 🕵 label May 4, 2018

Merge branch 'master' into TIMOB-16596

aea9ce7

build added android docs labels May 7, 2018

lokeshchdhry merged commit db37272 into tidev:master May 7, 2018

sgtcoolguy modified the milestones: 7.2.0, 7.3.0 May 16, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[TIMOB-16596] Android: Animation anchorPoints not working #10007

[TIMOB-16596] Android: Animation anchorPoints not working #10007

ypbnv commented Apr 19, 2018 •

edited

build commented Apr 19, 2018 •

edited

jquick-axway Apr 19, 2018

ypbnv Apr 24, 2018 •

edited

ypbnv commented May 2, 2018

lokeshchdhry commented May 2, 2018

jquick-axway May 2, 2018 •

edited

jquick-axway commented May 2, 2018

ypbnv commented May 3, 2018

jquick-axway May 3, 2018

ypbnv May 4, 2018 •

edited

jquick-axway May 4, 2018

jquick-axway left a comment

lokeshchdhry commented May 7, 2018

[TIMOB-16596] Android: Animation anchorPoints not working #10007

[TIMOB-16596] Android: Animation anchorPoints not working #10007

Conversation

ypbnv commented Apr 19, 2018 • edited

build commented Apr 19, 2018 • edited

jquick-axway Apr 19, 2018

Choose a reason for hiding this comment

ypbnv Apr 24, 2018 • edited

Choose a reason for hiding this comment

ypbnv commented May 2, 2018

lokeshchdhry commented May 2, 2018

jquick-axway May 2, 2018 • edited

Choose a reason for hiding this comment

jquick-axway commented May 2, 2018

ypbnv commented May 3, 2018

jquick-axway May 3, 2018

Choose a reason for hiding this comment

ypbnv May 4, 2018 • edited

Choose a reason for hiding this comment

jquick-axway May 4, 2018

Choose a reason for hiding this comment

jquick-axway left a comment

Choose a reason for hiding this comment

lokeshchdhry commented May 7, 2018

ypbnv commented Apr 19, 2018 •

edited

build commented Apr 19, 2018 •

edited

ypbnv Apr 24, 2018 •

edited

jquick-axway May 2, 2018 •

edited

ypbnv May 4, 2018 •

edited