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.

Sign up for GitHub

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

Jump to bottom

[TIMOB-25461] HEX backgroundColor with alpha channel act as a mask #9588

Closed
wants to merge 18 commits into from
Closed

[TIMOB-25461] HEX backgroundColor with alpha channel act as a mask #9588

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
1b8c339
Change HW acceleration checks.
ypbnv Nov 9, 2017
06e0363
Merge branch 'master' into TIMOB-25461
ypbnv Nov 10, 2017
0c8e0fc
Merge branch 'master' into TIMOB-25461
ypbnv Nov 14, 2017
e73e09d
Apply some changes again.
ypbnv Nov 14, 2017
526343d
Merge branch 'master' into TIMOB-25461
ypbnv Nov 15, 2017
f4cbda2
Merge branch 'master' into TIMOB-25461
ypbnv Nov 16, 2017
0861b0e
Avoid potential warning message.
ypbnv Nov 16, 2017
bb7f488
Merge branch 'master' into TIMOB-25461
ypbnv Jan 4, 2018
11ad53c
Fix formatting.
ypbnv Jan 4, 2018
2eb23b8
Merge branch 'master' into TIMOB-25461
ypbnv Jan 19, 2018
301cb3b
Merge branch 'master' into TIMOB-25461
ypbnv Jan 23, 2018
8326d79
Merge branch 'master' into TIMOB-25461
ypbnv Feb 13, 2018
d5d2e3e
Merge branch 'master' into TIMOB-25461
ypbnv Feb 20, 2018
3207259
Add unit test.
ypbnv Feb 20, 2018
7b89c8a
Merge branch 'master' into TIMOB-25461
sgtcoolguy Jun 26, 2018
7dbd622
Lint the unit tests.
ypbnv Jun 28, 2018
80af595
Merge branch 'master' into TIMOB-25461
ypbnv Jun 28, 2018
8163828
Merge branch 'master' into TIMOB-25461
ypbnv Sep 18, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 12 additions & 8 deletions android/modules/ui/src/java/ti/modules/titanium/ui/widget/webview/TiUIWebView.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,16 @@ public boolean onCheckIsTextEditor()
return super.onCheckIsTextEditor();
}
}


