Closed Bug 578938 Opened 10 years ago Closed 10 years ago

Shared, non-libxul builds failing due to ANGLE changes


(Core :: Canvas: WebGL, defect, blocker)

Not set





(Reporter: standard8, Assigned: vlad)




(2 files, 2 obsolete files)

We've got several failures on non-libxul, fully-shared builds.

- libangle fails to build because it needs NSPR and MOZALLOC libs in EXTRA_DSO_LDOPTS (easy to fix).

- libgklayout fails to build because libangle doesn't export the functions that are required elsewhere.

For the latter issue, I think we have two options:

1) Export the required symbols, this requires modification to the imported source of angle.

2) Build angle as a static lib into the gfx library.

I've a patch for the first of these, but do we do any modification to the imported code, would modification be acceptable?

If not I think the only option would be to make it a static lib into the gfx library, similar to what bug 562659 was suggesting for ycbcr.
Attachment #457527 - Flags: review?(vladimir)
Attached patch Option 2 - static angle library (obsolete) — Splinter Review
Possible option 2 - this makes angle a static library and links it into layout. The only downside is that we're linking something in gfx into layout, however if we don't export the symbols, that's the only option we've got.
Attachment #457533 - Flags: review?(vladimir)
(In reply to comment #0)
> Created attachment 457527 [details] [diff] [review]
> Option 1 - export requires symbols

Note: this patch doesn't work on windows due to the fastcall on the ShFinalize function. I'm leaving it up there for now until I get some feedback on which way we want to go.
Blocks: 571172
No longer depends on: 571172
Hmm.. I like option 1 better, but it would be better if it was something we could upstream.  Though I guess we could just keep it as a local patch.  Do we even need to patch the cpp file?  Seems like just getting the declspec's in the .h file should be enough.  Also, that __fastcall can just go, I don't think there's any reason for that to be __fastcall.
Duplicate of this bug: 579141
Attached patch fixSplinter Review
Ok, here's a fix; I'll check this in along with a .patch file.  This can also really be upstreamed, though it's not essential.
Assignee: bugzilla → vladimir
Attachment #457527 - Attachment is obsolete: true
Attachment #457533 - Attachment is obsolete: true
Attachment #457635 - Flags: review?(me)
Attachment #457527 - Flags: review?(vladimir)
Attachment #457533 - Flags: review?(vladimir)
Comment on attachment 457635 [details] [diff] [review]


Is LIBS_DIR necessary here?



r=me with that.
Attachment #457635 - Flags: review?(me) → review+
Closed: 10 years ago
Resolution: --- → FIXED
We seem to have fixed most of it, but static builds on windows are still busted:

I've played around with a few things, but not worked out a solution yet.
I will try clobbering those windows boxes though, just in case.
Comment on attachment 457635 [details] [diff] [review]

>+#ifdef ANGLE_BUILD
>+#define ANGLE_EXPORT  __declspec(dllexport)
> #else
>+#define ANGLE_EXPORT  __declspec(dllimport)
>+#define ANGLE_EXPORT
Sadly this is still broken for static Windows builds, as it needs the null #define as per LIBXUL builds. Locally I just tried wrapping an extra #ifndef MOZ_STATIC_BUILD ... #else #define ANGLE_EXPORT #endif but I guess you might prefer if I used !defined(MOZ_ENABLE_LIBXUL) && !define(MOZ_STATIC_BUILD) ?
yeah, !define && !define is better
Attached patch fix static tooSplinter Review
Vlad, this should fix static too; I marked the patch author as Neil, as it was none of my investigation/testing of this fix though.
Attachment #458039 - Flags: review?(vladimir)
Attachment #458039 - Flags: review?(vladimir) → review+
I won't get around to landing this for a day or two, please push earlier if you can. Remember, this is Neil's patch not my own.
Keywords: checkin-needed
Pushed as - thanks to Neil, Callek, and vlad!
Keywords: checkin-needed
You need to log in before you can comment on or make changes to this bug.