Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(428)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1421873005: Revert of "webgl: optimize webgl.texSubImage2D(video) path." (Closed)

Created:
5 years, 1 month ago by rjkroege
Modified:
5 years ago
CC:
chromium-reviews, posciak+watch_chromium.org, eric.carlson_apple.com, mlamouri+watch-media_chromium.org, rwlbuis, pdr+graphicswatchlist_chromium.org, drott+blinkwatch_chromium.org, blink-reviews-html_chromium.org, jam, Justin Novosad, danakj, blink-reviews-platform-graphics_chromium.org, dglazkov+blink, Rik, darin-cc_chromium.org, blink-reviews, blink-reviews-api_chromium.org, mlamouri+watch-content_chromium.org, vmpstr+blinkwatch_chromium.org, philipj_slow, jbroman, feature-media-reviews_chromium.org, krit, piman+watch_chromium.org, mlamouri+watch-blink_chromium.org, avayvod+watch_chromium.org, mcasas+watch_chromium.org, f(malita), mkwst+moarreviews-renderer_chromium.org, Stephen Chennney
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of "webgl: optimize webgl.texSubImage2D(video) path." (patchset #3 id:40001 of https://codereview.chromium.org/1413833006/ ) Reason for revert: Sorry but builder: Android Debug (Nexus 5) (stats) is failing the webgl_conformance_tests since this patch landed. This seems like the most logical candidate for introducing the failure: Expected exception while running WebglConformance.conformance_textures_video_tex_image_and_sub_image_2d_with_video_rgb_rgb_unsigned_byte Traceback (most recent call last): RunStoryWithRetries at content/test/gpu/gpu_tests/gpu_test_base.py:72 super(cls, shared_page_state).RunStory(results) RunStory at tools/telemetry/telemetry/page/shared_page_state.py:325 self._current_page, self._current_tab, results) ValidateAndMeasurePage at content/test/gpu/gpu_tests/webgl_conformance.py:78 raise page_test.Failure(_WebGLTestMessages(tab)) Failure: at (4, 4) expected: 0,255,0 was 0,0,0 testing: video/mp4; codecs="avc1.42E01E, mp4a.40.2" Testing texImage2D with flipY=true bindingTarget=TEXTURE_2D Checking lower left corner FAIL at (4, 4) expected: 0,255,0 was 0,0,0 Checking upper left corner at (4, 24) expected: 255,0,0 was 0,0,0 FAIL at (4, 24) expected: 255,0,0 was 0,0,0 Testing texImage2D with flipY=false bindingTarget=TEXTURE_2D Checking lower left corner at (4, 4) expected: 255,0,0 was 0,0,0 FAIL at (4, 4) expected: 255,0,0 was 0,0,0 Checking upper left corner at (4, 24) expected: 0,255,0 was 0,0,0 FAIL at (4, 24) expected: 0,255,0 was 0,0,0 Testing texSubImage2D with flipY=true bindingTarget=TEXTURE_2D Checking lower left corner at (4, 4) expected: 0,255,0 was 0,0,0 FAIL at (4, 4) expected: 0,255,0 was 0,0,0 Checking upper left corner at (4, 24) expected: 255,0,0 was 0,0,0 FAIL at (4, 24) expected: 255,0,0 was 0,0,0 Testing texSubImage2D with flipY=false bindingTarget=TEXTURE_2D Checking lower left corner at (4, 4) expected: 255,0,0 was 0,0,0 FAIL at (4, 4) expected: 255,0,0 was 0,0,0 Checking upper left corner at (4, 24) expected: 0,255,0 was 0,0,0 FAIL at (4, 24) expected: 0,255,0 was 0,0,0 testing: video/webm; codecs="vp8, vorbis" Testing texImage2D with flipY=true bindingTarget=TEXTURE_2D Checking lower left corner Checking upper left corner Testing texImage2D with flipY=false bindingTarget=TEXTURE_2D Checking lower left corner Checking upper left corner Testing texSubImage2D with flipY=true bindingTarget=TEXTURE_2D Checking lower left corner Checking upper left corner Testing texSubImage2D with flipY=false bindingTarget=TEXTURE_2D Checking lower left corner Checking upper left corner testing: video/ogg; codecs="theora, vorbis" video/ogg; codecs="theora, vorbis" unsupported testing: video/mp4; codecs="avc1.42E01E, mp4a.40.2" Testing texSubImage2D with flipY=true bindingTarget=TEXTURE_CUBE_MAP Checking lower left corner Checking upper left corner Checking lower left corner Checking upper left corner Checking lower left corner Checking upper left corner Checking lower left corner Checking upper left corner Checking lower left corner Checking upper left corner Checking lower left corner Checking upper left corner Testing texSubImage2D with flipY=false bindingTarget=TEXTURE_CUBE_MAP Checking lower left corner Checking upper left corner Checking lower left corner Checking upper left corner Checking lower left corner Checking upper left corner Checking lower left corner Checking upper left corner Checking lower left corner Checking upper left corner Checking lower left corner Checking upper left corner testing: video/webm; codecs="vp8, vorbis" Testing texSubImage2D with flipY=true bindingTarget=TEXTURE_CUBE_MAP Checking lower left corner Checking upper left corner Checking lower left corner Checking upper left corner Checking lower left corner Checking upper left corner Checking lower left corner Checking upper left corner Checking lower left corner Checking upper left corner Checking lower left corner Checking upper left corner Testing texSubImage2D with flipY=false bindingTarget=TEXTURE_CUBE_MAP Checking lower left corner Checking upper left corner Checking lower left corner Checking upper left corner Checking lower left corner Checking upper left corner Checking lower left corner Checking upper left corner Checking lower left corner Checking upper left corner Checking lower left corner Checking upper left corner testing: video/ogg; codecs="theora, vorbis" video/ogg; codecs="theora, vorbis" unsupported Locals: page : <gpu_tests.webgl_conformance.WebglConformancePage object at 0x7f90820c3a50> results : <telemetry.internal.results.page_test_results.PageTestResults object at 0x7f90820d7f50> tab : <telemetry.internal.browser.tab.Tab object at 0x7f90820e4f10> Original issue's description: > Reland of "webgl: optimize webgl.texSubImage2D(video) path." > > Original CL: https://codereview.chromium.org/1315323006/ > Revert: https://codereview.chromium.org/1418513016 > > Reason for revert: > Reverting to address webgl_conformance_tests break in: > https://build.chromium.org/p/chromium.gpu/builders/Android%20Debug%20%28Nexus%209%29/builds/3534 > > Reason for reland: > There is a bug in glCopySubTextureCHROMIUM extension. GLES2DecoderImpl::DoCopySubTextureCHROMIUM/DoCompressedCopySubTextureCHROMIUM > uses ValidForTexture to check dimensions of source texture. If the source > texture is backed by GL Image, the decoder can report false positive error. > This CL doesn't use Texture::ValidForTexture() for GL Image. > > TEST=WebglConformance.conformance_textures_video_tex_image_and_sub_image_2d_with_video_* > BUG=349871, 504773, 549531 > > Committed: https://crrev.com/7c26f04cf12c2c72fdab8e4764faf9305724c7ee > Cr-Commit-Position: refs/heads/master@{#357100} TBR=kbr@chromium.org,dalecurtis@chromium.org,philipj@opera.com,zmo@google.com,piman@chromium.org,dongseong.hwang@intel.com NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=349871, 504773, 549531

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+113 lines, -277 lines) Patch
M content/renderer/media/android/webmediaplayer_android.h View 1 chunk +5 lines, -1 line 0 comments Download
M content/renderer/media/android/webmediaplayer_android.cc View 3 chunks +11 lines, -22 lines 0 comments Download
M content/renderer/media/webmediaplayer_ms.h View 1 chunk +5 lines, -1 line 0 comments Download
M content/renderer/media/webmediaplayer_ms.cc View 2 chunks +7 lines, -13 lines 0 comments Download
M gpu/command_buffer/service/gles2_cmd_decoder.cc View 4 chunks +12 lines, -16 lines 0 comments Download
M media/blink/webmediaplayer_impl.h View 1 chunk +5 lines, -1 line 0 comments Download
M media/blink/webmediaplayer_impl.cc View 2 chunks +7 lines, -13 lines 0 comments Download
M media/renderers/skcanvas_video_renderer.h View 1 chunk +5 lines, -24 lines 0 comments Download
M media/renderers/skcanvas_video_renderer.cc View 4 chunks +10 lines, -42 lines 0 comments Download
M third_party/WebKit/Source/core/html/HTMLVideoElement.h View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/html/HTMLVideoElement.cpp View 1 chunk +3 lines, -6 lines 0 comments Download
M third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.h View 1 chunk +0 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp View 4 chunks +25 lines, -58 lines 0 comments Download
M third_party/WebKit/Source/platform/graphics/ImageBuffer.h View 2 chunks +1 line, -7 lines 0 comments Download
M third_party/WebKit/Source/platform/graphics/ImageBuffer.cpp View 2 chunks +4 lines, -27 lines 0 comments Download
M third_party/WebKit/Source/platform/graphics/gpu/Extensions3DUtil.cpp View 1 chunk +2 lines, -17 lines 0 comments Download
M third_party/WebKit/public/platform/WebMediaPlayer.h View 1 chunk +10 lines, -25 lines 0 comments Download

