1303443 - Update to ANGLE/2862

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Jeff Muizelaar [:jrmuizel]]

There have been some bug fixes in ANGLE that we should probably pick up.

    D3D11: Release SRV cache in Buffer11 destructor.
    D3D11: Detect driver version before workaround init.
    Add workaround for B5G6R5 format in Intel driver

Comment 1

[Peter Chang[:pchang]]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #0)
> There have been some bug fixes in ANGLE that we should probably pick up.
> 
>     D3D11: Release SRV cache in Buffer11 destructor.
>     D3D11: Detect driver version before workaround init.
>     Add workaround for B5G6R5 format in Intel driver

Jeff, I assume we need to include this for FF51 and FF50.

Comment 2

[Jeff Muizelaar [:jrmuizel]]

(In reply to Peter Chang[:pchang] from comment #1)
> (In reply to Jeff Muizelaar [:jrmuizel] from comment #0)
> > There have been some bug fixes in ANGLE that we should probably pick up.
> > 
> >     D3D11: Release SRV cache in Buffer11 destructor.
> >     D3D11: Detect driver version before workaround init.
> >     Add workaround for B5G6R5 format in Intel driver
> 
> Jeff, I assume we need to include this for FF51 and FF50.

It's not needed for sure, but it seems like we're better of taking these fixes in FF51 and FF50

Comment 3

[Peter Chang[:pchang]]

Ethan will upgrade the new ANGLE.

Comment 4

[Ethan Lin[:ethlin]]

I just finished angle update in my local. There are 2 changes in the angle/2862 that may effect gecko. :jrmuizel, do you have any idea?

1. SH_INIT_VARYINGS_WITHOUT_STATIC_USE was removed in this commit: https://github.com/google/angle/commit/7ebb97fc9a51aad770e07ee10de420b5fbc2a278, and gecko uses the parameter here: https://dxr.mozilla.org/mozilla-central/source/dom/canvas/WebGLShaderValidator.cpp#37
   I'm not sure if we still need this.

2. SH_EMULATE_BUILT_IN_FUNCTIONS was removed in this commit: https://github.com/google/angle/commit/e5bb72ff28773c42aae1a8e744882d194049b0a3, and gecko uses the parameter to do some workaround for driver bugs(https://dxr.mozilla.org/mozilla-central/source/dom/canvas/WebGLShaderValidator.cpp#79). Though the parameter might be not needed now.

Comment 5

[Ethan Lin[:ethlin]]

(In reply to Ethan Lin[:ethlin] from comment #4)
> I just finished angle update in my local. There are 2 changes in the
> angle/2862 that may effect gecko. :jrmuizel, do you have any idea?
> 
> 1. SH_INIT_VARYINGS_WITHOUT_STATIC_USE was removed in this commit:
> https://github.com/google/angle/commit/
> 7ebb97fc9a51aad770e07ee10de420b5fbc2a278, and gecko uses the parameter here:
> https://dxr.mozilla.org/mozilla-central/source/dom/canvas/
> WebGLShaderValidator.cpp#37
>    I'm not sure if we still need this.
> 

In the current ANGLE/2845 in gecko , ANGLE doesn't use SH_INIT_VARYINGS_WITHOUT_STATIC_USE to do any checking. So I think we don't need to set this parameter.

Comment 6

[Jeff Muizelaar [:jrmuizel]]

(In reply to Ethan Lin[:ethlin] from comment #4)
> 2. SH_EMULATE_BUILT_IN_FUNCTIONS was removed in this commit:
> https://github.com/google/angle/commit/
> e5bb72ff28773c42aae1a8e744882d194049b0a3, and gecko uses the parameter to do
> some workaround for driver
> bugs(https://dxr.mozilla.org/mozilla-central/source/dom/canvas/
> WebGLShaderValidator.cpp#79). Though the parameter might be not needed now.

Here's the bug for that removal: https://bugs.chromium.org/p/chromium/issues/detail?id=642227.
I don't really understand how removing this was safe. Perhaps you can ask the ANGLE people about it?

Comment 7

[Ethan Lin[:ethlin]]

try looks good.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1a855f92cc45

Comment 8

[Ethan Lin[:ethlin]]

(In reply to Ethan Lin[:ethlin] from comment #7)
> try looks good.
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=1a855f92cc45

OSX 10.10 has a failure: 
dom/canvas/test/webgl-conf/generated/test_2_conformance__glsl__misc__shaders-with-varyings.html | [unexpected link status] vertex shader with unused varying and fragment shader with used varying must succeed 

It may related to SH_INIT_VARYINGS_WITHOUT_STATIC_USE. I'll see how to fix it.

Comment 9

[Ethan Lin[:ethlin]]

(In reply to Ethan Lin[:ethlin] from comment #8)
> OSX 10.10 has a failure: 
> dom/canvas/test/webgl-conf/generated/test_2_conformance__glsl__misc__shaders-
> with-varyings.html | [unexpected link status] vertex shader with unused
> varying and fragment shader with used varying must succeed 
> 
> It may related to SH_INIT_VARYINGS_WITHOUT_STATIC_USE. I'll see how to fix
> it.

From my local test, gecko needs to add SH_INIT_OUTPUT_VARIABLES for MacOS to initialize variable. I'll send a try to double check

Comment 10

[Ethan Lin[:ethlin]]

Attachment: ANGLE/2862

Update to ANGLE/2862.

Comment 11

[Ethan Lin[:ethlin]]

Attachment: Change option parameters for angle update

I removed SH_INIT_VARYINGS_WITHOUT_STATIC_USE and SH_EMULATE_BUILT_IN_FUNCTIONS from gecko. SH_INIT_VARYINGS_WITHOUT_STATIC_USE is unused in the original ANGLE. SH_EMULATE_BUILT_IN_FUNCTIONS is for some Mac deives with OSX 10.6~10.8 driver which opengl version is 3.2 or before, but Nightly can only install on OSX 10.9 or later which opengl version is 3.3 or later.

Comment 12

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8797971 [details] [diff] [review]
ANGLE/2862

I'm not sure why it's changing all the file modes, but ok. I mean, they technically shouldn't be executable, so this is 'better', but it's still weird.

Comment 13

[Ethan Lin[:ethlin]]

(In reply to Jeff Gilbert [:jgilbert] from comment #12)
> Comment on attachment 8797971 [details] [diff] [review]
> ANGLE/2862
> 
> I'm not sure why it's changing all the file modes, but ok. I mean, they
> technically shouldn't be executable, so this is 'better', but it's still
> weird.

I didn't notice that! Per discussion with Morris, this may be because I did gyp_mozbuild on Mac and Morris did gyp_mozbuild on Windows.

Comment 14

[Ethan Lin[:ethlin]]

Attachment: ANGLE/2862 (carry r+: jgilbert)

Rebase the patch and remove the changes of file mode.

Comment 15

[Ethan Lin[:ethlin]]

Attachment: ANGLE/2862 (carry r+: jgilbert)

Update the correct patch.

Comment 16

[Ethan Lin[:ethlin]]

try looks good:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=11b701ccbdd6&filter-searchStr=gl

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/042d532e3d9e
Update ANGLE to chromium/2862. r=jgilbert

Comment 18

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for build bustage in WebGLShaderValidator.cpp after SH_INIT_VARYINGS_WITHOUT_STATIC_USE etc. got removed:

https://hg.mozilla.org/integration/mozilla-inbound/rev/1bd14dd39440eeef1e29dfc4d4427ad176828a88

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=042d532e3d9e553c89062ba169940602558755ff
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=38300962&repo=mozilla-inbound

 /builds/slave/m-in-lx-0000000000000000000000/build/src/dom/canvas/WebGLShaderValidator.cpp:37:19: error: 'SH_INIT_VARYINGS_WITHOUT_STATIC_USE' was not declared in this scope
/builds/slave/m-in-lx-0000000000000000000000/build/src/dom/canvas/WebGLShaderValidator.cpp:57:16: error: 'SH_EMULATE_BUILT_IN_FUNCTIONS' was not declared in this scope

These got removed here: https://hg.mozilla.org/integration/mozilla-inbound/rev/042d532e3d9e553c89062ba169940602558755ff#l2.137

Comment 19

[Ethan Lin[:ethlin]]

Please push attachment 8804528 [details] [diff] [review] and attachment 8797974 [details] [diff] [review] together.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

I'm going to push this as one folded-up patch because we generally try to ensure that everything that lands is individually buildable and passing tests so as to not break bisection.

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/57c5a60badc7
Update ANGLE to chromium/2862. r=jgilbert

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/57c5a60badc7

Comment 23

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8804528 [details] [diff] [review]
ANGLE/2862 (carry r+: jgilbert)

Approval Request Comment
[Feature/regressing bug #]: webgl2
[User impact if declined]:
[Describe test coverage new/current, TreeHerder]:
[Risks and why]: 
[String/UUID change made/needed]:

Comment 24

[Gerry Chang [:gchang]]

Comment on attachment 8804528 [details] [diff] [review]
ANGLE/2862 (carry r+: jgilbert)

WebGL 2 is the feature we want to ship in 51. Aurora51+.

Comment 25

[Iris Hsiao [:ihsiao]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/542efbbffdd3

Comment 26

[Iris Hsiao [:ihsiao]]

Sorry had to back out for bustages like https://treeherder.mozilla.org/logviewer.html#?job_id=4151297&repo=mozilla-aurora#L12814

Comment 27

[Ethan Lin[:ethlin]]

(In reply to Iris Hsiao [:ihsiao] from comment #26)
> Sorry had to back out for bustages like
> https://treeherder.mozilla.org/logviewer.html#?job_id=4151297&repo=mozilla-
> aurora#L12814

That's because the other patch should be uplifted too. I'll set the request comments.

Comment 28

[Ethan Lin[:ethlin]]

Comment on attachment 8797974 [details] [diff] [review]
Change option parameters for angle update

pproval Request Comment
[Feature/regressing bug #]: webgl2
[User impact if declined]:
[Describe test coverage new/current, TreeHerder]:tested locally and try server
[Risks and why]: Low. The patch just update the parameter for the new ANGLE.
[String/UUID change made/needed]: None.

Comment 29

[Ethan Lin[:ethlin]]

(In reply to Ethan Lin[:ethlin] from comment #28)
> Comment on attachment 8797974 [details] [diff] [review]
> Change option parameters for angle update
> 
> pproval Request Comment
> [Feature/regressing bug #]: webgl2
> [User impact if declined]:
> [Describe test coverage new/current, TreeHerder]:tested locally and try
> server
> [Risks and why]: Low. The patch just update the parameter for the new ANGLE.
> [String/UUID change made/needed]: None.

Please uplift attachment 8804528 [details] [diff] [review] and attachment 8797974 [details] [diff] [review] together.

Comment 30

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/57d2208d1fdc
https://hg.mozilla.org/releases/mozilla-aurora/rev/2d9b6132e7d7