Closed Bug 726396 Opened 13 years ago Closed 13 years ago

Huge performance regression on WebGL with ANGLE+D3D10

Categories

(Core :: Graphics: CanvasWebGL, defect)

Product:
Core
Component:
Graphics: CanvasWebGL
Version:
13 Branch
Platform:
x86
Windows 7
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla13

People

(Reporter: alice0775, Assigned: jgilbert)

References

(
URL
)

Details

(Keywords: regression)

Attachments

(2 files, 7 obsolete files)

console log
12.69 KB, text/plain
Details
Fix logic for d3d share handles, EGL PBuffer creation, and user/internal FBO bindings
30.39 KB, patch
bjacob
: review+
Details | Diff | Splinter Review
Build Identifier: http://hg.mozilla.org/mozilla-central/rev/d71dab82fff4 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0a1) Gecko/20120211 Firefox/13.0a1 ID:20120211031145 Huge performance regression on WebGL. Reproducible: Always Steps to Reproduce: 1. Start Firefox with new profile 2. Go http://people.mozilla.org/~jmuizelaar/fishie/fishie-fast.html Actual Results: 20fps / 20fish Expected Results: 60fps / 20fish Regression window(m-c) Last good: http://hg.mozilla.org/mozilla-central/rev/3c1cdbbea964 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0a1) Gecko/20120207 Firefox/13.0a1 ID:20120207010203 First bad: http://hg.mozilla.org/mozilla-central/rev/2b61af9d18ee Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0a1) Gecko/20120207 Firefox/13.0a1 ID:20120207023403 Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=3c1cdbbea964&tochange=2b61af9d18ee Regression window(m-i) Last good: http://hg.mozilla.org/integration/mozilla-inbound/rev/e2db006298c2 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0a1) Gecko/20120206 Firefox/13.0a1 ID:20120206185103 First bad: http://hg.mozilla.org/integration/mozilla-inbound/rev/fa92500f4ad2 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0a1) Gecko/20120206 Firefox/13.0a1 ID:20120206191607 Pushlog: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=e2db006298c2&tochange=fa92500f4ad2 Triggered by: Bug 720467
Maybe duplication of bug 724476
Probably a dup of bug 725747, which has a patch.
The try server build( https://bugzilla.mozilla.org/show_bug.cgi?id=725747#c10 ) does not help.
(In reply to Chris Jones [:cjones] [:warhammer] from comment #2) > Probably a dup of bug 725747, which has a patch. The try server build on https://bugzilla.mozilla.org/show_bug.cgi?id=725747#c14 and https://bugzilla.mozilla.org/show_bug.cgi?id=725747#c15 , both nothing changed.
Jeff, can you reproduce this on Windows, or is this again a driver-specific issue?
Alice, what doess about:support -> Graphics say?
Graphics Adapter Description : ATI Radeon HD 4300/4500 Series Vendor ID : 0x1002 Device ID : 0x954f Adapter RAM : 512 Adapter Drivers : aticfx64 aticfx64 aticfx32 aticfx32 atiumd64 atidxx64 atiumdag atidxx32 atiumdva atiumd6a atitmm64 Driver Version : 8.930.0.0 Driver Date : 12-5-2011 Direct2D Enabled : true DirectWrite Enabled : true (6.1.7601.17563) ClearType Parameters : Gamma: 2200 Pixel Structure: RGB ClearType Level: 50 Enhanced Contrast: 200 WebGL Renderer : Google Inc. -- ANGLE (ATI Radeon HD 4300/4500 Series) -- OpenGL ES 2.0 (ANGLE 1.0.0.963) GPU Accelerated Windows : 1/1 Direct3D 10 AzureBackend : direct2d
I believe this is actually bug 724476, but the fix for bug 720467 triggered similar performance problems as XP has in bug 720467.
Assignee: nobody → jgilbert
Status: NEW → ASSIGNED
(In reply to Jeff Gilbert [:jgilbert] from comment #8) > I believe this is actually bug 724476, but the fix for bug 720467 triggered > similar performance problems as XP has in bug 720467. The try server build( https://bugzilla.mozilla.org/show_bug.cgi?id=724476#c18 ) 35fps / 20fish still slower than prior landing of Bug 720467 60fps / 20fish
Is there any regression when rendering with native gl? (webgl.prefer-native-gl=true)
Depends on: 724476
(In reply to Jeff Gilbert [:jgilbert] from comment #10) > Is there any regression when rendering with native gl? > (webgl.prefer-native-gl=true) under condition of webgl.prefer-native-gl=true prior landing of Bug 720467 : 45-47fps / 20fish after landing of Bug 720467 : 45-47fps / 20fish The try server build( https://bugzilla.mozilla.org/show_bug.cgi?id=724476#c18 ): 44-45fps / 20fish
In addition to Comment #0 In local build: Last Good Changeset : e2db006298c2 First Bad Changeset : 7f6db301b290 This is regressed by 7f6db301b290 Joe Drew — Bug 720467 - Switch to using FBOs for WebGL, and support sharing textures between EGL contexts. r=jgilbert
FYI,The performance is back when I simply backed 7f6db301b290 out from tip.
How is performance on trunk now that I've fixed bug 725747?
(In reply to Joe Drew (:JOEDREW!) from comment #14) > How is performance on trunk now that I've fixed bug 725747? http://hg.mozilla.org/mozilla-central/rev/f88a05e00f47 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0a1) Gecko/20120217 Firefox/13.0a1 ID:20120217034049 Graphics section(see same as Comment#7) 18-20fps/20fish --- Nothing changed.
I am investigating if FBOs are 'being slow' for some reason on ANGLE.
I think I know what's up, pending tests results from a build. I think ANGLE is no longer supplying a d3d handle to d3d layers, and the fallback (which should never happen) is /slow/.
tracking-firefox13: ? → ---
I was correct, we were not using d3d share handles, resulting in a read-back-and-forth. Patch forthcoming.
Blocks: 729272
Comment on attachment 599319 [details] [diff] [review] Fix logic for d3d share handles and EGL PBuffer creation Review of attachment 599319 [details] [diff] [review]: ----------------------------------------------------------------- R+, but the code paths between pbuffers and not pbuffers keep making this code very difficult to understand. Are we going to completely get rid of the pbuffer paths? if yes please file a followup bug for that.
Attachment #599319 - Flags: review?(bjacob) → review+
Try running at: https://tbpl.mozilla.org/?tree=Try&rev=436a5bc4aa91 Follow up bug for removing PBuffer support at Bug 729285.
Backed out, on suspicion of causing webgl conformance test failures: https://hg.mozilla.org/integration/mozilla-inbound/rev/29ee49bff6a1
Target Milestone: mozilla13 → ---
There was a crippling bug in the logic for blitting between the draw and read FBOs: With PBuffers, the read FBO is 0, and we naively use our BindDrawFBO when we swap our draw/read buffers for the blit. BUT, our BindDrawFBO call emulates '0' as 'mOffscreenDrawFBO', which is not what we want at all.
Attachment #599319 - Attachment is obsolete: true
Attachment #599864 - Flags: review?(bjacob)
I should note that this issue caused the blitting to fail, resulting in an empty canvas, triggering a huge number of mochitest fails on both win7 (EGL PBuffers) and winxp (WGL PBuffers still, surprisingly).
(In reply to Jeff Gilbert [:jgilbert] from comment #26) > Try build running at: > https://tbpl.mozilla.org/?tree=Try&rev=58a248914434 The try build works well.
Comment on attachment 599864 [details] [diff] [review] Fix logic for d3d share handles and EGL PBuffer creation Review of attachment 599864 [details] [diff] [review]: ----------------------------------------------------------------- I think that all user-facing parts of GLContext, that map 0 to the actual buffers, should be renamed to mention 'User'. So one wouldn't accidentally use one when the other is meant. Makes sense?
Attachment #599864 - Flags: review?(bjacob) → review+
Just downloaded the latest hourly build, and its worse here for me: 2fps, 20 fish 1680x922 Adapter DescriptionATI Radeon HD 3200 GraphicsVendor ID0x1002Device ID0x9610Adapter RAM256Adapter Driversaticfx64 aticfx64 aticfx32 aticfx32 atiumd64 atidxx64 atiumdag atidxx32 atiumdva atiumd6a atitmm64Driver Version8.881.0.0Driver Date7-28-2011Direct2D EnabledtrueDirectWrite Enabledtrue (6.1.7601.17563)ClearType ParametersDISPLAY1 [ Gamma: 2200 Pixel Structure: RGB ClearType Level: 100 Enhanced Contrast: 300 ] DISPLAY4 [ Gamma: 2200 Pixel Structure: RGB ClearType Level: 100 Enhanced Contrast: 50 ] WebGL RendererGoogle Inc. -- ANGLE (ATI Radeon HD 3200 Graphics) -- OpenGL ES 2.0 (ANGLE 1.0.0.963)GPU Accelerated Windows1/1 Direct3D 10 AzureBackenddirect2d tested cset: https://hg.mozilla.org/mozilla-central/rev/15d7708672c1
Today'nightly is Landed Bug 724476,and this is slightly improved, But still slower than prior landing of Bug 720467(60fps / 20fish)(986x895) 45fps / 20fish http://hg.mozilla.org/mozilla-central/rev/ce20e9b47e9c Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20120225 Firefox/13.0a1 ID:20120225031723
This patch will be needed to bring perf back up to what it was before 720467, though it's great that 724476 improved it markedly. I am trying to fix this properly, but I may have to do a minimal fix (at first) to hit a release window. The more proper fix is hitting issues, still.
Summary: Huge performance regression on WebGL → Huge performance regression on WebGL with ANGLE+D3D10 since Fx13
Summary: Huge performance regression on WebGL with ANGLE+D3D10 since Fx13 → Huge performance regression on WebGL with ANGLE+D3D10
Comment on attachment 599864 [details] [diff] [review] Fix logic for d3d share handles and EGL PBuffer creation Still working on the try fails. We're getting FBO binding prediction mismatches.
Attachment #599864 - Flags: review-
Try run: https://tbpl.mozilla.org/?tree=Try&rev=59c2c9933ac8 Looks like some bad-slaves, but clean otherwise. This also cleans up our User/Internal FBO separation, which was causing FBO mispredictions on linux.
Attachment #599864 - Attachment is obsolete: true
Attachment #603227 - Flags: review?(bjacob)
Comment on attachment 603227 [details] [diff] [review] Fix logic for d3d share handles and EGL PBuffer creation Review of attachment 603227 [details] [diff] [review]: ----------------------------------------------------------------- Self r- with notes for tomorrow. bjacob, thoughts on my notes? There are a couple that are questions. ::: gfx/gl/GLContext.h @@ +953,5 @@ > > + bool abort = false; > + > + if (mInternalsBindingDrawFBO) { > + printf_stderr("!!! Draw FBO bound internally! (current internal: %d, user: %d)\n", ret, mUserBoundDrawFBO); These should maybe use a more standard error format? Also possibly NS_ERROR with printf_stderr for extra info. @@ +1068,3 @@ > return prev; > } > +/* Forgot to delete these unused functions. BindUserDraw/ReadFBO(0) is a better solution. @@ +1150,5 @@ > // flip read/draw for blitting > + GLuint prevDraw = GetUserDrawFBO(); > + GLuint prevRead = GetUserReadFBO(); > + > + NS_ASSERTION(SupportsOffscreenSplit(), "Doesn't support offscreen split?"); We should probably assert this during init and assume it thereafter. @@ +1152,5 @@ > + GLuint prevRead = GetUserReadFBO(); > + > + NS_ASSERTION(SupportsOffscreenSplit(), "Doesn't support offscreen split?"); > + > + BindInternalDrawFBO(mOffscreenReadFBO); Possibly add a note about these two bindings are 'internal', and how it puts the bind points into 'internal' mode. @@ +1162,5 @@ > 0, 0, width, height, > LOCAL_GL_COLOR_BUFFER_BIT, > LOCAL_GL_NEAREST); > > + BindUserDrawFBO(prevDraw); Add comment regarding how binding a 'user' FBO brings us out of 'internal' mode. ::: gfx/gl/GLContextProviderGLX.cpp @@ +781,1 @@ > return IsExtensionSupported("GL_EXT_framebuffer_object"); The order of these lines seems a little backwards.
Attachment #603227 - Flags: review?(bjacob) → review-
(In reply to Jeff Gilbert [:jgilbert] from comment #33) > Created attachment 603227 [details] [diff] [review] > Fix logic for d3d share handles and EGL PBuffer creation > > Try run: This try build fails to initialize WebGL. http://hg.mozilla.org/try/rev/59c2c9933ac8 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20120305 Firefox/13.0a1 Graphics Adapter Description : ATI Radeon HD 4300/4500 Series Vendor ID : 0x1002 Device ID : 0x954f Adapter RAM : 512 Adapter Drivers : aticfx64 aticfx64 aticfx32 aticfx32 atiumd64 atidxx64 atiumdag atidxx32 atiumdva atiumd6a atitmm64 Driver Version : 8.930.0.0 Driver Date : 12-5-2011 Direct2D Enabled : true DirectWrite Enabled : true (6.1.7601.17563) ClearType Parameters : Gamma: 2200 Pixel Structure: RGB ClearType Level: 50 Enhanced Contrast: 200 WebGL Renderer : false GPU Accelerated Windows : 1/1 Direct3D 10 AzureBackend : direct2d
Is there any DEBUG spew around where it fails initialization?
(In reply to Jeff Gilbert [:jgilbert] from comment #36) > Is there any DEBUG spew around where it fails initialization? Sorry,I do not know how to DEBUG WebGL. Error console: Timestamp: 2012/03/06 21:41:32 Warning: WebGL: Can't get a usable WebGL context Source File: http://people.mozilla.org/~jmuizelaar/fishie/fishie-fast.html Line: 47
Attached file console log —
Console log of DEBUG build attached ( download from /pub/mozilla.org/firefox/try-builds/jgilbert@mozilla.com-59c2c9933ac8/try-win32-debug/firefox-13.0a1.en-US.win32.zip )
Blocks: 723444
Updated as per self-review, and added more NS_WARNINGs for various failure situations.
Attachment #603227 - Attachment is obsolete: true
Attachment #603508 - Flags: review?(bjacob)
Got a logic backwards in for GLX.
Attachment #603508 - Attachment is obsolete: true
Attachment #603508 - Flags: review?(bjacob)
Attachment #603534 - Flags: review?(bjacob)
Forgot to qfold. ><
Attachment #603534 - Attachment is obsolete: true
Attachment #603534 - Flags: review?(bjacob)
Attachment #603537 - Flags: review?(bjacob)
Comment on attachment 603537 [details] [diff] [review] Fix logic for d3d share handles and EGL PBuffer creation Review of attachment 603537 [details] [diff] [review]: ----------------------------------------------------------------- Please write a quick explanation of what was wrong and how your patch fixes it.
(In reply to Jeff Gilbert [:jgilbert] from comment #43) > New try: > https://tbpl.mozilla.org/?tree=Try&rev=d7fc1cf501d2 This try build works very well. And I got 60fps / 20fish(986x895px) ;)
(In reply to Benoit Jacob [:bjacob] from comment #44) > Comment on attachment 603537 [details] [diff] [review] > Fix logic for d3d share handles and EGL PBuffer creation > > Review of attachment 603537 [details] [diff] [review]: > ----------------------------------------------------------------- > > Please write a quick explanation of what was wrong and how your patch fixes > it. There are a few things going on in this patch. The main one is that the logic behind bug 720467 is incorrect: EGL *must* use PBuffers on windows to provide an EGLSurface, which is what the ANGLE interop works with. No PBuffers, no interop. Further, some of the logic involved in EGL with and without PBuffers was not quite correct. Bug 720467 also added some incorrect logic to GetD3DShareHandle(), which has been removed. With that fixed, I was hitting errors on Linux Debug builds, because we were hitting internal bound FBO mispredictions. One of the major things this patch adds is a distinction between working with the semi-emulated 'user' bindings (where binding 0 actually binds to the offscreen buffer) and manual 'internal' bindings, which is needed for blitting between the offscreen buffers. I included checks (on debug) to makes sure we never fail to rebind to 'user' bindings after going into what I call 'internal mode'. We should never call raw_fBindFramebuffer in the general case, as it will kill our bind shadowing. For extra care, I initialize GLContexts to be /in internal mode/, so we must be sure we 'user'-bind to 0 with InitFramebuffers(). This assures that we are sure our 'user' bindings are always correct.
Comment on attachment 603537 [details] [diff] [review] Fix logic for d3d share handles and EGL PBuffer creation Review of attachment 603537 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/gl/GLContext.cpp @@ +1161,5 @@ > const bool useDepthStencil = > !mIsGLES2 || IsExtensionSupported(OES_packed_depth_stencil); > > // save a few things for later restoring > + curBoundFramebufferDraw = GetUserDrawFBO(); As discussed on IRC, I would prefer to keep 'Bound' in the name, as it makes it clear that what it returns is a current binding, as opposed to just something you happen to have in your sleeve. ::: gfx/gl/GLContext.h @@ +939,5 @@ > } > > +#ifdef DEBUG > + bool mInternalsBindingDrawFBO; > + bool mInternalsBindingReadFBO; I could have used a comment to understand this stuff and a better name for these variables. Especially the fact that binding is what switches mode. @@ +967,4 @@ > } > + > + if (abort) > + NS_ABORT(); Clever, so we get to see both error messages. @@ +1135,5 @@ > + // Store current bindings for restoring later > + GLuint prevDraw = GetUserDrawFBO(); > + GLuint prevRead = GetUserReadFBO(); > + > + NS_ASSERTION(SupportsOffscreenSplit(), "Doesn't support offscreen split?"); What happens if this assertion fails? should this be a hard assertion like NS_ABORT_IF_FALSE? @@ +1206,5 @@ > AfterGLReadCall(); > } > > void ForceDirtyFBOs() { > + GLuint draw = SwapUserReadFBO(0); There seems to be a bug here, that preexists your patch: this code looks like it wanted SwapUserDrawFBO. @@ +1217,4 @@ > } > > void BlitDirtyFBOs() { > + GLuint read = SwapUserReadFBO(0); This looks OK, but given what we just found in ForceDirtyFBO, please proof-read this function again. @@ +1624,5 @@ > printf_stderr("Requested level of multisampling is unavailable, continuing without multisampling\n"); > } > > + if (ResizeOffscreenFBO(aSize, aUseReadFBO, true)) > + return true; It's a bit confusing that this is a early-success return, while above we have early-fail returns. @@ +1626,5 @@ > > + if (ResizeOffscreenFBO(aSize, aUseReadFBO, true)) > + return true; > + > + NS_WARNING("ResizeOffscreenFBO failed to resize AA context with AA disabled!"); "with AA disabled" is a bit confusing, it seems to refer to a preference or blacklisting or something. Maybe "even after falling back to non-AA"?
Attachment #603537 - Flags: review?(bjacob) → review-
(In reply to Benoit Jacob [:bjacob] from comment #47) > Comment on attachment 603537 [details] [diff] [review] > Fix logic for d3d share handles and EGL PBuffer creation > > Review of attachment 603537 [details] [diff] [review]: > ----------------------------------------------------------------- > > @@ +1624,5 @@ > > printf_stderr("Requested level of multisampling is unavailable, continuing without multisampling\n"); > > } > > > > + if (ResizeOffscreenFBO(aSize, aUseReadFBO, true)) > > + return true; > > It's a bit confusing that this is a early-success return, while above we > have early-fail returns. > Here we actually have early-succeed for the previous ResizeOffscreenFBO(), but in the case of a non-AA canvas, there's no use retrying without AA, so we fail out. I would prefer to keep it as is.
Previous patch was bad. New patch, new try: https://tbpl.mozilla.org/?tree=Try&rev=1d99b9d7cc4d
Attachment #603905 - Attachment is obsolete: true
Attachment #603905 - Flags: review?(bjacob)
Attachment #603948 - Flags: review?(bjacob)
(In reply to Jeff Gilbert [:jgilbert] from comment #50) > Created attachment 603948 [details] [diff] [review] > Fix logic for d3d share handles, EGL PBuffer creation, and user/internal FBO > bindings > > Previous patch was bad. > New patch, new try: > https://tbpl.mozilla.org/?tree=Try&rev=1d99b9d7cc4d This try build also works very well.
Attachment #603948 - Flags: review?(bjacob) → review+
https://hg.mozilla.org/mozilla-central/rev/6610af68b24c
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Depends on: 769949
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: