Closed
Bug 1076743
Opened 10 years ago
Closed 9 years ago
Fixed angle compilation with mingw.
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
mozilla35
People
(Reporter: jacek, Assigned: jacek)
Details
Attachments
(3 files)
3.14 KB,
patch
|
gps
:
review+
|
Details | Diff | Splinter Review |
20.49 KB,
patch
|
Details | Diff | Splinter Review | |
1.62 KB,
patch
|
jrmuizel
:
review+
|
Details | Diff | Splinter Review |
Upstream fixes are already reviewed and on their way to angle: https://chromium-review.googlesource.com/#/c/220761/ On Mozilla side, moz.build files also need a few fixes: - EXTRA_DSO_LDOPTS in libEGL/moz.build is not needed, because it already has USE_LIBS - Use OS_LIBS instead of EXTRA_DSO_LDOPTS in libGLESv2/moz.build - ANGLE_PRELOADED_D3DCOMPILER_MODULE_NAMES is problematic when passed to windres (resource compiler, rc.exe replacement), which doesn't seem to support " and ' quotes combination correctly. Since renderer/d3d/HLSLCompiler.cpp is the only file needing the define, I just limited it to it. Here is try push (without ANGLE_PRELOADED_D3DCOMPILER_MODULE_NAMES, which I added later): https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=6601a4ed2d3f
Attachment #8498747 -
Flags: review?(jmuizelaar)
Assignee | ||
Comment 1•10 years ago
|
||
This is upstream part. AFAIU upstream approval is enough for the review, please let me know if I'm wrong.
Assignee | ||
Comment 2•10 years ago
|
||
Upstream part has been committed to ANGLE.
Comment 3•10 years ago
|
||
Comment on attachment 8498747 [details] [diff] [review] moz.build part Review of attachment 8498747 [details] [diff] [review]: ----------------------------------------------------------------- It's probably better to have a build peer review this.
Attachment #8498747 -
Flags: review?(jmuizelaar) → review?(gps)
Comment 4•10 years ago
|
||
Comment on attachment 8498747 [details] [diff] [review] moz.build part Review of attachment 8498747 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/angle/src/libGLESv2/moz.build @@ +224,5 @@ > > RCFILE = SRCDIR + '/libGLESv2.rc' > DEFFILE = SRCDIR + '/libGLESv2.def' > > +SOURCES['renderer/d3d/HLSLCompiler.cpp'].flags += ['-DANGLE_PRELOADED_D3DCOMPILER_MODULE_NAMES=\'{ TEXT("d3dcompiler_47.dll"), TEXT("d3dcompiler_46.dll"), TEXT("d3dcompiler_43.dll") }\''] This block is weird: it appears to be referencing a .dll on all platforms, not just Windows. But it existed before, so I guess it is OK. I appreciate you moving this to a per-source cflag as opposed to global.
Attachment #8498747 -
Flags: review?(gps) → review+
Assignee | ||
Comment 5•10 years ago
|
||
Thanks for the review. The whole dir is Windows only, so I think it's fine. https://hg.mozilla.org/integration/mozilla-inbound/rev/0d22e3162123 https://hg.mozilla.org/integration/mozilla-inbound/rev/80bf7cee307e
Comment 6•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/80bf7cee307e https://hg.mozilla.org/mozilla-central/rev/0d22e3162123
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Comment 7•10 years ago
|
||
(In reply to Jacek Caban from comment #1) > Created attachment 8498749 [details] [diff] [review] > upstream part > > This is upstream part. AFAIU upstream approval is enough for the review, > please let me know if I'm wrong. Changes to ANGLE should go through our updating process: http://hg.mozilla.org/mozilla-central/file/0c8ae792f1c0/gfx/angle/README.mozilla As it stands these changes are likely to get unintentionally backed out during the next ANGLE update. Maybe that's ok because they'll come back for the next one...
Assignee | ||
Comment 8•10 years ago
|
||
Yes, for ANGLE changes it was fine, because they were upstreamed. However, I didn't notice that moz.build files are also generated by a script living only in mozilla branch of angle, so they were backed out during update. The attached patch fixes that. Generated files are exactly the same as with patches committed to m-c earlier. I don't know the process to get it to github repo.
Attachment #8503599 -
Flags: review?(jmuizelaar)
Assignee | ||
Updated•10 years ago
|
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 9•10 years ago
|
||
Comment on attachment 8503599 [details] [diff] [review] generate_mozbuild.diff Review of attachment 8503599 [details] [diff] [review]: ----------------------------------------------------------------- I landed this fix on inbound in bug 1089199 and have pushed it to the github repo
Attachment #8503599 -
Flags: review?(jmuizelaar) → review+
Assignee | ||
Updated•9 years ago
|
Status: REOPENED → RESOLVED
Closed: 10 years ago → 9 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•