1507074 - Shader compilation failed when GL_OES_EGL_image_external_essl3 is not supported on android device

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect, P3)

Description

[Sotaro Ikeda [:sotaro]]

This bug is created by Bug 1499255  Comment 8.

Shader compilation failed when GL_OES_EGL_image_external_essl3 is not supported on android device. I saw it on HTC Nexus 9(Android 7.1.1) with the following log.

> E webrender::device::gl: Failed to compile shader: brush_image
> E webrender::device::gl: 0(21) : error C0202: extension OES_EGL_image_external_essl3 not supported
> E webrender::device::gl: 0(194) : error C7531: global type samplerExternalOES requires "#extension GL_OES_EGL_image_external : enable" before use
> E webrender::device::gl: 0(1217) : error C1115: unable to find compatible overloaded function "textureSize(struct samplerExternalOES, int)"
> I Gecko   : [GFX1-]: wr_renderer_render: Shader(Compilation("brush_image", "0(21) : error C0202: extension OES_EGL_image_external_essl3 not supported\n0(194) : error C7531: global type samplerExternalOES requires \"#extension GL_OES_EGL_image_external : enable\" before use\n0(1217) : error C1115: unable to find compatible overloaded function \"textureSize(struct samplerExternalOES, int)\"\n"))

Comment 2

[Jamie Nicol [:jnicol]]

A user has hit this on a Lenovo K3 Note (Mali-T760, Android 7) which supports GLES 3.1, but not the essl3 version of the extension. Let's make this block Mali-T rollout

Comment 3

[Jamie Nicol [:jnicol]]

Hrmm, from Sotaro's comment it seems like on the nvidia GPU in the Nexus 9 this indeed wouldn't be too much work, as the non-essl3 version of the extension appears to work fine in a #version 300 es shader. Working around the lack of textureSize would likely be the only work required, as Dzmitry points out.

Unfortunately, however, it doesn't appear to be so simple on old Mali devices. Requiring GL_OES_EGL_image_external (no-essl3) results in an error in a #version 300 es shader. According to this forum post that seems to be the expected behaviour: technically spec compliant but very annoying nonetheless.

If I declare our shaders as #version 100 then requiring the extension works correctly. However, there will then be a large number of issues where our shaders don't work in ESSL 1.00. First, using attribute instead of in: easy. Next, no uint or texelFetch support: harder. Presumably more issues too after those.

Currently lots of our shaders have a TEXTURE_EXTERNAL variant, though I'm unsure how many of them will get used in practice. Probably mainly just composite and brush_image. But in theory could any of them be used? eg by wrapping a video in a filter etc? I don't know how feasible making all of our shader code ESSL 1.00 compatible is, I expect not very.

Perhaps as an alternative, we can create a single ESSL 1.00 shader which simply copies from a samplerExternalOES to a regular texture. Then whenever we need to render an external texture we first copy it, then use the regular TEXTURE_2D shader variant. This would mean an extra copy when playing video, on precisely the low-end devices which can afford to do that the least. But on the other hand it will be a lot less work and the number of GLES3-but-no-image_external_essl3 devices is presumably quite low, so maybe it's the best short-term solution. Maybe as a compromise we can make a ESSL 1.0 compatible composite shader, because presumably that is the common case, and use the additional copy for the other cases.

