Closed
Bug 709914
Opened 13 years ago
Closed 13 years ago
Slice out WebGL and the ANGLE compiler from libxul
Categories
(Core :: Graphics: CanvasWebGL, defect)
Tracking
()
RESOLVED
FIXED
mozilla11
People
(Reporter: jpr, Assigned: glandium)
References
Details
Attachments
(2 files, 1 obsolete file)
2.11 KB,
patch
|
cpearce
:
review+
|
Details | Diff | Splinter Review |
2.27 KB,
patch
|
Details | Diff | Splinter Review |
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•13 years ago
|
||
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
Assignee | ||
Comment 2•13 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.
Updated•13 years ago
|
Assignee: nobody → jgilbert
Comment 3•13 years ago
|
||
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•13 years ago
|
||
http://hg.mozilla.org/releases/mozilla-beta/rev/d2f1a400e430
keeping open for central and aurora
Comment 5•13 years ago
|
||
bjacob's post was a mt from bug 709947.
Assignee | ||
Comment 6•13 years ago
|
||
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•13 years ago
|
Assignee: jgilbert → mh+mozilla
Assignee | ||
Comment 7•13 years ago
|
||
Apparently, this breaks webgl tests :(
Comment 8•13 years ago
|
||
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•13 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
Comment 10•13 years ago
|
||
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•13 years ago
|
||
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•13 years ago
|
||
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•13 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.
Comment 14•13 years ago
|
||
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•13 years ago
|
||
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•13 years ago
|
||
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•13 years ago
|
||
Attachment #581810 -
Flags: review?(chris)
Comment 18•13 years ago
|
||
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+
Comment 19•13 years ago
|
||
(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•13 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•13 years ago
|
||
Refreshed user info, in case someone else lands the patch.
Assignee | ||
Updated•13 years ago
|
Attachment #581206 -
Attachment is obsolete: true
Assignee | ||
Comment 22•13 years ago
|
||
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
Comment 23•13 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.
Description
•