MinGW needs to bundle d3dcompiler_47.dll

RESOLVED FIXED in Firefox -esr60

Status

defect
RESOLVED FIXED
Last year
6 months ago

People

(Reporter: tjr, Assigned: tjr)

Tracking

3 Branch
mozilla66
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr60 fixed, firefox66 fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

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
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...
Attachment #8974763 - Flags: review?(core-build-config-reviews)
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.
Version: Version 3 → 3 Branch

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.

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 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:

Attachment #8974763 - Attachment description: Bug 1460620 Have MinGW look for d3dcompiler_47.dll (so it will be packaged) → Bug 1460620 Have MinGW look for d3dcompiler_47.dll (so it will be packaged) (esr60)
Attachment #8974763 - Flags: approval-mozilla-esr60?

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

Attachment #9035144 - Flags: review?(core-build-config-reviews)
Attachment #9035144 - Attachment is patch: true

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?).

Attachment #9035144 - Flags: review?(core-build-config-reviews) → review+
Keywords: checkin-needed

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

Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66

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.

Attachment #8974763 - Flags: approval-mozilla-esr60? → approval-mozilla-esr60+
Blocks: 1518989
Blocks: 1519325
You need to log in before you can comment on or make changes to this bug.