Issue metadata
Sign in to add a comment
|
arm (peach, veyron, oak): thumbnails on NTP corrupted; page captured from extension corrupted |
||||||||||||||||||||||
Issue descriptionOriginally reported as: http://crosbug.com/p/52346 Chrome Version: 52.0.2734.0 Chrome OS Version: 8328.0.0 Chrome OS Platform: peach_pi, veyron_minnie, oak Steps To Reproduce issue: (1) Log in as a (test) user (2) close all tabs and windows (3) navigate to chrome://history: (4) clear [Browsing history ; Download history ; Cached images and files] (5) log out (6) log back in (7) browse to any page, eg: https://en.wikipedia.org/wiki/List_of_law_clerks_of_the_Supreme_Court_of_the_United_States (8) open new tab page (9) Open chrome://thumbnails (10) Install the "Awesome Screenshot" extension: https://chrome.google.com/webstore/detail/awesome-screenshot-screen/nlipoenfbbikpbjkfpfillcgkoblgpmj (10.a) [Googlers w/ Enterprise Enrolled Chromebooks can also repro w/ go/snipit: https://chrome.google.com/webstore/detail/snipit/ehoadneljpdggcbbknedodolkkjodefl ] (11) Navigate to any page, eg: https://en.wikipedia.org/wiki/List_of_law_clerks_of_the_Supreme_Court_of_the_United_States (12) Press "Ctrl-Shift-E" to take a snapshot of page contents Expected Result: (8) => Accurate thumbnail of page (7) (9) => Accurate thumbnail of page (7) (12) => Accurate screenshot of page (11) Actual Result: (8) => Corrupted thumbnail of page from (7) (9) => Corrupted thumbnail of page from (7) (12) => Corrupted screenshot of page (11) How frequently does this problem reproduce? (Always, sometimes, hard to reproduce?) Always What is the impact to the user, and is there a workaround? If so, what is it? Some thumbnails on New Tab Page are corrupted. Cannot use extensions to capture tab content. Please provide any additional information below. Attach a screen shot or log if possible. From /var/log/ui/ui.LATEST: [20251:20251:0516/112905:ERROR:gles2_cmd_decoder.cc(2236)] [.CompositorWorker-0xb8f3d400]GL ERROR :GL_INVALID_OPERATION : GLES2DecoderImpl::DoBindTexImage2DCHROMIUM: <- error from previous GL command Note: this issue reproduces on at least peach, veyron and oak. Note: for screenshots and more analysis, see original report: http://crosbug.com/p/52346
,
May 16 2016
Mali specific bug opened here: crosbug.com/p/53381
,
May 16 2016
,
May 16 2016
,
May 19 2016
This is also affecting the current BETA release of Chrome 51, running on ARM Platform
,
May 24 2016
Fixed on oak as of 8341.0.0 Fixed on daisy/peach/veyron as of 8350.0.0.
,
Jul 1 2016
,
Aug 29 2016
,
Aug 31 2016
,
Feb 7 2017
Moving old issues out of Internal>Graphics to delete this obsolete component ( crbug.com/685425 for details) |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by djkurtz@chromium.org
, May 16 2016It turns out the GL error is caused by the GLES driver rejecting Chrome's glReadPixels() call to fetch pixels for the extension/thumbnail. In fact the GLES library doesn't like the parameters we are passing, and just errors out. Chrome does not do any error checking near this glReadPixels, and hence it happily processes garbage pixels. However, at some unlucky point later, some code checks glGetError(), which reports that at some previous time, there had been a GL_INVALID_OPERATION. In the trace above, the unlucky function was doBindTexImage2DCHROMIUM. This is what a call looks like: glReadPixels(0, 0, 424, 284, GL_BGRA_EXT, GL_UNSIGNED_BYTE, 0); This call, and its format are ultimately initiated : (a) for tab capture extensions: bool WebContentsCaptureClient::CaptureAsync( ... host->CopyFromBackingStore(gfx::Rect(view_size), bitmap_size, callback, kN32_SkColorType); (b) for thumbnails: void ThumbnailTabHelper::AsyncProcessThumbnail( ... render_widget_host->CopyFromBackingStore( copy_rect, thumbnailing_context_->requested_copy_size, base::Bind(&ThumbnailTabHelper::ProcessCapturedBitmap, weak_factory_.GetWeakPtr(), algorithm), kN32_SkColorType); In both cases, the color format is: kN32_SkColorType, which is defined as: #if SK_PMCOLOR_BYTE_ORDER(B,G,R,A) kN32_SkColorType = kBGRA_8888_SkColorType, #elif SK_PMCOLOR_BYTE_ORDER(R,G,B,A) kN32_SkColorType = kRGBA_8888_SkColorType, #else #error "SK_*32_SHFIT values must correspond to BGRA or RGBA byte order" #endif For non Android builds, in skia/config/SkUserConfig.h Chromium sets: #if !defined(ANDROID) // On Android, we use the skia default settings. #define SK_A32_SHIFT 24 #define SK_R32_SHIFT 16 #define SK_G32_SHIFT 8 #define SK_B32_SHIFT 0 #endif For arm SK_CPU_LENDIAN is defined, so: #ifdef SK_CPU_BENDIAN # define SK_PMCOLOR_BYTE_ORDER(C0, C1, C2, C3) \ (SK_ ## C3 ## 32_SHIFT == 0 && \ SK_ ## C2 ## 32_SHIFT == 8 && \ SK_ ## C1 ## 32_SHIFT == 16 && \ SK_ ## C0 ## 32_SHIFT == 24) #else # define SK_PMCOLOR_BYTE_ORDER(C0, C1, C2, C3) \ (SK_ ## C0 ## 32_SHIFT == 0 && \ SK_ ## C1 ## 32_SHIFT == 8 && \ SK_ ## C2 ## 32_SHIFT == 16 && \ SK_ ## C3 ## 32_SHIFT == 24) #endif Therefore, SK_PMCOLOR_BYTE_ORDER(B,G,R,A) == true, and kN32_SkColorType = kBGRA_8888_SkColorType CopyFromBackingStore() eventually winds its way down to CopyFromCompositingSurface(), which has a somewhat misleading comment (since kN32_SkColorType is not necessarily ARGB888): void DelegatedFrameHost::CopyFromCompositingSurface( const gfx::Rect& src_subrect, const gfx::Size& output_size, const ReadbackRequestCallback& callback, const SkColorType preferred_color_type) { // Only ARGB888 and RGB565 supported as of now. bool format_support = ((preferred_color_type == kAlpha_8_SkColorType) || (preferred_color_type == kRGB_565_SkColorType) || (preferred_color_type == kN32_SkColorType)); This format winds its way through many layers, eventually getting converted to a gles format: void GLHelper::CopyTextureToImpl::CropScaleReadbackAndCleanTexture( { ... FormatSupport supported = GetReadbackConfig(readback_color_type, true, &format, &type, &bytes_per_pixel); ... case kBGRA_8888_SkColorType: *format = GL_BGRA_EXT; So: (1) Chrome overrides Skia's default settings to use BGRA instead of RGBA (2) Chrome then tries to use GL_BGRA_EXT for glReadPixels() (3) The GLES drivers reject GL_BGRA_EXT for glReadPixels() (4) Chrome doesn't check for errors reported by the driver (5) Chrome detects the GL error later, but doesn't know where it came from, so just prints and otherwise ignores it (5) Chrome uses whatever garbage was in the buffer when glReadPixel() fails as the thumbnail/screenshot Two questions remain: (1) Why are the ARM GLES drivers rejecting glReadPixels() w/ format = GL_BGRA_EXT? (2) This used to work.. What introduced this regression?