Closed Bug 630019 Opened 9 years ago Closed 9 years ago

After landing Bug 629538, ANGLE disabled

Categories

(Core :: Canvas: WebGL, defect)

x86
Windows 7
defect
Not set

Tracking

()

VERIFIED FIXED
Tracking Status
blocking2.0 --- betaN+

People

(Reporter: alice0775, Assigned: vlad)

References

Details

(Keywords: regression, Whiteboard: [hardblocker][has patch][fx4-fixed-bugday])

Attachments

(2 files, 4 obsolete files)

Build Identifier:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b11pre) Gecko/20110130 Firefox/4.0b11pre ID:20110130030342

After landing Bug 629538, ANGLE disabled.

Reproducible: Always

Steps to Reproduce:
1. Start Minefield with new profile
2. about:support
3.

Actual Results:
  WebGL RendererATI Technologies Inc. -- ATI Radeon HD 4300/4500 Series        -- 3.3.10428 Compatibility Profile Context
Expected Results:
  WebGL RendererTransGaming Inc. -- ANGLE -- OpenGL ES 2.0 (git-devel Jan 29 2011 13:40:18)

Regression window:
Works;
http://hg.mozilla.org/mozilla-central/rev/3ea2b5a7c9c8
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b11pre) Gecko/20110129 Firefox/4.0b11pre ID:20110129131351
Fails:
http://hg.mozilla.org/mozilla-central/rev/dd5e9bfd0f6a
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b11pre) Gecko/20110129 Firefox/4.0b11pre ID:20110129143521
Pushlog;
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=3ea2b5a7c9c8&tochange=dd5e9bfd0f6a
blocking2.0: --- → ?
And WinXP machine also disabled ANGLE  and WebGL unavailable

Actual Results:
  WebGL Renderer(WebGL unavailable)
Expected Results:
  WebGL RendererTransGaming Inc. -- ANGLE -- OpenGL ES 2.0 (git-devel Jan 29 2011 13:40:18)
Thanks for the report! Let's look at the build log ...
The WebGL unavailable on WinXP is really weird: this should not depend on ANGLE.
Duplicate of this bug: 630020
The error is NOT a configure error:

ftp://ftp.mozilla.org/pub/firefox/tinderbox-builds/mozilla-central-win32/1296385422/mozilla-central-win32-nightly-build49.txt.gz

This is changeset 336d5906cb0f7be789450241d9813f8412ebe360 which is after the ANGLE upgrade, and ANGLE is correctly built:

