Bug 38560 - WebGL demos show bad flicker
Summary: WebGL demos show bad flicker
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: 528+ (Nightly build)
Hardware: All OS X 10.6
: P1 Normal
Assignee: Zhenyao Mo
URL: http://plopbyte.net/?page_id=111/
Keywords:
Depends on: 33416 36908
Blocks:
  Show dependency treegraph
 
Reported: 2010-05-04 18:33 PDT by Simon Fraser (smfr)
Modified: 2010-06-18 13:51 PDT (History)
4 users (show)

See Also:


Attachments
patch (1.37 KB, patch)
2010-06-18 13:32 PDT, Zhenyao Mo
simon.fraser: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2010-05-04 18:33:20 PDT
With a recent nightly, these two WebGL demos show bad flickering:

http://plopbyte.net/?page_id=111/
http://www.cubicvr.org/CubicVR.js/LandscapeTest1.html
Comment 1 Simon Fraser (smfr) 2010-05-04 18:43:39 PDT
My graphics hardware:


ATI Radeon HD 2600 XT:

  Chipset Model:	ATI Radeon HD 2600
  Type:	GPU
  Bus:	PCIe
  Slot:	Slot-1
  PCIe Lane Width:	x16
  VRAM (Total):	256 MB
  Vendor:	ATI (0x1002)
  Device ID:	0x9588
  Revision ID:	0x0000
  ROM Revision:	113-B1480A-252
  EFI Driver Version:	01.00.252
  Displays:
Cinema HD Display:
  Resolution:	1920 x 1200
  Pixel Depth:	32-Bit Color (ARGB8888)
  Main Display:	Yes
  Mirror:	Off
  Online:	Yes
  Rotation:	Supported
DELL 2005FPW:
  Resolution:	1680 x 1050 @ 60 Hz
  Pixel Depth:	32-Bit Color (ARGB8888)
  Mirror:	Off
  Online:	Yes
  Rotation:	Supported
Comment 2 Simon Fraser (smfr) 2010-05-04 18:45:43 PDT
This is is particularly bad:
http://www.cubicvr.org/CubicVR.js/ObjectCompositeTest.html
Comment 3 Simon Fraser (smfr) 2010-05-28 17:03:13 PDT
This regressed in http://trac.webkit.org/changeset/56872
Performance in general on my machine is much worse after this change.
Comment 4 Zhenyao Mo 2010-05-28 17:16:33 PDT
Can you run the demo with {antialias: false} for context attributes?  And what's the performance like?
Comment 5 Zhenyao Mo 2010-05-28 17:19:19 PDT
Forget about the previous comments.  I see your card is ATI, for which the antialias is false.
Comment 6 Simon Fraser (smfr) 2010-05-28 17:23:40 PDT
For example, when http://www.cubicvr.org/CubicVR.js/ObjectCompositeTest.html is loading,  entire parts of the scene, like the walls and columns, flicker in and out, and the animation overall is jerky. That didn't happen prior to r56872.
Comment 7 Zhenyao Mo 2010-05-28 17:35:37 PDT
I had a quick look at the patch.  Seems like the difference (for your machine) is the stencil is on.

Can you try the demo with {stencil: false} in context creation and let me know the performance?
Comment 8 Chris Marrin 2010-05-29 09:50:25 PDT
This also happens for me on two different machines, both of which have different ATI chipsets. It looks like your logic for deciding how to deal with the textures is flipping frame-to-frame or something like that.

This needs to be fixed ASAP. If you can't determine the root cause, please disable whatever is causing the flicker on ATI cards in Safari.
Comment 9 Zhenyao Mo 2010-05-29 14:04:31 PDT
I don't have access to a machine with ATI card, so if anyone with ATI card can run the demo with {stensil: false} at content creation and let me know if it solves the performance issue, then I'll know what to do.
Comment 10 Zhenyao Mo 2010-06-02 17:17:47 PDT
See https://bugs.webkit.org/show_bug.cgi?id=40090.

