779152 - Use TextureView where available

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Toolbar, defect)

Description

[James Willcox (:snorp) ([e-mail]) (he/him)]

Right now we use an old school SurfaceView for rendering Gecko content. As of ICS, a new option exists called TextureView. This integrates into the OpenGL rendering present on ICS and higher, and would allow us to transform and smoothly animate the gecko view.

Comment 1

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Render into a TextureView on Android where available

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 647548 [details] [diff] [review]
Render into a TextureView on Android where available

Review of attachment 647548 [details] [diff] [review]:
-----------------------------------------------------------------

Cool! However you'll need to rebase this patch on top of the one from bug 770047, please test to make sure it doesn't re-introduce the black flash on Jelly Bean devices.

::: mobile/android/base/gfx/LayerView.java
@@ +277,5 @@
> +    }
> +
> +    private class SurfaceTextureListener implements TextureView.SurfaceTextureListener {
> +        public void onSurfaceTextureAvailable(SurfaceTexture surface, int width, int height) {
> +            // We don't do this for surfaceChanged above because it is always followed by a surfaceChanged,

Should the first "surfaceChanged" in this sentence actually be "surfaceCreated"? (And to be clear, I would prefer SurfaceListener.surfaceCreated and SurfaceListener.surfaceChanged).

Comment 3

[James Willcox (:snorp) ([e-mail]) (he/him)]

(In reply to Kartikaya Gupta (:kats) from comment #2)
> Comment on attachment 647548 [details] [diff] [review]
> Render into a TextureView on Android where available
> 
> Review of attachment 647548 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Cool! However you'll need to rebase this patch on top of the one from bug
> 770047, please test to make sure it doesn't re-introduce the black flash on
> Jelly Bean devices.
> 
> ::: mobile/android/base/gfx/LayerView.java
> @@ +277,5 @@
> > +    }
> > +
> > +    private class SurfaceTextureListener implements TextureView.SurfaceTextureListener {
> > +        public void onSurfaceTextureAvailable(SurfaceTexture surface, int width, int height) {
> > +            // We don't do this for surfaceChanged above because it is always followed by a surfaceChanged,
> 
> Should the first "surfaceChanged" in this sentence actually be
> "surfaceCreated"?

Oops, indeed. Fixed.

 (And to be clear, I would prefer
> SurfaceListener.surfaceCreated and SurfaceListener.surfaceChanged).

Well we aren't going to do anything in response to that are we? We only care when the size changes? It does kinda suck, but I left it as-is.

Comment 4

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Render into a TextureView on Android where available

Comment 5

[James Willcox (:snorp) ([e-mail]) (he/him)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fe59d3af41f2

Comment 6

[Kartikaya Gupta (email:[e-mail])]

(In reply to James Willcox (:snorp) ([e-mail]) from comment #3)
>  (And to be clear, I would prefer
> > SurfaceListener.surfaceCreated and SurfaceListener.surfaceChanged).
> 
> Well we aren't going to do anything in response to that are we? We only care
> when the size changes? It does kinda suck, but I left it as-is.

Oh, I totally didn't word that properly. I meant that the comment should refer to "SurfaceListener.surfaceCreated" instead of just "surfaceCreated" for clarity. But it's not a big deal.

Comment 7

[James Willcox (:snorp) ([e-mail]) (he/him)]

BTW, I did test to make sure this does not reintroduce the black flash on JB. (It does not)

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/fe59d3af41f2