Messages

Total messages: 6 (3 generated)
rjkroege
Created Revert of "webgl: optimize webgl.texSubImage2D(video) path."
5 years, 1 month ago (2015-10-30 19:01:16 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1421873005/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1421873005/1
5 years, 1 month ago (2015-10-30 19:04:49 UTC) #2
commit-bot: I haz the power
5 years, 1 month ago (2015-10-30 19:06:53 UTC) #4 
Failed to apply patch for gpu/command_buffer/service/gles2_cmd_decoder.cc:
While running git apply --index -3 -p1;
  error: patch failed: gpu/command_buffer/service/gles2_cmd_decoder.cc:13181
  error: repository lacks the necessary blob to fall back on 3-way merge.
  error: gpu/command_buffer/service/gles2_cmd_decoder.cc: patch does not apply

Patch:       gpu/command_buffer/service/gles2_cmd_decoder.cc
Index: gpu/command_buffer/service/gles2_cmd_decoder.cc
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.cc
b/gpu/command_buffer/service/gles2_cmd_decoder.cc
index
1c618c2dac9b30ead8996a2ec85fbdcaa75f1e15..34ae829ddb1f2b2389e480c85d6bf920f8682970
100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc
@@ -13181,19 +13181,10 @@
       return;
     }
   } else {
-    // TODO(dshwang): make GetLevelSize, ValidForTexture and ValidForTarget
-    // correct for GLImage also. crbug.com/549531
     if (!source_texture->GetLevelSize(source_texture->target(), 0,
                                       &source_width, &source_height, nullptr))
{
       LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, "glCopySubTextureCHROMIUM",
                          "source texture has no level 0");
-      return;
-    }
-
-    if (!source_texture->ValidForTexture(source_texture->target(), 0, x, y, 0,
-                                         width, height, 1)) {
-      LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, "glCopySubTextureCHROMIUM",
-                         "source texture bad dimensions.");
       return;
     }
 
