Wrong logic in GLSurfaceFormat::IsCompatible method
Reported by
artyo...@gmail.com,
Jun 12 2017
|
|||
Issue descriptionSteps to reproduce the problem: Call GLSurfaceFormat::IsCompatible(other) when: other.depth_bits_ == 24 other.stencil_bits_ == 8 and while this->depth_bits_ == -1 this->stencil_bits_ == -1 and it will return TRUE (meaning, these are compatible surfaces) What is the expected behavior? Must return FALSE, since 'this' surface doesn't have depth or stencil, while the 'other' does. What went wrong? The root of the issue lies in following lines: GetValue(stencil_bits_, 8) == GetValue(other.stencil_bits_, 8) && GetValue(depth_bits_, 24) == GetValue(other.depth_bits_, 24) && where constants 24 and 8 are used as 'default' values. This is wrong, since by default EGL surface doesn't have depth or stencil and those default values must be all '0'. Did this work before? No Does this work in other browsers? N/A Chrome version: M61/M60/M59/M58/M57.... Channel: dev OS Version: Flash Version: For me, this issue causes wrong EGL surface to be used in GpuCommandBufferStub::Initialize in the case of 'OwnOffscreenSurface path'. I know, you guys don't use it, but this issue may hit you differently. In any case, the IsCompatible is broken and should be fixed.
,
Jun 14 2017
Graphics Feature Status Canvas: Hardware accelerated Flash: Hardware accelerated Flash Stage3D: Hardware accelerated Flash Stage3D Baseline profile: Hardware accelerated Compositing: Hardware accelerated Multiple Raster Threads: Disabled Native GpuMemoryBuffers: Software only. Hardware acceleration disabled Rasterization: Hardware accelerated Video Decode: Hardware accelerated Video Encode: Software only, hardware acceleration unavailable VPx Video Decode: Hardware accelerated WebGL: Hardware accelerated WebGL2: Unavailable Driver Bug Workarounds clear_uniforms_before_first_program_use disable_framebuffer_cmaa max_texture_size_limit_4096 scalarize_vec_and_mat_constructor_args use_client_side_arrays_for_stream_buffers use_virtualized_gl_contexts must_use_virtualized_gl_context_for_offscreen Problems Detected MediaCodec is still too buggy to use for encoding (b/11536167): 615108 Disabled Features: accelerated_video_encode WebGL 2 is not yet ready on Android: 295792, 641635 Disabled Features: webgl2 ARM driver doesn't like uploading lots of buffer data constantly Applied Workarounds: use_client_side_arrays_for_stream_buffers The Mali-Txxx driver does not guarantee flush ordering: 154715, 10068, 269829, 294779, 285292 Applied Workarounds: use_virtualized_gl_contexts Clear uniforms before first program use on all platforms: 124764, 349137 Applied Workarounds: clear_uniforms_before_first_program_use Always rewrite vec/mat constructors to be consistent: 398694 Applied Workarounds: scalarize_vec_and_mat_constructor_args Limit max texure size to 4096 on all of Android Applied Workarounds: max_texture_size_limit_4096 Limited enabling of Chromium GL_INTEL_framebuffer_CMAA: 535198 Applied Workarounds: disable_framebuffer_cmaa Disable KHR_blend_equation_advanced until cc shaders are updated: 661715 The Mali-Txxx driver does not work correctly with non-virt contexts for offscreen surfaces: 0 Applied Workarounds: must_use_virtualized_gl_context_for_offscreen Raster is using a single thread. Disabled Features: multiple_raster_threads Native GpuMemoryBuffers have been disabled, either via about:flags or command line. Disabled Features: native_gpu_memory_buffers Version Information Data exported 6/15/2017, 9:21:04 PM Chrome version Chrome/57.0.2987.146 Operating system Android 7.0.0 Software rendering list version 12.13 Driver bug list version 9.29 ANGLE commit id c1a5d16e964a 2D graphics backend Skia/57 ae9cc5d3588d52f4b371b55845704b25d88cf06d Command Line Args --use-mobile-user-agent --top-controls-show-threshold=0.5 --top-controls-hide-threshold=0.5 --enable-logging --v=1 --vmodule=*/VR*/*=2,*/vr/*=2,*/oculus/*=2,gearvr_*=2,*/VR*=2,vr_*=2,gles2_cmd_decoder*=1,WebGL*=1 --use-mobile-user-agent --enable-pinch --enable-viewport --enable-overlay-scrollbar --validate-input-event-stream --enable-longpress-drag-selection --touch-selection-strategy=direction --disable-gpu-process-crash-limit --single-process --enable-webvr --main-frame-resizes-are-orientation-changes --disable-composited-antialiasing --ui-prioritize-in-gpu-process --profiler-timing=0 --prerender-from-omnibox=enabled --enable-dom-distiller --flag-switches-begin --flag-switches-end --disable-gpu-watchdog --supports-dual-gpus=false --gpu-driver-bug-workarounds=7,23,58,74,88,93,97 --disable-gl-extensions=GL_KHR_blend_equation_advanced GL_KHR_blend_equation_advanced_coherent --gpu-vendor-id=0x0000 --gpu-device-id=0x0000 --gpu-driver-vendor --gpu-driver-version=1. --gpu-driver-date --lang=en-US --num-raster-threads=1 --enable-gpu-rasterization --gpu-rasterization-msaa-sample-count=4 --enable-main-frame-before-activation --content-image-texture-target=0,0,3553;0,1,3553;0,2,3553;0,3,3553;0,4,3553;0,5,3553;0,6,3553;0,7,3553;0,8,3553;0,9,3553;0,10,3553;0,11,3553;0,12,3553;0,13,3553;0,14,3553;0,15,3553;1,0,3553;1,1,3553;1,2,3553;1,3,3553;1,4,3553;1,5,3553;1,6,3553;1,7,3553;1,8,3553;1,9,3553;1,10,3553;1,11,3553;1,12,3553;1,13,3553;1,14,3553;1,15,3553;2,0,3553;2,1,3553;2,2,3553;2,3,3553;2,4,3553;2,5,3553;2,6,3553;2,7,3553;2,8,3553;2,9,3553;2,10,3553;2,11,3553;2,12,3553;2,13,3553;2,14,3553;2,15,3553;3,0,3553;3,1,3553;3,2,3553;3,3,3553;3,4,3553;3,5,3553;3,6,3553;3,7,3553;3,8,3553;3,9,3553;3,10,3553;3,11,3553;3,12,3553;3,13,3553;3,14,3553;3,15,3553 --disable-webrtc-hw-vp8-encoding Driver Information Initialization time 229 In-process GPU true Passthrough Command Decoder false Sandboxed false GPU0 VENDOR = 0x0000 [ARM], DEVICE= 0x0000 [Mali-T760] Optimus false AMD switchable false Driver vendor Driver version 1. Driver date Pixel shader version 3.20 Vertex shader version 3.20 Max. MSAA samples 8 Machine model name SM-G920F Machine model version GL_VENDOR ARM GL_RENDERER Mali-T760 GL_VERSION OpenGL ES 3.2 v1.r15p0-00rel0.68b65ac7cf15907aeb95fa944f39eef2 GL_EXTENSIONS GL_EXT_debug_marker GL_ARM_rgba8 GL_ARM_mali_shader_binary GL_OES_depth24 GL_OES_depth_texture GL_OES_depth_texture_cube_map GL_OES_packed_depth_stencil GL_OES_rgb8_rgba8 GL_EXT_read_format_bgra GL_OES_compressed_paletted_texture GL_OES_compressed_ETC1_RGB8_texture GL_OES_standard_derivatives GL_OES_EGL_image GL_OES_EGL_image_external GL_OES_EGL_image_external_essl3 GL_OES_EGL_sync GL_OES_texture_npot GL_OES_vertex_half_float GL_OES_required_internalformat GL_OES_vertex_array_object GL_OES_mapbuffer GL_EXT_texture_format_BGRA8888 GL_EXT_texture_rg GL_EXT_texture_type_2_10_10_10_REV GL_OES_fbo_render_mipmap GL_OES_element_index_uint GL_EXT_shadow_samplers GL_OES_texture_compression_astc GL_KHR_texture_compression_astc_ldr GL_KHR_texture_compression_astc_hdr GL_KHR_texture_compression_astc_sliced_3d GL_KHR_debug GL_EXT_occlusion_query_boolean GL_EXT_disjoint_timer_query GL_EXT_blend_minmax GL_EXT_discard_framebuffer GL_OES_get_program_binary GL_OES_texture_3D GL_EXT_texture_storage GL_EXT_multisampled_render_to_texture GL_OES_surfaceless_context GL_OES_texture_stencil8 GL_EXT_shader_pixel_local_storage GL_ARM_shader_framebuffer_fetch GL_ARM_shader_framebuffer_fetch_depth_stencil GL_ARM_mali_program_binary GL_EXT_sRGB GL_EXT_sRGB_write_control GL_EXT_texture_sRGB_decode GL_EXT_texture_sRGB_R8 GL_EXT_texture_sRGB_RG8 GL_KHR_blend_equation_advanced GL_KHR_blend_equation_advanced_coherent GL_OES_texture_storage_multisample_2d_array GL_OES_shader_image_atomic GL_EXT_robustness GL_EXT_draw_buffers_indexed GL_OES_draw_buffers_indexed GL_EXT_texture_border_clamp GL_OES_texture_border_clamp GL_EXT_texture_cube_map_array GL_OES_texture_cube_map_array GL_OES_sample_variables GL_OES_sample_shading GL_OES_shader_multisample_interpolation GL_EXT_shader_io_blocks GL_OES_shader_io_blocks GL_EXT_tessellation_shader GL_OES_tessellation_shader GL_EXT_primitive_bounding_box GL_OES_primitive_bounding_box GL_EXT_geometry_shader GL_OES_geometry_shader GL_ANDROID_extension_pack_es31a GL_EXT_gpu_shader5 GL_OES_gpu_shader5 GL_EXT_texture_buffer GL_OES_texture_buffer GL_EXT_copy_image GL_OES_copy_image GL_EXT_color_buffer_half_float GL_EXT_color_buffer_float GL_EXT_YUV_target GL_OVR_multiview GL_OVR_multiview2 GL_OVR_multiview_multisampled_render_to_texture GL_KHR_robustness GL_KHR_robust_buffer_access_behavior GL_EXT_draw_elements_base_vertex GL_OES_draw_elements_base_vertex GL_EXT_protected_textures Disabled Extensions GL_KHR_blend_equation_advanced GL_KHR_blend_equation_advanced_coherent Window system binding vendor Window system binding version Window system binding extensions Direct rendering Yes Reset notification strategy 0x8252 GPU process crash count 0 Compositor Information Tile Update Mode One-copy Partial Raster Enabled GpuMemoryBuffers Status ATC Software only ATCIA Software only DXT1 Software only DXT5 Software only ETC1 Software only R_8 Software only RG_88 Software only BGR_565 Software only RGBA_4444 Software only RGBX_8888 Software only RGBA_8888 Software only BGRX_8888 Software only BGRA_8888 Software only YVU_420 Software only YUV_420_BIPLANAR Software only UYVY_422 Software only
,
Jun 14 2017
Summary: I think this is indeed a logic error, though harmless due to nonzero depth not currently being used, and fixing it seems both appropriate and safe. Longer explanation following. kbr@, my original change was intended to be a refactor, but there was indeed a logic error where the format class's depth default was out of sync with the default surface's properties. In current usage this didn't have any visible effect, since to the best of my knowledge nobody is specifying depth bits for an offscreen context. The default depth=-1 (EGL_DONTCARE) is considered compatible with itself, so it continues using a virtualized context. The only non-test call to SurfaceFormat's SetDepthBits() is here: https://cs.chromium.org/chromium/src/gpu/ipc/service/gpu_command_buffer_stub.cc?type=cs&l=655 Note that this only modifies the setting away from "don't care" for nonzero depth, so depth 0 remains as "don't care". The bug here is that depth 24 is erroneously considered compatible to the default surface, even though the default offscreen surface is created with zero depth bits: That section is conditional on init_params.attribs.own_offscreen_surface being set, specifically to avoid incompatibilities such as an unexpected Ozone usage. (See http://crrev.com/2761153002 .) own_offscreen_surface is currently only being activated by a WebVR runtime experiment that's off by default due to fundamental synchronization issues with non-virtualized contexts: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp?gsn=support_depth&l=628
,
Jun 14 2017
For the synchronization issue with non-virtualized contexts, see http://crbug.com/691102#c10 where I tried to summarize my (tentative) interpretation of what's going on.
,
Jun 14 2017
In Oculus we use this own offscreen surface path, since we do not experience those sync issues Klaus is talking about on our HW. Please, fix it :) Or, we would need to fix it every time we sync with Chromium, which is a big PITA.
,
Jun 19 2017
|
|||
►
Sign in to add a comment |
|||
Comment 1 by kbr@chromium.org
, Jun 14 2017Cc: klausw@chromium.org aelias@chromium.org bajones@chromium.org
Components: -Blink>WebGL Internals>GPU>Internals
Owner: klausw@chromium.org
Status: Assigned (was: Unconfirmed)