Bug 210992 – REGRESSION (r259589): Google Maps background layer fades out and back in

• Status: RESOLVED
• Resolution: FIXED
• Component: WebGL

[Dean Jackson]

* SUMMARY
- Google Maps background layer fades out and back in

* STEPS TO REPRODUCE
1. Navigate to https://maps.google.com/
2. Review the loading experience

(this was an internal report so I'm trying to get a public screen recording)

[Dean Jackson]

rdar://61907344

[Dean Jackson]

Note this is with iOS hardware. I'm not sure it reproduces in the simulator yet.

[Dean Jackson]

Created attachment 397520 [details]
Screen recording

[Dean Jackson]

Created attachment 397521 [details]
Screen recording

[David Kilzer (:ddkilzer)]

The content of attachment 397520 [details] has been deleted for the following reason:

Wrong file uploaded.

[Kenneth Russell]

Trac link is hard to click in description:
http://trac.webkit.org/r259589

[Kenneth Russell]

Apparent regression from Bug 209689.

[Dean Jackson]

I can confirm that maps is getting WebGL, and the page works fine after it has loaded. There is just a black flash at the beginning.

Nothing in the console either.

[Kenneth Russell]

Note: the most recent ANGLE roll into WebKit should have picked up the fix for this ANGLE request for enhancement:
https://bugs.chromium.org/p/angleproject/issues/detail?id=4451

so the ANGLE_depth_texture extension should be being advertised correctly by ANGLE and picked up in WebKit's WebGL implementation. This was the cause of the most recent Maps rendering bug with the ANGLE backend, Bug 208331.

[Dean Jackson]

ANGLE initialised Major: 1 Minor: 4
Extensions: EGL_KHR_create_context EGL_EXT_device_query EGL_KHR_get_all_proc_addresses EGL_ANGLE_create_context_webgl_compatibility EGL_CHROMIUM_create_context_bind_generates_resource EGL_EXT_pixel_format_float EGL_KHR_surfaceless_context EGL_ANGLE_display_texture_share_group EGL_ANGLE_create_context_client_arrays EGL_ANGLE_program_cache_control EGL_ANGLE_robust_resource_initialization EGL_ANGLE_iosurface_client_buffer EGL_ANGLE_create_context_extensions_enabled EGL_ANDROID_blob_cache EGL_ANDROID_recordable EGL_ANGLE_create_context_backwards_compatible EGL_KHR_create_context_no_error
Got EGLConfig
Got EGLContext
Created a GraphicsContextGLOpenGL (0x15730c3e0).
GraphicsContextGLOpenGL::allocateIOSurfaceBackingStore at 300 x 150. (0x15730c3e0)
GraphicsContextGLOpenGL::updateFramebufferTextureBackingStoreFromLayer(). (0x15730c3e0)
GraphicsContextGLOpenGL::allocateIOSurfaceBackingStore at 2048 x 150. (0x15730c3e0)
GraphicsContextGLOpenGL::updateFramebufferTextureBackingStoreFromLayer(). (0x15730c3e0)
GraphicsContextGLOpenGL::allocateIOSurfaceBackingStore at 2048 x 1396. (0x15730c3e0)
GraphicsContextGLOpenGL::updateFramebufferTextureBackingStoreFromLayer(). (0x15730c3e0)

[Kenneth Russell]

Those are the EGL extensions; could you also print out the GL extensions on the device? (The result of glGetString(GL_EXTENSIONS).

[Dean Jackson]

WebGLRenderingContextBase::setupFlags() had the following
#if USE(ANGLE)
    m_isGLES2NPOTStrict = true;
    m_isDepthStencilSupported = true;
#endif

I've replaced that by 

m_isDepthStencilSupported = m_context->getExtensions().isEnabled("GL_OES_packed_depth_stencil");

and confirmed the extension is enabled.

The actual list of extensions (of the GraphicsContextGL) is:

GL_ANGLE_client_arrays GL_ANGLE_program_cache_control GL_ANGLE_request_extension GL_ANGLE_robust_client_memory GL_ANGLE_robust_resource_initialization GL_ANGLE_translated_shader_source GL_ANGLE_webgl_compatibility GL_CHROMIUM_bind_generates_resource GL_CHROMIUM_bind_uniform_location GL_CHROMIUM_copy_texture GL_EXT_debug_marker GL_EXT_discard_framebuffer GL_KHR_debug GL_NV_fence GL_OES_depth24 GL_OES_packed_depth_stencil GL_OES_surfaceless_context

Checking the ANGLE context now.

[Dean Jackson]

OpenGL Extensions: GL_ANGLE_client_arrays GL_ANGLE_program_cache_control GL_ANGLE_request_extension GL_ANGLE_robust_client_memory GL_ANGLE_robust_resource_initialization GL_ANGLE_translated_shader_source GL_ANGLE_webgl_compatibility GL_CHROMIUM_bind_generates_resource GL_CHROMIUM_bind_uniform_location GL_CHROMIUM_copy_texture GL_EXT_debug_marker GL_EXT_discard_framebuffer GL_KHR_debug GL_NV_fence GL_OES_depth24 GL_OES_packed_depth_stencil GL_OES_surfaceless_context

[Dean Jackson]

This took longer than I hoped, but GenerateCaps in ANGLE sees this:

GL_DEPTH_COMPONENT16 yes
GL_DEPTH_COMPONENT32_OES no
GL_DEPTH24_STENCIL8_OES yes

[Dean Jackson]

If I remove the requirement for GL_DEPTH_COMPONENT32_OES then i do get the depth textures, but it doesn't fix the maps issue.

[Kenneth Russell]

One thought: looking at the specific conformance failures for:
webgl/1.0.3/conformance/textures/tex-image-and-sub-image-2d-with-canvas.html

from Bug 207858, it looks like flip-Y canvas-to-WebGL-texture uploads might be broken in the ANGLE path for some reason. That could possibly explain why the initial background is black.

[James Darpinian]

I'm looking at this now. First I'm fixing bug 211398.

The reason why the map appears at first and fades to black is that the map is loaded as img tags before WebGL is initialized, and there is a fade transition when switching to WebGL. I'm pretty sure that the black map is related to the progressive antialiasing feature, which renders the map to a offscreen render target before copying it to the screen. The symptoms are consistent with the first frame not being rendered to the offscreen render target. Maybe a bindFramebuffer call is failing for some reason.

[Dean Jackson]

ANGLE produces an error: Feedback loop formed between Framebuffer and active Texture.

I confirmed the issue goes away when commenting out ANGLE's feedback detection.

[Dean Jackson]

(In reply to Dean Jackson from comment #18)
> ANGLE produces an error: Feedback loop formed between Framebuffer and active
> Texture.
> 
> I confirmed the issue goes away when commenting out ANGLE's feedback
> detection.

There are two places where this can be triggered in ANGLE. This one in in ValidateDrawStates.

Comes from FramebufferState::updateHasRenderingFeedbackLoop()

Happens when trying to draw what appears to be a quad. The incoming code is obfuscated, but the shaders are:

precision highp float;
varying vec2 a;
uniform vec2 b,d,e;
uniform float c,f;
attribute vec2 g;
void main(){
  vec2 h,i;
  h=g*b*c+e;
  i=2.*(h/d)-1.;
  i.y*=f;
  gl_Position=vec4(i,1,1);
  a=g;
}

precision highp float;
varying vec2 a;
uniform sampler2D h;
uniform float i,j;
void main(){
  gl_FragColor = texture2D(h,a);
  gl_FragColor.a = gl_FragColor.a*i+j;
}

[Dean Jackson]

Feedback loop detected in
[WebGLLayer display]
at
gl::BindTexture(WebCore::GraphicsContextGL::IOSurfaceTextureTarget, texture)

because it thinks texture is already bound as the framebuffer texture

[Kenneth Russell]

Great find Dean.

James tried saving/restoring the WebCore::GraphicsContextGL::IOSurfaceTextureTarget binding point - would that help?

--------
diff --git a/Source/WebCore/platform/graphics/GraphicsContextGL.h b/Source/WebCore/platform/graphics/GraphicsContextGL.h
index ee0de95b96f..96f6d6e0fcb 100644
--- a/Source/WebCore/platform/graphics/GraphicsContextGL.h
+++ b/Source/WebCore/platform/graphics/GraphicsContextGL.h
@@ -666,7 +666,8 @@ public:
         MAX_CLIENT_WAIT_TIMEOUT_WEBGL = 0x9247,
 
         // Necessary desktop OpenGL constants.
-        TEXTURE_RECTANGLE_ARB = 0x84F5
+        TEXTURE_RECTANGLE_ARB = 0x84F5,
+        TEXTURE_BINDING_RECTANGLE_ARB  0x84F6
     };
 
     // Attempt to enumerate all possible native image formats to
@@ -787,9 +788,11 @@ public:
         // reference them.
 #if PLATFORM(MAC)
         IOSurfaceTextureTarget = TEXTURE_RECTANGLE_ARB, // also GL_TEXTURE_RECTANGLE_ANGLE
+        IOSurfaceTextureBinding = TEXTURE_BINDING_RECTANGLE_ARB, // also GL_TEXTURE_BINDING_RECTANGLE_ANGLE
         EGLIOSurfaceTextureTarget = 0x345B, // EGL_TEXTURE_RECTANGLE_ANGLE
 #else
         IOSurfaceTextureTarget = TEXTURE_2D,
+        IOSurfaceTextureBinding = TEXTURE_BINDING_2D,
         EGLIOSurfaceTextureTarget = 0x305F, // EGL_TEXTURE_2D
 #endif // PLATFORM(MAC)
     };
--------

and then in WebGLLayer.mm, something like:

GLint lastTexture = 0;
glGetIntegerv(WebCore::GraphicsContextGL::IOSurfaceTextureBinding, &lastTexture);
glBindTexture(...)
...

glBindTexture(WebCore::GraphicsContextGL::IOSurfaceTextureTarget, lastTexture);

?

One other thought is that we could try temporarily unbinding the framebuffer, but that's problematic because the code needs to know whether it's running on GLES2 or GLES3, and whether to save/restore the GL_FRAMEBUFFER or both the GL_DRAW_FRAMEBUFFER and GL_READ_FRAMEBUFFER binding points.

[Dean Jackson]

James has confirmed that restoring the texture state didn't avoid the feedback loop detection. 

Unfortunately this is a bit tricky to debug since, as far as I can tell, there are "valid" feedback loops that temporarily exist. So setting a breakpoint when they are detected is too noisy.

[Kenneth Russell]

Should we try teaching WebGLLayer about whether it's for WebGL 1.0 or 2.0, and then try clearing/restoring the GL_FRAMEBUFFER or GL_DRAW_FRAMEBUFFER / GL_READ_FRAMEBUFFER binding points?

[James Darpinian]

Created attachment 399432 [details]
Patch

[EWS Watchlist]

Note that there are important steps to take when updating ANGLE. See http://trac.webkit.org/wiki/UpdatingANGLE

[James Darpinian]

It was a bug in ANGLE's rendering feedback loop detection. The cached feedback loop state was not updated properly when sampler uniforms were set. I don't know if this patch will apply now since the ANGLE roll was reverted, but we should be able to roll forward with it, or just do a new ANGLE roll once the patch lands in ANGLE: https://chromium-review.googlesource.com/c/angle/angle/+/2203431

[Dean Jackson]

This fails to compile because we reverted the version of ANGLE that caused it in r261262.

I'll land this manually along with re-applying r259589.

[Dean Jackson]

Created attachment 399445 [details]
EWS Test

Uploading a patch to test EWS.

[Dean Jackson]

Created attachment 399469 [details]
EWS Test 2

[Dean Jackson]

Committed r261751: <https://trac.webkit.org/changeset/261751>