1460620 - MinGW needs to bundle d3dcompiler_47.dll

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Way back when, in Bug 1314979 and Bug 1359552, we changed the Angle configuration in old-configure to not look for the Windows SDK. That's alright, but it also undefined MOZ_D3DCOMPILER_VISTA_DLL_PATH (which is d3dcompiler_47.dll)

We need d3dcompiler_47.dll for Firefox to run, otherwise it will produce the following error:

> WARN: initializeD3DDevice(799): Failed creating Debug D3D11 device - falling back to release runtime.
> Initializing context 0000000016913fd0 surface 0000000016783e80 on display 0000000012cef660
> [gl:0000000012883390] void mozilla::gl::GLContext::fLinkProgram(GLuint): Generated unexpected GL_OUT_OF_MEMORY error. (0x0505)
> ERR: ensureInitialized(141): D3D compiler module not found.
> Hit MOZ_CRASH(Unexpected error with MOZ_GL_DEBUG_ABORT_ON_ERROR. (Run with MOZ_GL_DEBUG_ABORT_ON_ERROR=0 to disable)) at /builds/worker/workspace/build/src/gfx/gl/GLContext.cpp:3033

For normal builds of Firefox, we bundle d3dcompiler_47.dll, copying it from the SDK:
https://searchfox.org/mozilla-central/rev/eb6c5214a63e20a3fff455e92c876287a8d2e188/gfx/angle/Makefile.in#9

For MinGW, we don't have the SDK but we _do_ have the dll, because we need it for fxc2:
https://github.com/mozilla/fxc2

Comment 1

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Attachment: Bug 1460620 Have MinGW look for d3dcompiler_47.dll (so it will be packaged) (esr60)

Review commit: https://reviewboard.mozilla.org/r/243160/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/243160/

Comment 2

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Comment on attachment 8974763 [details]
Bug 1460620 Have MinGW look for d3dcompiler_47.dll (so it will be packaged) (esr60)

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/243160/diff/1-2/

Comment 3

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Hrm. It seems this commit it not sufficient. While the dll does wind up in target.zip; on TC we still hit this error:
https://searchfox.org/mozilla-central/rev/3f17a234769d25fca5144ebb8abc8e1cb3c56c16/gfx/angle/checkout/src/libANGLE/renderer/d3d/HLSLCompiler.cpp#141

If I uncomment ANGLE_PRELOADED_D3DCOMPILER_MODULE_NAMES so it is present; it succeeds. I think I need to add logging to see what is happening on TC and locally...

Comment 4

[Michael Shal [:mshal]]

Just clearing out of the queue for now based on #c3. If you still wanted to review just this part, feel free to re-add it to the queue.

Comment 5

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

A cypherpunk over at https://trac.torproject.org/projects/tor/ticket/28874#comment:12 pointed out this inconsistency: in the DirectX SDK we look for _47: https://github.com/apitrace/dxsdk/blob/master/Include/d3dcompiler.h#L18 but in MinGW we look for _43: https://sourceforge.net/p/mingw-w64/mingw-w64/ci/master/tree/mingw-w64-headers/direct-x/include/d3dcompiler.h#l38

This is almost certainly the cause of this problem.

Comment 6

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Here's a try run with this patch:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7bfb8a8eb9f07e4de22a05730b3715c6d9acc706
Notice all the gl failures.

Here's a try run with this patch and updating MinGW to look for _47.dll:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2d604bfba609fec964d8f1ab0ca1ba80b1ad38c8&selectedJob=220674737
All the gl tests are now green.

Horrah!

Comment 7

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Comment on attachment 8974763 [details]
Bug 1460620 Have MinGW look for d3dcompiler_47.dll (so it will be packaged) (esr60)

[ESR Uplift Approval Request]

If this is not a sec:{high,crit} bug, please state case for ESR consideration: Needed for MinGW Build

User impact if declined: We will be one step further from being able to run tests for mingw-gcc builds in TaskCluster

Fix Landed on Version:

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): This is a toolchain-only patch that bundles a new file in the resulting target.zip

String or UUID changes made by this patch:

Comment 8

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Attachment: Bug 1460620 Have MinGW look for d3dcompiler_47.dll (so it will be packaged) r?Build

The original patch applies cleanly to ESR60, so I left it there and marked it for approval. But I need review on the patch still; here is one that applies to central.

Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d5ab209f56c145273ac2c666cce2b826a08fd059

Comment 9

[Nathan Froyd [:froydnj]]

Comment on attachment 9035144 [details] [diff] [review]
Bug 1460620 Have MinGW look for d3dcompiler_47.dll (so it will be packaged) r?Build

Review of attachment 9035144 [details] [diff] [review]:

Splinter is yelling at me that these are windows line-ending patches, please fix that prior to landing (maybe hg does that automagically?).

Comment 10

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Attachment: Bug 1460620 - Have MinGW look for d3dcompiler_47.dll (so it will be packaged) r=froydnj

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/763f90cc2fac
Have MinGW look for d3dcompiler_47.dll (so it will be packaged) r=froydnj

Comment 12

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/763f90cc2fac

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8974763 [details]
Bug 1460620 Have MinGW look for d3dcompiler_47.dll (so it will be packaged) (esr60)

NPOTB for official Firefox builds. Makes MinGW builds easier to test by packaging necessary DLLs with them. Approved for 60.5.0esr.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr60/rev/6e4da7e63d35