Skip to content
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

[TIMOB-25238] Android: Refactor TiBorderWrapperView to improve performance #9410

Merged
merged 5 commits into from Oct 2, 2017

Conversation

garymathews
Copy link
Contributor

  • Refactor TiBorderWrapperView to improve performance of Titanium.UI.View with borderColor defined
TEST CASE #1
var win = Ti.UI.createWindow({backgroundColor: 'gray'}),
    list = Ti.UI.createListView({
        templates: {
            template: {
                childTemplates: [
                    {
                        type: 'Ti.UI.View',
                        properties: {
                            left: 5,
                            right: 5,
                            top: 5,
                            backgroundColor: 'blue',
                            height: 90,
                            bottom: 5,
                            borderColor: 'orange',
                            borderRadius: 10,
                            borderWidth: 5
                        },
                        childTemplates: [
                            {
                                type: 'Ti.UI.View',
                                bindId: 'item',
                                properties: {
                                    left: 10,
                                    right: 10,
                                    height: 30,
                                    backgroundColor: 'red',
                                    borderColor: 'purple',
                                    borderRadius: 5,
                                    borderWidth: 3
                                }
                            }
                        ]
                    }
                ]
            }
        },
        defaultItemTemplate: 'template'
    }),
    section = Ti.UI.createListSection(),
    items = [];

for (var i = 0; i < 64; i++) {
    items.push({
        item: {
            width: '50%'
        }
    });
}

section.setItems(items);
list.setSections([section]);

win.add(list);
win.open();
  • Should scroll without UI lag
TEST CASE #2
var win = Ti.UI.createWindow({backgroundColor: 'white'}),
    a = Ti.UI.createView({
        top: 10,
        left: 10,
        width: 200,
        height: 100,
        elevation: 20,
        backgroundColor: 'red',
        borderColor: 'blue',
        borderWidth: 8
    }),
    b = Ti.UI.createView({
        top: 10,
        right: 10,
        width: 100,
        height: 100,
        elevation: 20,
        opacity: 0.5,
        backgroundColor: 'blue',
        borderColor: 'red',
        borderWidth: 8,
        borderRadius: 100
    });

win.add([a, b]);
win.open();
  • Rectangle should have a border and elevation
  • Circle should have a border with no aliasing

JIRA Ticket

@garymathews garymathews added this to the 7.0.0 milestone Sep 7, 2017
Copy link
Contributor

@ypbnv ypbnv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

@jquick-axway
Copy link
Contributor

I'd like to make a suggestion. (Note that this suggestion does not block this PR.)

It would be nice if our TiUIView.hasBorder() method would only return true if the border width or radius is greater than zero. The border color property should be ignored in this check since it only applies if a border width is greater than zero.
https://github.com/appcelerator/titanium_mobile/blob/master/android/titanium/src/java/org/appcelerator/titanium/view/TiUIView.java#L445

As it is now, a border view will always be applied to a view, even if these properties are initialized to zero (like how it is with WebView). So, not only will this get rid of the extra canvas draws/clipping in TiBorderWrapperView that'll do nothing, it'll also help prevent hardware acceleration from being disabled when it doesn't need to be.

@lokeshchdhry
Copy link
Contributor

FR Passed.

For the first test - UI does not show any lag & for the second test -- Rectangle shows border and elevation & circle has a border with no aliasing.

Studio Ver: 4.10.0.201709271713
SDK Ver: 7.0.0 local build
OS Ver: 10.12.3
Xcode Ver: Xcode 8.3.3
Appc NPM: 4.2.9
Appc CLI: 6.2.4
Ti CLI Ver: 5.0.14
Alloy Ver: 1.9.14
Node Ver: 7.10.1
Java Ver: 1.8.0_101
Devices: ⇨ google Nexus 5 --- Android 6.0.1
⇨ google Pixel --- Android 7.1.1

@build
Copy link
Contributor

build commented Oct 2, 2017

Warnings
⚠️

🔬 There are library changes, but no changes to the unit tests. That's OK as long as you're refactoring existing code

Messages
📖

👍 Hey!, You deleted more code than you added. That's awesome!

Generated by 🚫 dangerJS

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants