709914 - Slice out WebGL and the ANGLE compiler from libxul

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[JP Rosevear [:jpr]]

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.

Comment 1

[Benoit Jacob [:bjacob] (mostly away)]

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.

Comment 2

[Mike Hommey [:glandium]]

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.

Comment 3

[Benoit Jacob [:bjacob] (mostly away)]

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.

Comment 4

[Benoit Jacob [:bjacob] (mostly away)]

http://hg.mozilla.org/releases/mozilla-beta/rev/d2f1a400e430

keeping open for central and aurora

Comment 5

[Kelsey Gilbert [:jgilbert]]

bjacob's post was a mt from bug 709947.

Comment 6

[Mike Hommey [:glandium]]

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

Comment 7

[Mike Hommey [:glandium]]

Apparently, this breaks webgl tests :(

Comment 8

[Kelsey Gilbert [:jgilbert]]

Some of those webgl fails look like problems with video. How does the try push look before the ANGLE split patch?

Comment 9

[Mike Hommey [:glandium]]

(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

Comment 10

[Benoit Jacob [:bjacob] (mostly away)]

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?

Comment 11

[Benoit Jacob [:bjacob] (mostly away)]

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.

Comment 12

[Benoit Jacob [:bjacob] (mostly away)]

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.

Comment 13

[Mike Hommey [:glandium]]

(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.

Comment 14

[Benoit Jacob [:bjacob] (mostly away)]

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

Comment 15

[Benoit Jacob [:bjacob] (mostly away)]

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.

Comment 16

[(no longer active)]

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.

Comment 17

[(no longer active)]

Attachment: part 2: fix the variable access

Comment 18

[Chris Pearce [:cpearce (Not reading bugmail)]]

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

Comment 19

[(no longer active)]

(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

[Mozilla RelEng Bot]

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/[e-mail]-5026656c54b6

Comment 21

[Mike Hommey [:glandium]]

Attachment: Slice out the ANGLE compiler from libxul on Windows

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

Comment 22

[Mike Hommey [:glandium]]

http://hg.mozilla.org/mozilla-central/rev/beac16509534

Comment 23

[Mozilla RelEng Bot]

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/[e-mail]-5026656c54b6