@Override
protected boolean hasBorder(KrollDict d) {
Copy link
Contributor

Choose a reason for hiding this comment

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

minor, but a warning could be thrown suggesting the method does not return a value (even though it does). You should change it to this:

protected boolean hasBorder(KrollDict d) {
	if (LOWER_THAN_JELLYBEAN) {
		return false;
	}
	return super.hasBorder(d);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.

if (LOWER_THAN_JELLYBEAN) {
return false;
} else {
return super.hasBorder(d);
}
}

private boolean isHTCSenseDevice()
{
boolean isHTC = false;
Expand Down Expand Up @@ -938,12 +947,7 @@ public boolean interceptOnBackPressed()
}

@Override
protected void disableHWAcceleration()
{
if (Build.VERSION.SDK_INT <= Build.VERSION_CODES.JELLY_BEAN) {
super.disableHWAcceleration();
} else {
Log.d(TAG, "Do not disable HW acceleration for WebView.", Log.DEBUG_MODE);
}
protected void disableHWAcceleration() {
Log.d(TAG, "Do not disable HW acceleration for WebView.", Log.DEBUG_MODE);
}
}
17 changes: 15 additions & 2 deletions android/titanium/src/java/org/appcelerator/titanium/view/TiBorderWrapperView.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ protected void onDraw(Canvas canvas)
{
getDrawingRect(bounds);


int maxPadding = (int) Math.min(bounds.right / 2, bounds.bottom / 2);
int padding = (int) Math.min(borderWidth, maxPadding);
RectF innerRect = new RectF(bounds.left + padding, bounds.top + padding, bounds.right - padding, bounds.bottom - padding);
Expand All @@ -66,27 +67,39 @@ protected void onDraw(Canvas canvas)

Path outerPath = new Path();
if (radius > 0f) {
//region create paths
float innerRadius = radius - padding;
if (innerRadius > 0f) {
outerPath.addRoundRect(innerRect, innerRadius, innerRadius, Direction.CW);
} else {
outerPath.addRect(innerRect, Direction.CW);
}
Path innerPath = new Path(outerPath);
//endregion

// draw border
//region draw border
outerPath.addRoundRect(outerRect, radius, radius, Direction.CCW);
canvas.drawPath(outerPath, paint);
//endregion

//region draw inner path with background color
// TIMOB-16909: hack to fix anti-aliasing
if (backgroundColor != Color.TRANSPARENT) {
paint.setColor(backgroundColor);
canvas.drawPath(innerPath, paint);
}
//endregion

//region clip inner path
canvas.clipPath(innerPath);
//endregion

//region fill canvas with transparency
if (backgroundColor != Color.TRANSPARENT) {
canvas.drawColor(0, PorterDuff.Mode.CLEAR);
canvas.drawColor(Color.TRANSPARENT, PorterDuff.Mode.CLEAR);
}
//endregion

} else {
outerPath.addRect(outerRect, Direction.CW);
outerPath.addRect(innerRect, Direction.CCW);
Expand Down
13 changes: 6 additions & 7 deletions android/titanium/src/java/org/appcelerator/titanium/view/TiUIView.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public abstract class TiUIView

private static final boolean HONEYCOMB_OR_GREATER = (Build.VERSION.SDK_INT >= 11);
private static final boolean LOLLIPOP_OR_GREATER = (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP);
private static final boolean LOWER_THAN_JELLYBEAN = (Build.VERSION.SDK_INT < Build.VERSION_CODES.JELLY_BEAN_MR2);
protected static final boolean LOWER_THAN_JELLYBEAN = (Build.VERSION.SDK_INT < Build.VERSION_CODES.JELLY_BEAN_MR2);
private static final boolean LOWER_THAN_MARSHMALLOW = (Build.VERSION.SDK_INT < Build.VERSION_CODES.M);

private static final int LAYER_TYPE_SOFTWARE = 1;
Expand Down Expand Up @@ -442,7 +442,7 @@ private boolean hasGradient(KrollDict d)
return d.containsKeyAndNotNull(TiC.PROPERTY_BACKGROUND_GRADIENT);
}

private boolean hasBorder(KrollDict d)
protected boolean hasBorder(KrollDict d)
{
return d.containsKeyAndNotNull(TiC.PROPERTY_BORDER_COLOR)
|| (d.containsKeyAndNotNull(TiC.PROPERTY_BORDER_WIDTH)
Expand Down Expand Up @@ -886,8 +886,8 @@ public void propertyChanged(String key, Object oldValue, Object newValue, KrollP

// TIMOB-24898: disable HW acceleration to allow transparency
// when the backgroundColor alpha channel has been set
byte bgAlpha = bgColor != null ? (byte)(bgColor >> 24) : (byte)0xFF;
if (bgAlpha != 0xFF) {

if (bgColor != null && bgColor >>> 24 != 0xFF) {
disableHWAcceleration();
}
}
Expand Down Expand Up @@ -1451,8 +1451,7 @@ private void initializeBorder(KrollDict d, Integer bgColor)

// TIMOB-24898: disable HW acceleration to allow transparency
// when the backgroundColor alpha channel has been set
byte bgAlpha = bgColor != null ? (byte)(bgColor >> 24) : (byte)0xFF;
if (bgAlpha != 0xFF) {
if (bgColor != null && bgColor >>> 24 != 0xFF) {
disableHWAcceleration();
}
}
Expand Down Expand Up @@ -1990,7 +1989,7 @@ public boolean onLongClick(View view)

protected void disableHWAcceleration()
{
if (borderView == null || (Build.VERSION.SDK_INT > Build.VERSION_CODES.JELLY_BEAN && !borderView.isHardwareAccelerated())) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This was added to resolve a WebView crash on Android 4.1...
https://jira.appcelerator.org/browse/TIMOB-25266

The reason it crashed is because a BorderView was always being applied to a WebView (because it initializing itself with a zero border width unlike other views) and a hardware accelerator border causes a C/C++ segfault, likely because the WebKit driven WebView's hardware accelerated rendering conflicts with it.

We don't want to re-introduce this crash. So, if you feel we need to change the code here, then we may need to look into an alternative solution for Android 4.1 WebViews. We could simply always disable hardware accelerated WebView rendering on 4.1, but my understanding is that could break HTML5 video rendering. The other approach is to keep hardware accelerated rendering but don't support a border. Hmm...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I double checked the test cases in the ticket and the PR. There are no crashes on API 16. But I have tested only on emulator - no device with that version at hand.

TiBorderWrapperView currently draws a border if we have set a borderColor or a borderWidth greater than 1 or borderRadius greater than 1.
https://github.com/appcelerator/titanium_mobile/blob/master/android/titanium/src/java/org/appcelerator/titanium/view/TiUIView.java#L445
So I believe there is not reason to reintroduce this problem.

As for the conflict for HTML 5 video rendering when having a border on API 16 - I suppose we could drop the support for borderRadius only. It is the one which combined with opacity and background color with alpha channel requires disabling of HW acceleration - the unsupported methods for this API from:
https://developer.android.com/guide/topics/graphics/hardware-accel.html#unsupported
are called in case we have a radius:
https://github.com/appcelerator/titanium_mobile/blob/master/android/titanium/src/java/org/appcelerator/titanium/view/TiBorderWrapperView.java#L86

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for looking into this.

I suppose the thing to do is have QE re-run the test attached to PR #9432 when testing this PR, just in case. @lokeshchdhry has done the test before and has a physical Android 4.1. device to test with.

if (borderView == null) {
return;
}
Log.d(TAG, "Disabling hardware acceleration for instance of " + borderView.getClass().getSimpleName(),
Expand Down