@@ -13210,6 +13201,12 @@
   GLenum source_internal_format = 0;
   source_texture->GetLevelType(source_texture->target(), 0, &source_type,
                                &source_internal_format);
+  if (!source_texture->ValidForTexture(source_texture->target(), 0, x, y, 0,
+                                       width, height, 1)) {
+    LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, "glCopySubTextureCHROMIUM",
+                       "source texture bad dimensions.");
+    return;
+  }
 
   GLenum dest_type = 0;
   GLenum dest_internal_format = 0;
@@ -13513,13 +13510,6 @@
       return;
     }
 
-    if (!source_texture->ValidForTexture(source_texture->target(), 0, x, y, 0,
-                                         width, height, 1)) {
-      LOCAL_SET_GL_ERROR(GL_INVALID_VALUE,
"glCompressedCopySubTextureCHROMIUM",
-                         "source texture bad dimensions.");
-      return;
-    }
-
     // Check that this type of texture is allowed.
     if (!texture_manager()->ValidForTarget(source_texture->target(), 0,
                                            source_width, source_height, 1)) {
@@ -13533,6 +13523,12 @@
   GLenum source_internal_format = 0;
   source_texture->GetLevelType(source_texture->target(), 0, &source_type,
                                &source_internal_format);
+  if (!source_texture->ValidForTexture(source_texture->target(), 0, x, y, 0,
+                                       width, height, 1)) {
+    LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, "glCompressedCopySubTextureCHROMIUM",
+                       "source texture bad dimensions.");
+    return;
+  }
 
   GLenum dest_type = 0;
   GLenum dest_internal_format = 0;

Powered by Google App Engine
This is Rietveld 408576698