Slice out WebGL and the ANGLE compiler from libxul

RESOLVED FIXED in mozilla11

Status

()

Core
Canvas: WebGL
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: jpr, Assigned: glandium)

Tracking

11 Branch
mozilla11
x86
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

6 years ago
In order to make libxul lighter and help move out of bug 709193, we could separate out ANGLE in a separate library.

According to glandium, we should probably put all this sliced out stuff into one library.
(Reporter)

Updated

6 years ago
Depends on: 70972
Notice that ANGLE consists of 3 parts:
 - the compiler
 - libGLESv2   (Windows-only)
 - libEGL      (Windows-only)

libGLESv2 and libEGL are already separate DLLs.

What we could do is split out the WebGL implementation and the ANGLE compiler (i.e. the only part of ANGLE that's currently in libxul) into a separate 'mozwebgl' library.
Summary: Slice out ANGLE from libxul → Slice out WebGL and the ANGLE compiler from libxul

Updated

6 years ago
Depends on: 709721
No longer depends on: 70972
(Assignee)

Comment 2

6 years ago
It would be better to put the whole of angle in one place (so, including libGLESv2 and libEGL, except if they are LoadLibrary()ed), and group that with the media libraries. I named the media library gkmedias.dll, but it could be renamed to anything.
Assignee: nobody → jgilbert
I disagree with comment 2, some additional background: the libGLESv2.dll and libEGL.dll parts of ANGLE implement standards, they're interchangeable with other implementations of these standards. It's actually useful to be able to replace them with e.g. the ES drivers provided by ATI on Windows. So, keep them unchanged please.
http://hg.mozilla.org/releases/mozilla-beta/rev/d2f1a400e430

keeping open for central and aurora
bjacob's post was a mt from bug 709947.
(Assignee)

Comment 6

6 years ago
Created attachment 581206 [details] [diff] [review]
Slice out the ANGLE compiler from libxul on Windows

This folds the ANGLE compiler with the media libraries from bug 709721. I kept using the layout/media directory and the gkmedias library name. Pushed to try on all platforms, to confirm that nothing breaks as a result:
https://tbpl.mozilla.org/?noignore=1&tree=Try&rev=683e92eb1df0
Attachment #581206 - Flags: review?(khuey)
(Assignee)

Updated

6 years ago
Assignee: jgilbert → mh+mozilla
(Assignee)

Comment 7

6 years ago
Apparently, this breaks webgl tests :(
Some of those webgl fails look like problems with video. How does the try push look before the ANGLE split patch?
Attachment #581206 - Flags: review?(khuey) → review+
(Assignee)

Comment 9

6 years ago
(In reply to Jeff Gilbert [:jgilbert] from comment #8)
> Some of those webgl fails look like problems with video. How does the try
> push look before the ANGLE split patch?

https://tbpl.mozilla.org/?tree=Try&rev=6493efb5b8e9
My first concern looking at this patch is that the ANGLE compiler files are added to 3 makefiles: the one you touched, and again in the 2 other makefiles

$ find . -name Makefile.in
./Makefile.in
./src/libGLESv2/Makefile.in
./src/libEGL/Makefile.in

So is the current patch compiling these in *both* libxul and the new lib?
Ignore comment 10, i'm stupid. these 2 other makefiles compile into other libraries, not into libxul, and it is fully intentional that they ship a copy of the angle compiler, as they need that code and we dont want (I guess) to have them depend on libxul.
Though... what we could do is compile the angle compiler into a standalone library and then have libxul, libEGL and libGLESv2 all link to it.
(Assignee)

Comment 13

6 years ago
(In reply to Benoit Jacob [:bjacob] from comment #12)
> Though... what we could do is compile the angle compiler into a standalone
> library and then have libxul, libEGL and libGLESv2 all link to it.

That's basically what this is about, except libEGL and libGLESv2 statically link with angle with the patch here.
I've looked into these WebGL test failures relating to video, they look like regressions from bug 709721. The WebGL tests failing here are trying to load WebM videos. The videos never get loaded, so they time out.

I opened one of these WebM files in a new tab,
https://cvs.khronos.org/svn/repos/registry/trunk/public/webgl/sdk/tests/conformance/resources/red-green.webmvp8.webm

and got a null-nsRefPtr-deref assertion failure:

 	KernelBase.dll!_DebugBreak@0()  + 0x2 bytes	
 	xul.dll!RealBreak()  Line 422	C++
 	xul.dll!Break(const char * aMsg)  Line 521	C++
 	xul.dll!NS_DebugBreak_P(unsigned int aSeverity, const char * aStr, const char * aExpr, const char * aFile, int aLine)  Line 380 + 0xc bytes	C++
 	xul.dll!nsRefPtr<nsWebMBufferedState>::operator->()  Line 1055 + 0x23 bytes	C++
 	xul.dll!nsWebMReader::NotifyDataArrived(const char * aBuffer, unsigned int aLength, unsigned int aOffset)  Line 806 + 0x1a bytes	C++
>	xul.dll!nsBuiltinDecoderStateMachine::NotifyDataArrived(const char * aBuffer, unsigned int aLength, unsigned int aOffset)  Line 229	C++
 	xul.dll!nsBuiltinDecoder::NotifyDataArrived(const char * aBuffer, unsigned int aLength, unsigned int aOffset)  Line 511	C++
 	xul.dll!nsMediaChannelStream::CopySegmentToCache(nsIInputStream * aInStream, void * aClosure, const char * aFromSegment, unsigned int aToOffset, unsigned int aCount, unsigned int * aWriteCount)  Line 381	C++
 	xul.dll!nsInputStreamTee::WriteSegmentFun(nsIInputStream * in, void * closure, const char * fromSegment, unsigned int offset, unsigned int count, unsigned int * writeCount)  Line 223 + 0x23 bytes	C++
 	xul.dll!nsPipeInputStream::ReadSegments(unsigned int (nsIInputStream *, void *, const char *, unsigned int, unsigned int, unsigned int *)* writer, void * closure, unsigned int count, unsigned int * readCount)  Line 799 + 0x1d bytes	C++
 	xul.dll!nsInputStreamTee::ReadSegments(unsigned int (nsIInputStream *, void *, const char *, unsigned int, unsigned int, unsigned int *)* writer, void * closure, unsigned int count, unsigned int * bytesRead)  Line 277	C++
 	xul.dll!nsMediaChannelStream::OnDataAvailable(nsIRequest * aRequest, nsIInputStream * aStream, unsigned int aCount)  Line 411 + 0x1f bytes	C++
 	xul.dll!nsMediaChannelStream::Listener::OnDataAvailable(nsIRequest * aRequest, nsISupports * aContext, nsIInputStream * aStream, unsigned int aOffset, unsigned int aCount)  Line 128	C++
 	xul.dll!mozilla::dom::MediaDocumentStreamListener::OnDataAvailable(nsIRequest * request, nsISupports * ctxt, nsIInputStream * inStr, unsigned int sourceOffset, unsigned int count)  Line 117 + 0x30 bytes	C++
 	xul.dll!nsDocumentOpenInfo::OnDataAvailable(nsIRequest * request, nsISupports * aCtxt, nsIInputStream * inStr, unsigned int sourceOffset, unsigned int count)  Line 322 + 0x30 bytes	C++
 	xul.dll!nsStreamListenerTee::OnDataAvailable(nsIRequest * request, nsISupports * context, nsIInputStream * input, unsigned int offset, unsigned int count)  Line 111 + 0x35 bytes	C++
 	xul.dll!nsHttpChannel::OnDataAvailable(nsIRequest * request, nsISupports * ctxt, nsIInputStream * input, unsigned int offset, unsigned int count)  Line 4418 + 0x60 bytes	C++
 	xul.dll!nsInputStreamPump::OnStateTransfer()  Line 512 + 0x40 bytes	C++
 	xul.dll!nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream * stream)  Line 402 + 0xb bytes	C++
 	xul.dll!nsInputStreamReadyEvent::Run()  Line 115	C++
 	xul.dll!nsThread::ProcessNextEvent(bool mayWait, bool * result)  Line 625 + 0x19 bytes	C++
 	xul.dll!NS_ProcessNextEvent_P(nsIThread * thread, bool mayWait)  Line 245 + 0x17 bytes	C++
 	xul.dll!mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate * aDelegate)  Line 110 + 0xe bytes	C++
 	xul.dll!MessageLoop::RunInternal()  Line 209	C++
 	xul.dll!MessageLoop::RunHandler()  Line 202	C++
 	xul.dll!MessageLoop::Run()  Line 176	C++
 	xul.dll!nsBaseAppShell::Run()  Line 191	C++
 	xul.dll!nsAppShell::Run()  Line 258 + 0x9 bytes	C++
 	xul.dll!nsAppStartup::Run()  Line 220 + 0x1c bytes	C++
 	xul.dll!XRE_main(int argc, char * * argv, const nsXREAppData * aAppData)  Line 3525 + 0x25 bytes	C++
 	firefox.exe!do_main(const char * exePath, int argc, char * * argv)  Line 201 + 0x13 bytes	C++
 	firefox.exe!NS_internal_main(int argc, char * * argv)  Line 287 + 0x14 bytes	C++
 	firefox.exe!wmain(int argc, wchar_t * * argv)  Line 107 + 0xd bytes	C++
 	firefox.exe!__tmainCRTStartup()  Line 552 + 0x19 bytes	C
 	firefox.exe!wmainCRTStartup()  Line 371	C
 	kernel32.dll!@BaseThreadInitThunk@12()  + 0x12 bytes	
 	ntdll.dll!___RtlUserThreadStart@8()  + 0x27 bytes	
 	ntdll.dll!__RtlUserThreadStart@8()  + 0x1b bytes
It's consistently reproducible, just apply this patch and the one from bug 709721 (actually only bug 709721 should be enough but i didn't check) and load in a new tab the video mentioned in previous comment.
What's happening is that inside nsWebMReader::Init, vpx_codec_dec_init fails.  The reason is that it tries to access vpx_codec_vp8_dx_algo, which is a private variable inside gkmedias.dll.  And apparently something is preventing libxul and gkmedias.dll to see the same variable.  Accessing that variable through the accessor function fixes things.  I have a patch for that.
Created attachment 581810 [details] [diff] [review]
part 2: fix the variable access
Attachment #581810 - Flags: review?(chris)
Comment on attachment 581810 [details] [diff] [review]
part 2: fix the variable access

Review of attachment 581810 [details] [diff] [review]:
-----------------------------------------------------------------

We should land/track this in bug 709721. ;)
Attachment #581810 - Flags: review?(chris) → review+
(In reply to Chris Pearce (:cpearce) (Mozilla Corporation) from comment #18)
> Comment on attachment 581810 [details] [diff] [review]
> part 2: fix the variable access
> 
> Review of attachment 581810 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> We should land/track this in bug 709721. ;)

Yeah, sorry, will adjust the commit message before landing.

Comment 20

6 years ago
Try run for 5026656c54b6 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=5026656c54b6
Results (out of 268 total builds):
    success: 236
    warnings: 32
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/eakhgari@mozilla.com-5026656c54b6
(Assignee)

Comment 21

6 years ago
Created attachment 581868 [details] [diff] [review]
Slice out the ANGLE compiler from libxul on Windows

Refreshed user info, in case someone else lands the patch.
(Assignee)

Updated

6 years ago
Attachment #581206 - Attachment is obsolete: true
(Assignee)

Comment 22

6 years ago
http://hg.mozilla.org/mozilla-central/rev/beac16509534
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla11

Comment 23

6 years ago
Try run for 5026656c54b6 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=5026656c54b6
Results (out of 270 total builds):
    success: 238
    warnings: 32
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/eakhgari@mozilla.com-5026656c54b6
You need to log in before you can comment on or make changes to this bug.