Glenn, does adding an extra copy sound like a viable solution to you? Could you point me in the direction of how to implement such a thing? I assume this is what the render task graph is for? (Not an area of the code I've looked at before)

Comment 4

[Jamie Nicol [:jnicol]]

Removing this as a Mali-T release blocker. Initially we can just roll out to Mali-T devices which also support OES_EGL_image_external_essl3.

Re-adding it as a wr-android blocker, it's not just a GLES 2 issue, as there are a small number of GLES 3 devices without support for this extension

Comment 5

[Glenn Watson [:gw]]

I think that the GL_OES_EGL_image_external extension is used for video playback (and maybe webgl / 2d canvas?). It shouldn't be used for anything created by WR.

I wonder if there is a fallback path / option in Gecko to not create textures with this sampler type? (maybe they can go through a slower path?). What does regular Gecko do now for video on devices that don't support that extension?

Otherwise, a copy sounds feasible, but not great. Shouldn't be any need to do a render task etc for this, we could probably just do it in the renderer when we lock / reference one of these textures, if that's the option we end up with.

I'm not sure about making composite/brush_image be 1.0 compatible - it might actually be feasible. I'd need to take a look at the spec, but I wouldn't assume it's totally impossible without investigating.

Comment 6

[Jamie Nicol [:jnicol]]

(In reply to Glenn Watson [:gw] from comment #5)

I think that the GL_OES_EGL_image_external extension is used for video playback (and maybe webgl / 2d canvas?). It shouldn't be used for anything created by WR.

Correct. Anything that we use the android SurfaceTexture API for, so video and webgl I think.

I wonder if there is a fallback path / option in Gecko to not create textures with this sampler type? (maybe they can go through a slower path?). What does regular Gecko do now for video on devices that don't support that extension?

I'll have a look, but I'm doubtful this is possible without being worse performance than us adding a copy. Android media APIs are all built around SurfaceTextures, though I think we used to have an alternative path so I'll dig through some old code.

In opengl layers we use GLES 2.0 and ESSL1.0, and I guess the non-essl3 extension is supported everywhere.

Comment 7

[Jamie Nicol [:jnicol]]

Attachment: Bug 1507074 - Update glslopt-rs to add support for GL_OES_EGL_image_external. r?nical

Comment 8

[Jamie Nicol [:jnicol]]

Attachment: Bug 1507074 - Use GL_OES_EGL_image_external to render external textures. r?nical

On android video data is provided to webrender via EGL external
images. Webrender currently relies on the extension
GL_OES_EGL_image_external_essl3 to render these images. Unfortunately,
however, there are a number of devices which support GLES 3, but do
not support GL_OES_EGL_image_external_essl3.

This means we that must use the older GL_OES_EGL_image_external
(non-essl3) extension to render video on such devices. This requires
shaders to be written in ESSL1 rather than ESSL3.

Most of webrender's shaders use too many modern GLSL features to be
compatible with ESSL1. For that reason, this patch implements ESSL1
compatible variants of just the composite and cs_scale shaders, as
they are both relatively simple.

In the happy path, videos are promoted to compositor surfaces and we
simply use this new composite shader variant. In other cases, this
patch makes it so that we use a render task to perform a copy of the
video frame using the cs_scale shader, then the output of that render
task can be rendered using the regular ESSL3 TEXTURE_2D variant of
whatever shader is required. The extra copy is unfortunate, but
rewriting every shader to be ESSL1 compatible is unrealistic.

Depends on D108907

Comment 9

[Jamie Nicol [:jnicol]]

Attachment: Bug 1507074 - Don't block webrender due to lack of GL_OES_EGL_image_external_essl3 support. r?aosmond

Thanks to the previous patch in this series we can now play video
using the non-essl3 GL_OES_EGL_image_external extension, therefore we
no longer require the essl3 version.

It is assumed that all android devices support
GL_OES_EGL_image_external (non-essl3). Even if that is not the case,
webrender is no worse off than layers in that regard.

Depends on D108908

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9121723e9849
Update glslopt-rs to add support for GL_OES_EGL_image_external. r=nical
https://hg.mozilla.org/integration/autoland/rev/07a571afefdc
Use GL_OES_EGL_image_external to render external textures. r=nical
https://hg.mozilla.org/integration/autoland/rev/e963fdd247a7
Don't block webrender due to lack of GL_OES_EGL_image_external_essl3 support. r=aosmond

Comment 11

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/9121723e9849
https://hg.mozilla.org/mozilla-central/rev/07a571afefdc
https://hg.mozilla.org/mozilla-central/rev/e963fdd247a7