Closed
Bug 1303443
Opened 8 years ago
Closed 8 years ago
Update to ANGLE/2862
Categories
(Core :: Graphics: CanvasWebGL, defect)
Core
Graphics: CanvasWebGL
Tracking
()
RESOLVED
FIXED
mozilla52
People
(Reporter: jrmuizel, Assigned: ethlin)
References
(Blocks 1 open bug)
Details
Attachments
(2 files, 2 obsolete files)
2.73 KB,
patch
|
jgilbert
:
review+
gchang
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
953.57 KB,
patch
|
gchang
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
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
Reporter | ||
Updated•8 years ago
|
Assignee: nobody → mtseng
Comment 1•8 years ago
|
||
(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.
Reporter | ||
Comment 2•8 years ago
|
||
(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
Assignee | ||
Comment 4•8 years ago
|
||
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.
Flags: needinfo?(jmuizelaar)
Assignee | ||
Comment 5•8 years ago
|
||
(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.
Reporter | ||
Comment 6•8 years ago
|
||
(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?
Flags: needinfo?(jmuizelaar)
Assignee | ||
Comment 7•8 years ago
|
||
try looks good. https://treeherder.mozilla.org/#/jobs?repo=try&revision=1a855f92cc45
Assignee | ||
Comment 8•8 years ago
|
||
(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.
Assignee | ||
Comment 9•8 years ago
|
||
(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
Assignee | ||
Comment 10•8 years ago
|
||
Update to ANGLE/2862.
Attachment #8797971 -
Flags: review?(jgilbert)
Assignee | ||
Comment 11•8 years ago
|
||
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.
Attachment #8797974 -
Flags: review?(jgilbert)
Updated•8 years ago
|
Attachment #8797974 -
Flags: review?(jgilbert) → review+
Comment 12•8 years ago
|
||
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.
Attachment #8797971 -
Flags: review?(jgilbert) → review+
Assignee | ||
Comment 13•8 years ago
|
||
(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.
Assignee | ||
Comment 14•8 years ago
|
||
Rebase the patch and remove the changes of file mode.
Attachment #8797971 -
Attachment is obsolete: true
Assignee | ||
Comment 15•8 years ago
|
||
Update the correct patch.
Attachment #8804524 -
Attachment is obsolete: true
Assignee | ||
Comment 16•8 years ago
|
||
try looks good: https://treeherder.mozilla.org/#/jobs?repo=try&revision=11b701ccbdd6&filter-searchStr=gl
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Comment 17•8 years ago
|
||
Pushed by cbook@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/042d532e3d9e Update ANGLE to chromium/2862. r=jgilbert
Keywords: checkin-needed
Comment 18•8 years ago
|
||
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
Flags: needinfo?(ethlin)
Assignee | ||
Comment 19•8 years ago
|
||
Please push attachment 8804528 [details] [diff] [review] and attachment 8797974 [details] [diff] [review] together.
Flags: needinfo?(ethlin)
Keywords: checkin-needed
Comment 20•8 years ago
|
||
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•8 years ago
|
||
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/57c5a60badc7 Update ANGLE to chromium/2862. r=jgilbert
Keywords: checkin-needed
Comment 22•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/57c5a60badc7
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox52:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Updated•8 years ago
|
status-firefox51:
--- → affected
Comment 23•8 years ago
|
||
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]:
Attachment #8804528 -
Flags: approval-mozilla-aurora?
Comment 24•7 years ago
|
||
Comment on attachment 8804528 [details] [diff] [review] ANGLE/2862 (carry r+: jgilbert) WebGL 2 is the feature we want to ship in 51. Aurora51+.
Attachment #8804528 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 26•7 years ago
|
||
Sorry had to back out for bustages like https://treeherder.mozilla.org/logviewer.html#?job_id=4151297&repo=mozilla-aurora#L12814
Flags: needinfo?(ethlin)
Assignee | ||
Comment 27•7 years ago
|
||
(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.
Flags: needinfo?(ethlin)
Assignee | ||
Comment 28•7 years ago
|
||
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.
Attachment #8797974 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 29•7 years ago
|
||
(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.
Updated•7 years ago
|
Attachment #8797974 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 30•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/57d2208d1fdc https://hg.mozilla.org/releases/mozilla-aurora/rev/2d9b6132e7d7
You need to log in
before you can comment on or make changes to this bug.
Description
•