Hopefully the landing of a patch there will resolve this issue.
Comment 11 Chris Marrin 2010-06-03 07:06:33 PDT
Yes, that patch may resolve the issue, but I assume the problem would return if you turn on stencil in your content. So we really need to understand why stencil being on causes the flicker
Comment 12 Zhenyao Mo 2010-06-03 11:11:28 PDT
(In reply to comment #11)
> Yes, that patch may resolve the issue, but I assume the problem would return if you turn on stencil in your content. So we really need to understand why stencil being on causes the flicker

I tried on kbr's Mac which has ATI card and couldn't reproduce the flicking.

When the patch lands and either you or Simon can confirm it does resolve the issue, then we should file a driver bug to Apple.
Comment 13 Zhenyao Mo 2010-06-03 11:17:32 PDT
(In reply to comment #11)
> Yes, that patch may resolve the issue, but I assume the problem would return if you turn on stencil in your content. So we really need to understand why stencil being on causes the flicker

One thing we could do is use something other than GL_DEPTH24_STENCIL8_EXT for certain ATI cards when a user requests stencil buffer.
Comment 14 Zhenyao Mo 2010-06-04 16:18:50 PDT
Patch for https://bugs.webkit.org/show_bug.cgi?id=40090 landed last night.  Please verify if the flicking is gone.  We couldn't reproduce it on our side.
Comment 15 Simon Fraser (smfr) 2010-06-11 16:50:24 PDT
There is no improvement in the flickering.
Comment 16 Simon Fraser (smfr) 2010-06-11 16:53:11 PDT
http://www.cubicvr.org/CubicVR.js/ObjectCompositeTest.html doesn't work at all for me in a recent nightly.
Comment 17 Zhenyao Mo 2010-06-14 15:27:17 PDT
(In reply to comment #15)
> There is no improvement in the flickering.

That's really weird.  Because with antialias==false and stencil==false, WebGL should be the same with/without http://trac.webkit.org/changeset/56872.
Comment 18 Zhenyao Mo 2010-06-14 15:28:36 PDT
(In reply to comment #17)
> (In reply to comment #15)
> > There is no improvement in the flickering.
> 
> That's really weird.  Because with antialias==false and stencil==false, WebGL should be the same with/without http://trac.webkit.org/changeset/56872.

(In reply to comment #16)
> http://www.cubicvr.org/CubicVR.js/ObjectCompositeTest.html doesn't work at all for me in a recent nightly.

This is also weird.  I just synced with the codebase and built webkit.  The demo worked fine on my side.
Comment 19 Kenneth Russell 2010-06-14 15:56:56 PDT
FYI, we're trying to schedule some time in the Apple Compatibility Lab to diagnose this on the hardware in questions.
Comment 20 Kenneth Russell 2010-06-18 12:04:32 PDT
zmo and I tracked down the regression to the moving of the ensureContext call in GraphicsContext3D::prepareTexture (GraphicsContext3DMac.mm) into the "if (m_attrs.antialias)" block in bug 33416. Bug 36908 fixed part of this regression (the moving of a glFinish call into the same block) but not this part. svn diff follows; a patch will be filed shortly.



Index: WebCore/platform/graphics/mac/GraphicsContext3DMac.mm
===================================================================
--- WebCore/platform/graphics/mac/GraphicsContext3DMac.mm	(revision 61422)
+++ WebCore/platform/graphics/mac/GraphicsContext3DMac.mm	(working copy)
@@ -383,8 +383,8 @@
 
 void GraphicsContext3D::prepareTexture()
 {
+    ensureContext(m_contextObj);
     if (m_attrs.antialias) {
-        ensureContext(m_contextObj);
         ::glBindFramebufferEXT(GL_READ_FRAMEBUFFER_EXT, m_multisampleFBO);
         ::glBindFramebufferEXT(GL_DRAW_FRAMEBUFFER_EXT, m_fbo);
         ::glBlitFramebufferEXT(0, 0, m_currentWidth, m_currentHeight, 0, 0, m_currentWidth, m_currentHeight, GL_COLOR_BUFFER_BIT, GL_LINEAR);
Comment 21 Zhenyao Mo 2010-06-18 13:32:39 PDT
Created attachment 59149 [details]
patch
Comment 22 Kenneth Russell 2010-06-18 13:44:36 PDT
Looks good.
Comment 23 Zhenyao Mo 2010-06-18 13:47:11 PDT
Committed manually: http://trac.webkit.org/changeset/61432
Comment 24 Eric Seidel (no email) 2010-06-18 13:51:37 PDT
Please copy the ChangeLog into the commit message.  Tools like "webkit-patch land" can help do that for you.