=== Building ANGLE via devenv.exe ===
Upgrading solution...
Building solution, target Release|Win32...
Copying dlls...
e:/builds/moz2_slave/cen-w32-ntly/build/obj-firefox/config/nsinstall.exe -m 755 libGLESv2.dll libEGL.dll ../../dist/bin
make[6]: Leaving directory `/e/builds/moz2_slave/cen-w32-ntly/build/obj-firefox/gfx/angle'
Attached patch attempt to fix angle (obsolete) — Splinter Review
Here's my WIP patch. Still not sure what happened, because I couldn't find the ANGLE build logs from tinderbox and I'm still building locally with this patch.

But these are changes that I'd like anyway:

 * always search in the registry for DirectX SDK and set MOZ_DIRECTX_SDK_PATH. Previously it was only searching if d3dx9.h was not found in INCLUDE path. That could have resulted in attempting to build ANGLE without LIB properly set in gfx/angle/Makefile.

 * also detect June 2010 SDK if February 2010 SDK not found. That was asked for two people last week on #developers who found it really hard to build with ANGLE.

 * once MOZ_DIRECT_SDK_PATH is found, do a proper AC_CHECK_HEADER for d3dx9.h instead of custom bash script to detect file existence; drop the dxguid.lib check as AC_CHECK_LIB is not practical to use with libs that dont seem to export functions.
Quick update --- I've been stuck on the fact that, when building with pymake, the script in gfx/angle/Makefile building libGLESv2.dll, calling devenv.exe, failed to pick up the DX SDK path from the INCLUDE environment variable. When building with make -C gfx/angle instead of pymake, it works.

This seems to be a pymake bug!

Now implementing a work-around...
Specifically, this bit of the gfx/angle/makefile:

ifdef MOZ_DIRECTX_SDK_PATH
INCLUDE := $(INCLUDE);$(MOZ_DIRECTX_SDK_PATH)\include
LIB := $(LIB);$(MOZ_DIRECTX_SDK_PATH)\lib\x86
endif

does not take effect with pymake.
Sigh ... pymake -j8 has the issue, but pymake -j1 works ...
Ah no OK... the real problem is that I was counting on

   INCLUDE := foo

to assign to the INCLUDE environment variable, not just to some INCLUDE Make variable. That works with make but not with pymake, apparently.
Attached patch attempt to fix angle, v2 (obsolete) — Splinter Review
OK, this new version explicitly exports Make variables into environment variables, to please pymake.

Another (sun)day lost to our build system :-( grr.
Attachment #508252 - Attachment is obsolete: true
Comment on attachment 508276 [details] [diff] [review]
attempt to fix angle, v2


>diff --git a/gfx/angle/Makefile.in b/gfx/angle/Makefile.in
>--- a/gfx/angle/Makefile.in
>+++ b/gfx/angle/Makefile.in
>@@ -166,16 +166,18 @@ libs:: libGLESv2.dll libEGL.dll
> 	$(INSTALL) $(IFLAGS2) libGLESv2.dll libEGL.dll $(DIST)/bin
> 
> # we don't want this to attempt to parallel-build these dlls;
> # building one will build both.
> libGLESv2.dll: libEGL.dll
> 
> libEGL.dll: $(GLOBAL_DEPS) $(ANGLE_DEPS)
> 	@(echo "=== Building ANGLE via devenv.exe ===" \
>+        && export INCLUDE="$(INCLUDE)" \ # pymake does not take care of this automatically! GNU make does.
>+        && export LIB="$(LIB)" \
> 	&& rm -rf angle-build && mkdir angle-build \
> 	&& cp -r $(srcdir)/src $(srcdir)/include angle-build \
> 	&& cd angle-build/src \
> 	&& echo "Upgrading solution..." \
> 	&& devenv angle.sln //upgrade \
> 	&& echo "Building solution, target $(ANGLE_DIR)|Win32..." \
> 	&& devenv angle.sln //useenv //build "$(ANGLE_DIR)|Win32" \
> 	&& echo "Copying dlls..." \

Is any of the other stuff other than this needed to fix the issue?  I don't really like the June search, because we're only hardcoding that SDK due to simplicity on the tinderboxes.  The right fix is to look in the registry and pick the latest one (or one we like), but I don't think we need to do that now.
(In reply to comment #12)
> Comment on attachment 508276 [details] [diff] [review]
> attempt to fix angle, v2
> 
> 
> >diff --git a/gfx/angle/Makefile.in b/gfx/angle/Makefile.in
> >--- a/gfx/angle/Makefile.in
> >+++ b/gfx/angle/Makefile.in
> >@@ -166,16 +166,18 @@ libs:: libGLESv2.dll libEGL.dll
> > 	$(INSTALL) $(IFLAGS2) libGLESv2.dll libEGL.dll $(DIST)/bin
> > 
> > # we don't want this to attempt to parallel-build these dlls;
> > # building one will build both.
> > libGLESv2.dll: libEGL.dll
> > 
> > libEGL.dll: $(GLOBAL_DEPS) $(ANGLE_DEPS)
> > 	@(echo "=== Building ANGLE via devenv.exe ===" \
> >+        && export INCLUDE="$(INCLUDE)" \ # pymake does not take care of this automatically! GNU make does.
> >+        && export LIB="$(LIB)" \
> > 	&& rm -rf angle-build && mkdir angle-build \
> > 	&& cp -r $(srcdir)/src $(srcdir)/include angle-build \
> > 	&& cd angle-build/src \
> > 	&& echo "Upgrading solution..." \
> > 	&& devenv angle.sln //upgrade \
> > 	&& echo "Building solution, target $(ANGLE_DIR)|Win32..." \
> > 	&& devenv angle.sln //useenv //build "$(ANGLE_DIR)|Win32" \
> > 	&& echo "Copying dlls..." \
> 
> Is any of the other stuff other than this needed to fix the issue?

Depends a lot on people's configuration. Without my configure.in patch, if you happen to have d3dx9.h in your include path, then it doesn't search for the SDK and consequently doesn't set LIB, giving you a linking error.

>  I don't
> really like the June search, because we're only hardcoding that SDK due to
> simplicity on the tinderboxes.  The right fix is to look in the registry and
> pick the latest one (or one we like), but I don't think we need to do that now.

Sure, I completely agree that this is the right fix. I just dont know enough about 'reg' to implement it. Meanwhile, since everybody has either Feb 2010 or June 2010, this works. (Two people asked on #developers last week, both had June, which is the current version).
Attached patch attempt to fix angle, v3 (obsolete) — Splinter Review
Thanks to jag on #developers, this is the right fix: use export directly in the make code.
So, the error is "No configs" at GLContextProviderEGL.cpp:1762:

    // if we're running under ANGLE, we can't set
    // BIND_TO_TEXTURE since we're probably doing d3d interop
    if (sEGLLibrary.IsANGLE()) {
        int k = sizeof(attribs)/sizeof(EGLint) - 3;
        attribs[k] = LOCAL_EGL_NONE;
        attribs[k+1] = LOCAL_EGL_NONE;
    }

    EGLConfig configs[64];
    int numConfigs = 64;

    if (!sEGLLibrary.fChooseConfig(EGL_DISPLAY(),
                                   attribs,
                                   configs, numConfigs,
                                   &numConfigs)
        || numConfigs == 0)
    {
        NS_WARNING("No configs");
        // no configs? no pbuffers!
        return nsnull;
    }
Attachment #508276 - Attachment is obsolete: true
So, my patch here only fixes the build for people building locally, it doesn't fix the actual issue that ANGLE is disabled in nightlies, but Vlad is working on that.
Blocking betaN per discussion wiht bjacob.
blocking2.0: ? → betaN+
Whiteboard: [hardblocker]
OK, now it's ready for review, it automatically finds what version of the DX SDK you have.
Attachment #508281 - Attachment is obsolete: true
Attachment #508301 - Flags: review?(vladimir)
Could we use DXSDK_DIR? At least for me after installing the June SDK on Win7 I see this in my env:

DXSDK_DIR=C:\Program Files (x86)\Microsoft DirectX SDK (June 2010)\


And I tried running your registry query here but came up empty:

C:\Users\jag>reg query "HKLM\Software\Microsoft\DirectX" /s

HKEY_LOCAL_MACHINE\Software\Microsoft\DirectX
    InstalledVersion    REG_BINARY    0000000900000000
    Version    REG_SZ    4.09.00.0904
    SDKVersion    REG_SZ    9.29.1962.0

In my registry, what you're looking for seems to be at:

HKEY_LOCAL_MACHINE\SOFTWARE\Wow6432Node\Microsoft\DirectX\Microsoft DirectX SDK (June 2010)\InstallPath

Also, can more than one version of the SDK be installed? Left-over registry keys? Is the first entry (head -n 1) guaranteed to be the one you're looking for?

Hopefully a moot issue, if we can just depend on DXSDK_DIR. See also bug 535590.

Finally, for what it's worth, with the free Visual Studio Express I don't seem to have devenv.exe installed (as someone also mentioned in that bug).
Comment on attachment 508301 [details] [diff] [review]
fix angle build (and automatically find DX SDK)

How about something like this instead?

Though if we can just depend on DXSDK_DIR then let's drop the registry fu.

    MOZ_ANGLE=,
    MOZ_ANGLE=1)

if test -n "$MOZ_ANGLE"; then
  if test -z "$_WIN32_MSVC"; then
    AC_MSG_ERROR([Building ANGLE requires MSVC.  To build without ANGLE, reconfigure with --disable-angle.])
  fi

  AC_CHECK_HEADER(d3dx9.h, [], [MOZ_ANGLE=])

  if test -z "$MOZ_ANGLE"; then
    # Let's see if it's in $DXSDK_DIR
    MOZ_DIRECTX_SDK_PATH=$DXSDK_DIR
    if test -n "$MOZ_DIRECTX_SDK_PATH" ; then
      if test -f "$MOZ_DIRECTX_SDK_PATH"/include/d3dx9.h && test -f "$MOZ_DIRECTX_SDK_PATH"/lib/x86/dxguid.lib ; then
        AC_MSG_WARN([Found DirectX SDK via DXSDK_DIR, using $MOZ_DIRECTX_SDK_PATH])
        MOZ_ANGLE=1
      fi
    fi

    if test -z "$MOZ_ANGLE"; then
      # Let's search the registry for the June 2010 and February 2010 versions
      MOZ_DIRECTX_SDK_PATH=`reg query 'HKLM\Software\Microsoft\DirectX\Microsoft DirectX SDK (June 2010)' //v InstallPath | grep REG_SZ | sed 's,  *, ,g' | cut -d' ' -f4-`
      if test -n "$MOZ_DIRECTX_SDK_PATH" ; then
        if test -f "$MOZ_DIRECTX_SDK_PATH"/include/d3dx9.h && test -f "$MOZ_DIRECTX_SDK_PATH"/lib/x86/dxguid.lib ; then
          AC_MSG_WARN([Found DirectX SDK in registry, using $MOZ_DIRECTX_SDK_PATH])
          MOZ_ANGLE=1
        fi
      fi

      if test -z "$MOZ_ANGLE"; then
        MOZ_DIRECTX_SDK_PATH=`reg query 'HKLM\Software\Microsoft\DirectX\Microsoft DirectX SDK (February 2010)' //v InstallPath | grep REG_SZ | sed 's,  *, ,g' | cut -d' ' -f4-`
        if test -n "$MOZ_DIRECTX_SDK_PATH" ; then
          if test -f "$MOZ_DIRECTX_SDK_PATH"/include/d3dx9.h && test -f "$MOZ_DIRECTX_SDK_PATH"/lib/x86/dxguid.lib ; then
            AC_MSG_WARN([Found DirectX SDK in registry, using $MOZ_DIRECTX_SDK_PATH])
            MOZ_ANGLE=1
          fi
        fi
      fi
    fi
  fi

  if test -z "$MOZ_ANGLE"; then
    AC_MSG_WARN([Couldn't find the DirectX SDK, needed for ANGLE.  Please install the February 2010 or June 2010 DirectX SDK.  To explicitly build without ANGLE, reconfigure with --disable-angle.])
    AC_MSG_WARN([This will become an error in the future.])
    MOZ_DIRECTX_SDK_PATH=
  fi
fi
Oh, right, and that doesn't take the Wow6432Node stuff into account.
Updated ANGLE pbuffers patch.  Needed some additional tweaks; the version/vendor string changed (TransGaming -> Google, but ANGLE remains in the versionstring), and due to the internal change to ANGLE, we no longer need a Y-flip for d3d rendering.
Attachment #508311 - Flags: review?(bjacob)
Whiteboard: [hardblocker] → [hardblocker][has patch]
(In reply to comment #20)
> Comment on attachment 508301 [details] [diff] [review]
> fix angle build (and automatically find DX SDK)
> 
> How about something like this instead?
> 
> Though if we can just depend on DXSDK_DIR then let's drop the registry fu.

Indeed --- thanks for finding about this env var. Will try on tryserver.

> 
>     MOZ_ANGLE=,
>     MOZ_ANGLE=1)
> 
> if test -n "$MOZ_ANGLE"; then
>   if test -z "$_WIN32_MSVC"; then
>     AC_MSG_ERROR([Building ANGLE requires MSVC.  To build without ANGLE,
> reconfigure with --disable-angle.])
>   fi
> 
>   AC_CHECK_HEADER(d3dx9.h, [], [MOZ_ANGLE=])
> 
>   if test -z "$MOZ_ANGLE"; then

As I explained above, we really want to do all this stuff and assign to MOZ_DIRECTX_SDK_PATH even if d3dx9.h is found.
Ah, for LIB. Sorry, hadn't read that far back. Then I'd skip the initial test altogether, so (note I'm clearing MOZ_ANGLE to compensate for not running the header check anymore):

if test -n "$MOZ_ANGLE"; then
  MOZ_ANGLE=

  if test -z "$_WIN32_MSVC"; then
    AC_MSG_ERROR([Building ANGLE requires MSVC.  To build without ANGLE,
reconfigure with --disable-angle.])
  else
    # Let's see if it's in $DXSDK_DIR
    MOZ_DIRECTX_SDK_PATH=$DXSDK_DIR
    if test -n "$MOZ_DIRECTX_SDK_PATH" ; then
      if test -f "$MOZ_DIRECTX_SDK_PATH"/include/d3dx9.h && test -f
"$MOZ_DIRECTX_SDK_PATH"/lib/x86/dxguid.lib ; then
        AC_MSG_WARN([Found DirectX SDK via DXSDK_DIR, using
$MOZ_DIRECTX_SDK_PATH])
        MOZ_ANGLE=1
      fi
    fi

    if test -z "$MOZ_ANGLE"; then
      AC_MSG_WARN([Couldn't find the DirectX SDK, needed for ANGLE.  Please
install the February 2010 or June 2010 DirectX SDK.  To explicitly build
without ANGLE, reconfigure with --disable-angle.])
      AC_MSG_WARN([This will become an error in the future.])
    fi
  fi
fi
OK, I updated the patch to use your idea. Can you review? (it's mostly your own patch now... :-) )
Attachment #508301 - Attachment is obsolete: true
Attachment #508407 - Flags: review?
Attachment #508301 - Flags: review?(vladimir)
Attachment #508407 - Flags: review? → review?(jag-mozbugs)
Comment on attachment 508407 [details] [diff] [review]
fix angle build (and automatically find DX SDK)

> if test -n "$MOZ_ANGLE"; then
>   if test -z "$_WIN32_MSVC"; then
>     AC_MSG_ERROR([Building ANGLE requires MSVC.  To build without ANGLE, reconfigure with --disable-angle.])

Ah, right, no need to clear MOZ_ANGLE before this 'coz it'll error out.

>+  MOZ_DIRECTX_SDK_PATH=$DXSDK_DIR
>+  MOZ_ANGLE=
>+
>+  if test -n "$MOZ_DIRECTX_SDK_PATH" ; then
>+    if test -f "$MOZ_DIRECTX_SDK_PATH"/include/d3dx9.h && test -f "$MOZ_DIRECTX_SDK_PATH"/lib/x86/dxguid.lib ; then
>+      AC_MSG_WARN([Found DirectX SDK via DXSDK_DIR, using $MOZ_DIRECTX_SDK_PATH])
>+      MOZ_ANGLE=1
>+    fi
>   fi
> 
>+  if test -z "$MOZ_ANGLE" ; then
>+    AC_MSG_WARN([Couldn't find the DirectX SDK, needed for ANGLE. Please install it (February 2010 or newer). To explicitly build without ANGLE, reconfigure with --disable-angle.])
>     AC_MSG_WARN([This will become an error in the future.])
>+    MOZ_ANGLE=

Redundant, no? Would we wanna clear MOZ_DIRECTX_SDK_PATH, or is that only ever used if MOZ_ANGLE is set?

r=jag
Attachment #508407 - Flags: review?(jag-mozbugs) → review+
(In reply to comment #26)
> >+    MOZ_ANGLE=
> 
> Redundant, no? Would we wanna clear MOZ_DIRECTX_SDK_PATH, or is that only ever
> used if MOZ_ANGLE is set?

Redundant indeed; and yes, this is only used if MOZ_ANGLE is set.

But, problem: today I got again the same issue as yesterday with pymake -j8 !

Namely the problem that the ANGLE build using devenv.exe fails to find d3dx9.h. Investigating...
Hah! the DXSDK_DIR variable has a trailing \ that confuses something down the road; eventually INCLUDE  has

    C:\Program Files (x86)\Microsoft DirectX SDK (June 2010) \include

with that weird space before the last \ and it fails.
Comment on attachment 508311 [details] [diff] [review]
updated ANGLE pbuffers patch

Don't see anything wrong in this patch! And confirmed it works.
Attachment #508311 - Flags: review?(bjacob) → review+
http://hg.mozilla.org/mozilla-central/rev/55196a9321b1 for core pbuffers issue..
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Actually, the approach with the DXSDK_DIR environment variable failed on tinderboxes, perhaps related to the fact that the DXSDK is installed at a nonstandard location on them. So I replaced it with the registry approach (the later patch that detects the version from the registry instead of hardcoding it):

http://hg.mozilla.org/mozilla-central/rev/4a62f7e1f128

and now the tinderboxes are happy.
Can we file a bug on getting the DXSDK_DIR variable set on the Tinderboxen? Or I guess really we should add support for setting MOZ_DIRECTX_SDK_PATH in the .mozconfig somehow.

Like I pointed out, if there are multiple matches (again, I'm assuming this can be the case), we're not guaranteed to get the latest (or intended) version by simply taking the first item on the returned list of matches. "Go hack your registry" is not the kind of advice we'd like to give in that case.

Too bad you didn't keep the DXSDK_DIR stuff, it would've been easy enough to introduce the registry hack as fail-over code. Let's get that fixed in the new bug :-)
(Discussion continued on bug 630463)
I had WebGL, then it broke and I assumed it was this bug. I'm on today's nightly and I still do not have WebGL with WebGL Renderer (WebGL unavailable) in about:support. Is this bug not yet completely fixed or am I experiencing a different issue?
Why am I seing: 

WebGL Renderer ATI Technologies Inc. -- ATI Mobility Radeon HD 4500/5100 Series -- 3.3.10428 Compatibility Profile Context

in about:support? (Latest nightly), Win7 x64
@ Asa and Wamsaya:

  This is a different issue. Currently you still have to install the DirectX runtime (google for it) in order for ANGLE to work. In Bug 630628 we are working on shipping with Firefox the relevant bits so that that won't be needed anymore. I have made a tryserver build, so you are very welcome to try it and let me know if it works (without installing DirectX runtime):

http://stage.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/bjacob@mozilla.com-70d9e50c0217/try-w32/firefox-4.0b12pre.en-US.win32.zip
Ok! Your build works fine.

Thanks
Ok! Your build works fine.

Thanks
bjacob, yes, that build works great. you have identified my problem. thank you.
Verified fixed in Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b12pre) Gecko/20110204 Firefox/4.0b12pre
Status: RESOLVED → VERIFIED
Whiteboard: [hardblocker][has patch] → [hardblocker][has patch][fx4-fixed-bugday]
Duplicate of this bug: 631886
Depends on: 633954
Assignee: nobody → vladimir
You need to log in before you can comment on or make changes to this bug.