Closed Bug 1371190 Opened 8 years ago Closed 8 years ago

Update ANGLE to chromium/3229 in Firefox 58

Categories

(Core :: Graphics: CanvasWebGL, enhancement, P5)

Unspecified
All
enhancement

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox57 --- unaffected
firefox58 --- fixed

People

(Reporter: cleu, Assigned: cleu)

References

(Blocks 1 open bug)

Details

(Keywords: feature, Whiteboard: [gfx-noted])

Attachments

(18 files, 1 obsolete file)

59 bytes, text/x-review-board-request
Details
59 bytes, text/x-review-board-request
jgilbert
: review+
Details
59 bytes, text/x-review-board-request
jgilbert
: review+
Details
59 bytes, text/x-review-board-request
jgilbert
: review+
Details
59 bytes, text/x-review-board-request
jgilbert
: review+
Details
59 bytes, text/x-review-board-request
jgilbert
: review+
Details
59 bytes, text/x-review-board-request
jgilbert
: review+
Details
59 bytes, text/x-review-board-request
jgilbert
: review+
Details
59 bytes, text/x-review-board-request
jgilbert
: review+
Details
59 bytes, text/x-review-board-request
jgilbert
: review+
Details
59 bytes, text/x-review-board-request
jgilbert
: review+
Details
59 bytes, text/x-review-board-request
jgilbert
: review+
Details
59 bytes, text/x-review-board-request
jgilbert
: review+
Details
59 bytes, text/x-review-board-request
jgilbert
: review+
Details
59 bytes, text/x-review-board-request
jgilbert
: review+
Details
59 bytes, text/x-review-board-request
jgilbert
: review+
Details
59 bytes, text/x-review-board-request
jgilbert
: review+
Details
59 bytes, text/x-review-board-request
cleu
: review+
Details
This version of ANGLE contains various bugfixes and some major update with some of its interfaces changed, which requires some modification on Gecko code. Although the recent update on ANGLE is primarily focused on vulkan support which is hardly useful for gecko, we still need to make our webgl backend be compatible with newer ANGLE soon or later.
I have already made a prototype to port ANGLE3118 to gecko with some workarounds, however, various conformance failures present in this version. Chromium has similar ANGLE version but exhibits almost no failures, I think we would need more effort to modify Gecko in order to pair with ANGLE with everything runs correctly. https://treeherder.mozilla.org/#/jobs?repo=try&revision=f459160242d33df728e0b663c6fa684baa51331e&selectedJob=105115934 BTW, I also tried updating the conformance suite to 2.0.1, the result remains same, so it must be some problem between ANGLE and us. https://treeherder.mozilla.org/#/jobs?repo=try&revision=66857f5ad4c96b9e071f473aa01099bd72e0f781
Keywords: feature
Priority: -- → P5
Whiteboard: [gfx-noted]
See Also: → 1371799
Blocks: 1371799
See Also: 1371799
Blocks: 1372203
Blocks: 1372196
It seems that updating ANGLE to 3118 does fix some conformance failures on Windows, but generate some new failures as well. I found that using ANGLE3118 without the local changes made by us applied would mitigate some of them. Maybe part of our local changes are not essential anymore since ANGLE has been updated a lot.
https://github.com/subsevenx2001/ANGLE_gyp2mozbuild I created a git repo to track scripts and patches needed to build gecko.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d3b496df4b25084a83d16f38ee9d710ba40bd9b3&selectedJob=109141495 OK, there are quite a few conformance failures to be fixed before updating. But I found that the failure in mochitest-gl1 conformance2/textures/canvas/tex-2d-r11f_g11f_b10f-rgb-float.html is good in local machine. I will investigate other failures especially crashes.
BTW, I'm now reporting some of our local change to Google so they can integrate the patches into their upstream. I will evaluate our changeset whether they should land or not. I started with the most trivial one. https://bugs.chromium.org/p/angleproject/issues/detail?id=2082#c1
Depends on: 1377527
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5d2af53917401a7d046e5da067dd4df792901851&selectedJob=112950592 I tried to rebase patch in bug 1347866 manually on ANGLE3118 but it results in even more conformance failures. Maybe the patch is not compatible with newer ANGLE anymore.
Blocks: 1354197
The patch is in bug 1325741, sorry for the typo.
Assignee: nobody → cleu
Summary: Update ANGLE to chromium/3118 → Update ANGLE to chromium/3173
Attachment #8894411 - Flags: review?(jgilbert)
Attachment #8894412 - Flags: review?(jgilbert)
Attachment #8894413 - Flags: review?(jgilbert)
Attachment #8894414 - Flags: review?(jgilbert)
Attachment #8894415 - Flags: review?(jgilbert)
Attachment #8894416 - Flags: review?(jgilbert)
Attachment #8894417 - Flags: review?(jgilbert)
Attachment #8894418 - Flags: review?(jgilbert)
Attachment #8894419 - Flags: review?(jgilbert)
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0fd3aa7800147f0e4a217a4b9618f44d28e62315&selectedJob=120864096 After updating ANGLE to 3173 with correctly rebased patches, there are only one perma-orange. The failure test is dom/canvas/test/webgl-conf/generated/test_conformance__context__context-attribute-preserve-drawing-buffer.html | Did not render ok with preserveDrawingBuffer true.
OK, I found the cause. There is a new framebuffer format RGB10_A2 added into ANGLE recently, and ANGLE will choose it as its colorbuffer format if our device caps support when we are creating ANGLE context, this leads to inconsistency between ANGLE(RGB10_A2) and us(BGRA8888) so we fail the validation in glBlitFramebuffer in ANGLE when we are trying to copy framebuffer and being unable to draw correct result. I think we should add RGB10_A2 support in our WebGLContext so we can make sure our format will match with ANGLE.
Depends on: 1390397
(In reply to Michael Leu[:Lenzak](UTC+8) from comment #25) > OK, I found the cause. > > There is a new framebuffer format RGB10_A2 added into ANGLE recently, and > ANGLE will choose it as its colorbuffer format if our device caps support > when we are creating ANGLE context, this leads to inconsistency between > ANGLE(RGB10_A2) and us(BGRA8888) so we fail the validation in > glBlitFramebuffer in ANGLE when we are trying to copy framebuffer and being > unable to draw correct result. > > I think we should add RGB10_A2 support in our WebGLContext so we can make > sure our format will match with ANGLE. Jeff, I just discussed with Michael. So far he only got one conformance failure from WebGL1 after upgrading new ANGLE 3173. Any concern if we disable that WebGL failure first? And we can keep following the failure in bug 1390397.
Flags: needinfo?(jgilbert)
As I discussed with Michael, he will try to pass alpha as true to get RGBA8888 format from ANGLE in bug 1390397.
Flags: needinfo?(jgilbert)
Attachment #8877388 - Flags: review?(jgilbert)
Attachment #8877387 - Flags: review?(jgilbert)
Attachment #8877389 - Flags: review?(jgilbert)
Attachment #8894410 - Flags: review?(jgilbert)
Attachment #8894413 - Flags: review?(jgilbert)
Attachment #8894411 - Flags: review?(jgilbert)
Attachment #8894412 - Flags: review?(jgilbert)
Attachment #8894414 - Flags: review?(jgilbert)
Attachment #8894415 - Flags: review?(jgilbert)
Attachment #8897688 - Flags: review?(jgilbert)
Attachment #8894416 - Flags: review?(jgilbert)
Attachment #8894417 - Flags: review?(jgilbert)
Attachment #8894418 - Flags: review?(jgilbert)
Attachment #8894419 - Flags: review?(jgilbert)
Oops, looks like Windows 10 has some problems. Almost all failures are caused by context create failure, I suspect it is related to the hardware because there was no context issue in previous try push.
Comment on attachment 8877387 [details] Bug 1371190 - Part2: Update ANGLE to chromium/3229 r=upstream
Attachment #8877387 - Flags: review?(jgilbert)
Comment on attachment 8894414 [details] Bug 1371190 - Part6: Update array-length-side-effects.html conformance test https://reviewboard.mozilla.org/r/165594/#review177262
Attachment #8894414 - Flags: review?(jgilbert) → review+
Attachment #8894411 - Flags: review?(jgilbert) → review+
Attachment #8894412 - Flags: review?(jgilbert) → review+
Comment on attachment 8894416 [details] Bug 1371190 - (Rebase)Bug 1370865 - Suppress more MSVC warnings in gfx/angle. https://reviewboard.mozilla.org/r/165598/#review177268
Attachment #8894416 - Flags: review?(jgilbert) → review+
Comment on attachment 8894417 [details] Bug 1371190 - (Rebase)Bug 1373525 - gfx/angle: Suppress -Wimplicit-fallthrough warnings in third-party code. https://reviewboard.mozilla.org/r/165600/#review177272
Attachment #8894417 - Flags: review?(jgilbert) → review+
Attachment #8894410 - Flags: review?(jgilbert) → review+
Attachment #8894418 - Flags: review?(jgilbert) → review+
Comment on attachment 8894413 [details] Bug 1371190 - Part5: Force enable alpha channel to prevent ANGLE from using incompatible backbuffer format https://reviewboard.mozilla.org/r/165592/#review177278 ::: gfx/gl/GLContextProviderEGL.cpp:1014 (Diff revision 2) > + if (sEGLLibrary.IsANGLE()) { > + // Force enable alpha channel to prevent ANGLE from > + // creating pbuffers with incompatible internal format > + minBackbufferCaps.alpha = true; > + } Ask for it where WebGL requests an ANGLE EGL context creation.
Attachment #8894413 - Flags: review?(jgilbert) → review-
Attachment #8877389 - Flags: review?(jgilbert) → review+
Comment on attachment 8897688 [details] Bug 1371190 - (Rebase)Bug 1366425 - Avoid losing context on out of memory error for ANGLE https://reviewboard.mozilla.org/r/168962/#review177282
Attachment #8897688 - Flags: review?(jgilbert) → review+
Comment on attachment 8894415 [details] Bug 1371190 - (Rebase)Bug 1364169 - on ANGLE context creation asks for robustness but does not get it https://reviewboard.mozilla.org/r/165596/#review177284
Attachment #8894415 - Flags: review?(jgilbert) → review+
Comment on attachment 8877388 [details] Bug 1371190 - Part1: Modify gecko to be compatible to newer ANGLE https://reviewboard.mozilla.org/r/148770/#review177288
Attachment #8877388 - Flags: review?(jgilbert) → review+
Let's hold off landing this until after bug 1322746. We'll need to add a rebase for the patch from that bug, too.
Depends on: 1322746
Attachment #8877387 - Flags: review?(jgilbert)
Attachment #8894413 - Flags: review?(jgilbert)
Attachment #8877387 - Flags: review?(jgilbert)
Attachment #8877387 - Flags: review?(jgilbert)
https://treeherder.mozilla.org/#/jobs?repo=try&revision=946c67af197a&selectedJob=126700006 The reason why we failed on Windows 10 is because the hardware configuration is blacklisted so we cannot create a context. 10:41:02 INFO - GECKO(2152) | JavaScript warning: http://mochi.test:8888/tests/dom/canvas/test/webgl-conf/checkout/js/webgl-test-utils.js, line 1443: Error: WebGL warning: Refused to create WebGL2 context because of blacklist entry: FEATURE_FAILURE_UNKNOWN_DEVICE_VENDOR 10:41:02 INFO - GECKO(2152) | JavaScript warning: http://mochi.test:8888/tests/dom/canvas/test/webgl-conf/checkout/js/webgl-test-utils.js, line 1443: Error: WebGL warning: Failed to create WebGL context: WebGL creation failed: 10:41:02 INFO - GECKO(2152) | * Refused to create WebGL2 context because of blacklist entry: FEATURE_FAILURE_UNKNOWN_DEVICE_VENDOR 10:41:02 INFO - TEST-INFO | started process screenshot 10:41:03 INFO - TEST-INFO | screenshot: exit 0
Blocks: 1354997
I've rebased the changesets.
Attachment #8897688 - Attachment is obsolete: true
Now Chromium uses ANGLE branch 3229, we should move to that.
Summary: Update ANGLE to chromium/3173 → Update ANGLE to chromium/3229
https://treeherder.mozilla.org/#/jobs?repo=try&revision=577b7f85266f2252432bf7148990b5fd59aa0b45&selectedJob=134327820 https://treeherder.mozilla.org/#/jobs?repo=try&revision=3377c66273f943a87bc75ef280596ba47d8c0c21&selectedJob=134332182&group_state=expanded Here is the try result, the failures in gl1 is actually a spec change so it will pass in newer conformance suite. The most worrying part is Windows7, it seems that it has difficulty fetching ANGLE context.
Hmm... Looks better after update mochitest-errata and outdated conformance tests. https://treeherder.mozilla.org/#/jobs?repo=try&revision=6077bd5b1fa64d414d0279c8013b218d8861b6b8 Now for Windows 7, it seems that they cannot get context because they cannot find a suitable config when calling eglChooseConfig. I will compare the behavior between Win7 and Win10 to see if there is anything we can do.
Comment on attachment 8894413 [details] Bug 1371190 - Part5: Force enable alpha channel to prevent ANGLE from using incompatible backbuffer format https://reviewboard.mozilla.org/r/165592/#review190772
Attachment #8894413 - Flags: review?(jgilbert) → review+
Attachment #8894419 - Flags: review?(jgilbert) → review+
Comment on attachment 8904424 [details] Bug 1371190 - (Rebase)Bug 1322746 - Add general ID3D11Texture2D to EGLStream support to ANGLE. https://reviewboard.mozilla.org/r/176254/#review190848
Attachment #8904424 - Flags: review?(jgilbert) → review+
OK, I found why newer ANGLE breaks WebGL2 support on Windows7. They added D3D_FEATURE_LEVEL_11_1 flag into their FeatureLevelList when they query D3D11Device which is not supported by Windows 7 in most cases, this leads to a device creation fail and make the whole context fail back to D3D9 so we cannot create WebGL2 context. The solution is to determine the Windows version before adding this flag, omitting it if our Windows version is prior to 8.
Attachment #8914261 - Flags: review?(jgilbert)
Attachment #8914633 - Flags: review?(jgilbert)
Comment on attachment 8914261 [details] Bug 1371190 - (Rebase)Bug 1366425 - Avoid losing context on out of memory error for ANGLE https://reviewboard.mozilla.org/r/185580/#review191238 ::: gfx/angle/src/libANGLE/Context.cpp (Diff revision 1) > } > > void Context::initWorkarounds() > { > - // Apply back-end workarounds. > - mImplementation->applyNativeWorkarounds(&mWorkarounds); I think we still need this line. ::: gfx/angle/src/libANGLE/Context.cpp (Diff revision 1) > - // Apply back-end workarounds. > - mImplementation->applyNativeWorkarounds(&mWorkarounds); > - > - // Lose the context upon out of memory error if the application is > - // expecting to watch for those events. > - mWorkarounds.loseContextOnOutOfMemory = (mResetStrategy == GL_LOSE_CONTEXT_ON_RESET_EXT); Just set this to false.
Comment on attachment 8914633 [details] Bug 1371190 - Retry D3D11CreateDevice with feature level 11_0 if 11_1 is not supported https://reviewboard.mozilla.org/r/185962/#review191240 ::: gfx/angle/src/libANGLE/renderer/d3d/d3d11/Renderer11.cpp:496 (Diff revision 1) > if (requestedMinorVersion == EGL_DONT_CARE || requestedMinorVersion >= 1) > { > - // This could potentially lead to failed context creation if done on a system > - // without the platform update which installs DXGI 1.2. Currently, for Chrome users > - // D3D11 contexts are only created if the platform update is available, so this > - // should not cause any issues. > + // We should avoid using D3D_FEATURE_LEVEL_11_1 on Windows > + // prior to Windows 8 since they usually don't support > + // D3D11.1, leading to a D3D11Device creation fail. > + if (IsWindows8OrGreater()) This doesn't seem like the right fix.
Attachment #8914633 - Flags: review?(jgilbert) → review-
Comment on attachment 8914633 [details] Bug 1371190 - Retry D3D11CreateDevice with feature level 11_0 if 11_1 is not supported https://reviewboard.mozilla.org/r/185962/#review191244 ::: gfx/angle/src/libANGLE/renderer/d3d/d3d11/Renderer11.cpp:496 (Diff revision 1) > if (requestedMinorVersion == EGL_DONT_CARE || requestedMinorVersion >= 1) > { > - // This could potentially lead to failed context creation if done on a system > - // without the platform update which installs DXGI 1.2. Currently, for Chrome users > - // D3D11 contexts are only created if the platform update is available, so this > - // should not cause any issues. > + // We should avoid using D3D_FEATURE_LEVEL_11_1 on Windows > + // prior to Windows 8 since they usually don't support > + // D3D11.1, leading to a D3D11Device creation fail. > + if (IsWindows8OrGreater()) Please identify the code that's actually failing to create the device. By default d3d will traverse the requested feature levels until it finds one that does work, so adding higher feature levels should not cause device creation failure.
Comment on attachment 8914261 [details] Bug 1371190 - (Rebase)Bug 1366425 - Avoid losing context on out of memory error for ANGLE https://reviewboard.mozilla.org/r/185580/#review191590
Attachment #8914261 - Flags: review?(jgilbert) → review+
Comment on attachment 8914633 [details] Bug 1371190 - Retry D3D11CreateDevice with feature level 11_0 if 11_1 is not supported https://reviewboard.mozilla.org/r/185962/#review191592 ::: gfx/angle/src/libANGLE/renderer/d3d/d3d11/Renderer11.cpp:780 (Diff revision 3) > D3D11_CREATE_DEVICE_DEBUG, mAvailableFeatureLevels.data(), > static_cast<unsigned int>(mAvailableFeatureLevels.size()), > D3D11_SDK_VERSION, &mDevice, > &(mRenderer11DeviceCaps.featureLevel), &mDeviceContext); > > + if (result == E_INVALIDARG) Same here: Only try again if it might be 11_1's fault. ::: gfx/angle/src/libANGLE/renderer/d3d/d3d11/Renderer11.cpp:785 (Diff revision 3) > + if (result == E_INVALIDARG) > + { > + // In some older Windows platform, D3D11.1 is not supported which returns E_INVALIDARG > + // so we omit the 11.1 feature level flag and try again > + result = D3D11CreateDevice(nullptr, mRequestedDriverType, nullptr, > + D3D11_CREATE_DEVICE_DEBUG, &mAvailableFeatureLevels.data()[1], Just do mAvailableFeatureLevels.data()+1 ::: gfx/angle/src/libANGLE/renderer/d3d/d3d11/Renderer11.cpp:807 (Diff revision 3) > result = D3D11CreateDevice( > nullptr, mRequestedDriverType, nullptr, 0, mAvailableFeatureLevels.data(), > static_cast<unsigned int>(mAvailableFeatureLevels.size()), D3D11_SDK_VERSION, > &mDevice, &(mRenderer11DeviceCaps.featureLevel), &mDeviceContext); > > + if (result == E_INVALIDARG) { if (result == E_INVALIDARG && mAvailableFeatureLevels[0] == 11_1) We don't want to do this if 11_1 couldn't be the reason for this failure.
Comment on attachment 8914633 [details] Bug 1371190 - Retry D3D11CreateDevice with feature level 11_0 if 11_1 is not supported https://reviewboard.mozilla.org/r/185962/#review192064
Attachment #8914633 - Flags: review?(jgilbert) → review+
Update ANGLE to chromium/3229 just took the entire code package from Google's github repository unmodified, so r=upstream
Keywords: checkin-needed
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/472a419a8d65 Part 1: Modify gecko to be compatible to newer ANGLE. r=jgilbert https://hg.mozilla.org/integration/mozilla-inbound/rev/88ddf39c0d79 Part 2: Update ANGLE to chromium/3229. r=jgilbert https://hg.mozilla.org/integration/mozilla-inbound/rev/b869b8355ddd Part 3: Fix moz.build. r=jgilbert https://hg.mozilla.org/integration/mozilla-inbound/rev/b8930cdc9357 Part 4: Update mochitest-errata. r=jgilbert https://hg.mozilla.org/integration/mozilla-inbound/rev/85a3dd1433e2 Part 5: Force enable alpha channel to prevent ANGLE from using incompatible backbuffer format. r=jgilbert https://hg.mozilla.org/integration/mozilla-inbound/rev/c20b4b61163f Part 6: Update array-length-side-effects.html conformance test. r=jgilbert https://hg.mozilla.org/integration/mozilla-inbound/rev/9aa8f8c7fe61 Part 7: Re-apply various patches to the in-tree copy of ANGLE. r=jgilbert https://hg.mozilla.org/integration/mozilla-inbound/rev/3e5a825eac7d Part 8: Retry D3D11CreateDevice with feature level 11_0 if 11_1 is not supported. r=jgilbert
Keywords: checkin-needed
Backed out for build bustage, at least on Windows Static: https://hg.mozilla.org/integration/mozilla-inbound/rev/cf27d9749603dd9c8344ccb50dad0e40ba4dc8f8 https://hg.mozilla.org/integration/mozilla-inbound/rev/108ee2f14c2f13e1ce31a69df858ee450a1fe0ea https://hg.mozilla.org/integration/mozilla-inbound/rev/ded72b49255551c447fcfb67c04638c91adea0c8 https://hg.mozilla.org/integration/mozilla-inbound/rev/912dbc6ded47bd631b4972a258ecaa35d925171d https://hg.mozilla.org/integration/mozilla-inbound/rev/539031137eae49995696b4cc11f81c19123cc882 https://hg.mozilla.org/integration/mozilla-inbound/rev/5469c7544f1280b3b4cf665af3ed476e55f6f1c0 https://hg.mozilla.org/integration/mozilla-inbound/rev/cea32e55bad6819dcd634bd426ddcec9af6fd32b https://hg.mozilla.org/integration/mozilla-inbound/rev/62686b7f1f2a37c78da85da9ad0ab57f876dc0a9 Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=3e5a825eac7d6b07cc6c56a414ab4087030d8ef7&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=135341246&repo=mozilla-inbound 13:13:51 INFO - In file included from z:/build/build/src/gfx/angle/src\libANGLE/angletypes.h:12: 13:13:51 INFO - z:/build/build/src/gfx/angle/src\common/bitset_utils.h(38,9): error: bad implicit conversion operator for 'Reference' 13:13:51 INFO - operator bool() const { return mParent->test(mBit); } 13:13:51 INFO - ^ 13:13:51 INFO - z:/build/build/src/gfx/angle/src\common/bitset_utils.h(38,9): note: consider adding the explicit keyword to 'operator bool' 13:13:51 INFO - In file included from z:/build/build/src/gfx/angle/src/libANGLE/Display.cpp:26: 13:13:51 INFO - In file included from z:/build/build/src/gfx/angle/src\libANGLE/Context.h:21: 13:13:51 INFO - In file included from z:/build/build/src/gfx/angle/src\libANGLE/ContextState.h:14: 13:13:51 INFO - In file included from z:/build/build/src/gfx/angle/src\libANGLE/State.h:27: 13:13:51 INFO - z:/build/build/src/gfx/angle/src\libANGLE/VertexAttribute.h(26,14): error: Move constructors may not be marked explicit 13:13:51 INFO - explicit VertexBinding(VertexBinding &&binding); 13:13:51 INFO - ^ 13:13:51 INFO - z:/build/build/src/gfx/angle/src\libANGLE/VertexAttribute.h(55,14): error: Move constructors may not be marked explicit 13:13:51 INFO - explicit VertexAttribute(VertexAttribute &&attrib); 13:13:51 INFO - ^ 13:13:51 INFO - 3 errors generated. 13:13:51 INFO - z:/build/build/src/config/rules.mk:1075: recipe for target 'Display.obj' failed 13:13:51 INFO - mozmake.EXE[5]: *** [Display.obj] Error 1
Flags: needinfo?(cleu)
Attachment #8916515 - Flags: review?(jgilbert)
Added a patch to make st-an happier, but I really think st-an should be skipped on third-party code.
Flags: needinfo?(cleu)
Ehsan please advise: comment 171
Flags: needinfo?(ehsan)
> <dmajor> jgilbert: Search "angle" in https://dxr.mozilla.org/mozilla-central/source/build/clang-plugin/Utils.h, it is already opted out of a few things, are you wanting to opt it out of more? > <jgilbert> dmajor, an angle update got backed out for static analysis opinions > <jgilbert> dmajor, https://bugzilla.mozilla.org/show_bug.cgi?id=1371190#c171 > <dmajor> jgilbert: you can probably fix the first error by adding angle to https://dxr.mozilla.org/mozilla-central/source/build/clang-plugin/Utils.h#211 > <jgilbert> dmajor, is there a reason we don't just opt it out of everything? > <dmajor> I neither created the policy nor wrote the code but my guess is that it was deemed a bigger hammer than necessary > <dmajor> you can take it up with ehsan and mystor :) > <jgilbert> sigh, ok, thanks > <mystor> jgilbert: what's the problem? > <jgilbert> mystor, static analysis errors in third-party code > <jgilbert> mystor, specifically https://bugzilla.mozilla.org/show_bug.cgi?id=1371190#c171 > <mystor> jgilbert: yeah. Ideally we want to unify the third party code logic at some point. Right now each analysis has its own opt out. > <mystor> Sorry for the mess :-(. Ideally at some point being in third party paths will opt you out of all of our static analyses. > <jgilbert> mystor, can you toss me a patch for this? Or confirm dmajor's guess as correct? > <jgilbert> (for doing this individual opt-out) > <mystor> jgilbert: yeah, it should work. Not at a computer right now. > <jgilbert> ok, thanks > <dmajor> There's a later error about move constructors further down, which doesn't have any opt outs right now, so you'd have to cook your own thing into this unfortunately https://dxr.mozilla.org/mozilla-central/source/build/clang-plugin/NoExplicitMoveConstructorChecker.cpp > <dmajor> Or worst case get permission to comment it out and let someone else fix it up Let's opt ANGLE out.
Flags: needinfo?(ehsan)
Comment on attachment 8916515 [details] Bug 1371190 - Fix st-an fail https://reviewboard.mozilla.org/r/187642/#review192962 I don't care how we get this working, honestly. This is small enough of a difference that we can just take this instead of pursuing the opt-out.
Attachment #8916515 - Flags: review?(jgilbert) → review+
Me either, we are lucky enough to have so little things to change. But we still need ANGLE being skipped in st-an since it's third-party code, we won't know if there are some "fatal change" or not. Let's land it first.
Keywords: checkin-needed
Summary: Update ANGLE to chromium/3229 → Update ANGLE to chromium/3229 in Firefox 58
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/6db4543e49c7 Part 1: Modify gecko to be compatible to newer ANGLE. r=jgilbert https://hg.mozilla.org/integration/mozilla-inbound/rev/fa4b8dca11bb Part 2: Update ANGLE to chromium/3229. r=jgilbert https://hg.mozilla.org/integration/mozilla-inbound/rev/3101f71ea30f Part 3: Fix moz.build. r=jgilbert https://hg.mozilla.org/integration/mozilla-inbound/rev/5cf9988a5a11 Part 4: Update mochitest-errata. r=jgilbert https://hg.mozilla.org/integration/mozilla-inbound/rev/162ddc0d22f6 Part 5: Force enable alpha channel to prevent ANGLE from using incompatible backbuffer format. r=jgilbert https://hg.mozilla.org/integration/mozilla-inbound/rev/bafc5511aee8 Part 6: Update array-length-side-effects.html conformance test. r=jgilbert https://hg.mozilla.org/integration/mozilla-inbound/rev/ed85185ff27e Part 7: Re-apply various patches to the in-tree copy of ANGLE. r=jgilbert https://hg.mozilla.org/integration/mozilla-inbound/rev/9f5ac1ae7001 Part 8: Retry D3D11CreateDevice with feature level 11_0 if 11_1 is not supported. r=jgilbert https://hg.mozilla.org/integration/mozilla-inbound/rev/8731edf699f3 Part 9: Changes to make the static analysis job happy. r=jgilbert
Keywords: checkin-needed
Comment on attachment 8917538 [details] Bug 1371190 - Enable SH_INITIALIZE_UNINITIALIZED_LOCALS. - https://reviewboard.mozilla.org/r/188500/#review193836 LGTM, do we need to open another bug to land it?
Attachment #8917538 - Flags: review?(cleu) → review+
(In reply to Michael Leu[:Lenzak](UTC+8) from comment #182) > Comment on attachment 8917538 [details] > Bug 1371190 - Enable SH_INITIALIZE_UNINITIALIZED_LOCALS. - > > https://reviewboard.mozilla.org/r/188500/#review193836 > > LGTM, do we need to open another bug to land it? Oops, I used the wrong bug number!
Depends on: 1409302
Depends on: 1410304
Blocks: 1395107
Depends on: 1448534
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: