Reland "Fix HTML5 video blurry" (issue 2547683003 by dongseong.hwang@intel.com)

0 views
Skip to first unread message

dongseo...@intel.com

unread,
Dec 1, 2016, 7:42:12 PM12/1/16
to ccameron...@chromium.org, dalec...@chromium.org, dpr...@chromium.org, en...@chromium.org, ju...@chromium.org, dongseo...@chromium.org, chromium...@chromium.org, blink-reviews-p...@chromium.org, sche...@chromium.org, miu+...@chromium.org, dsch...@chromium.org, drott+bl...@chromium.org, j...@chromium.org, ju...@chromium.org, jbauma...@chromium.org, poscia...@chromium.org, caba...@adobe.com, dari...@chromium.org, blink-...@chromium.org, einbinder+wat...@chromium.org, xjz+...@chromium.org, rob....@samsung.com, ajuma...@chromium.org, jbr...@chromium.org, feature-me...@chromium.org, pdr+graphi...@chromium.org, kalyan.k...@intel.com, piman...@chromium.org, danakj...@chromium.org, jochen...@chromium.org, mlamouri+watc...@chromium.org, ajuma+wat...@chromium.org, fma...@chromium.org, cc-...@chromium.org
Reviewers: ccameron, DaleCurtis, Dirk Pranke, enne, Justin Novosad, dongseong.hwang
CL: https://codereview.chromium.org/2547683003/

Message:
enne@, could you review this CL again?
Sorry for delay of reland. I come back from vacation.
I reproduced the crash on my Win7 and fixed it. The change of this CL is diff
between patch set 1 and patch set 2.

Reference
It's the crash backtrace. Win7 bot did good jobs.
D:\chromium\src>out\Debug\cc_unittests.exe
--gtest_filter=GLRendererPixelTest.TextureQuadBatching
IMPORTANT DEBUGGING NOTE: batches of tests are run inside their
own process. For debugging a test inside a debugger, use the
--gtest_filter=<your_test_name> flag along with
--single-process-tests.
Using sharding settings from environment. This is shard 0/1
Using 1 parallel jobs.
Note: Google Test filter = GLRendererPixelTest.TextureQuadBatching
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from GLRendererPixelTest
[ RUN ] GLRendererPixelTest.TextureQuadBatching
Backtrace:
std::_Debug_message [0x000007FEE0820806+70]

std::_Vector_const_iterator<std::_Vector_val<std::_Simple_types<cc::Float4> >
>::operator* [0x0000000180234C02+114] (c:\program file
s (x86)\microsoft visual studio 14.0\vc\include\vector:74)
std::_Vector_iterator<std::_Vector_val<std::_Simple_types<cc::Float4> >
>::operator* [0x00000001802350E3+19] (c:\program files (x86)
\microsoft visual studio 14.0\vc\include\vector:332)
std::vector<cc::Float4,std::allocator<cc::Float4> >::front
[0x000000018026D4D0+32] (c:\program files (x86)\microsoft visual studio 1
4.0\vc\include\vector:1257)
cc::GLRenderer::FlushTextureQuadCache [0x0000000180245E0D+893]
(d:\chromium\src\cc\output\gl_renderer.cc:2599)
cc::GLRenderer::EnqueueTextureQuad [0x0000000180243E4A+682]
(d:\chromium\src\cc\output\gl_renderer.cc:2705)
cc::GLRenderer::DoDrawQuad [0x000000018023D01B+971]
(d:\chromium\src\cc\output\gl_renderer.cc:608)
cc::DirectRenderer::DrawRenderPass [0x0000000180208DE8+1800]
(d:\chromium\src\cc\output\direct_renderer.cc:529)
cc::DirectRenderer::DrawRenderPassAndExecuteCopyRequests
[0x0000000180209113+195] (d:\chromium\src\cc\output\direct_renderer.cc:434)

cc::DirectRenderer::DrawFrame [0x0000000180208418+2312]
(d:\chromium\src\cc\output\direct_renderer.cc:306)
cc::PixelTest::RunPixelTestWithReadbackTargetAndArea
[0x00000001410A9BD2+450] (d:\chromium\src\cc\test\pixel_test.cc:88)
cc::PixelTest::RunPixelTestWithReadbackTarget [0x00000001410A99F4+68]
(d:\chromium\src\cc\test\pixel_test.cc:59)
cc::PixelTest::RunPixelTest [0x00000001410A998B+75]
(d:\chromium\src\cc\test\pixel_test.cc:50)
cc::`anonymous
namespace'::GLRendererPixelTest_TextureQuadBatching_Test::TestBody
[0x00000001406D4521+2561] (d:\chromium\src\cc\outp
ut\renderer_pixeltest.cc:3397)

testing::internal::HandleExceptionsInMethodIfSupported<testing::Test,void>
[0x0000000141190608+72] (d:\chromium\src\testing\gtest\sr
c\gtest.cc:2460)
testing::Test::Run [0x00000001411B55CE+158]
(d:\chromium\src\testing\gtest\src\gtest.cc:2481)
testing::TestInfo::Run [0x00000001411B58C1+209]
(d:\chromium\src\testing\gtest\src\gtest.cc:2660)
testing::TestCase::Run [0x00000001411B5721+225]
(d:\chromium\src\testing\gtest\src\gtest.cc:2775)
testing::internal::UnitTestImpl::RunAllTests [0x00000001411B5E2D+813]
(d:\chromium\src\testing\gtest\src\gtest.cc:4648)

testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl,bool>
[0x0000000141190728+72] (d:\chromium\sr
c\testing\gtest\src\gtest.cc:2460)
testing::UnitTest::Run [0x00000001411B5A7C+236]
(d:\chromium\src\testing\gtest\src\gtest.cc:4255)
RUN_ALL_TESTS [0x0000000141110731+17]
(d:\chromium\src\testing\gtest\include\gtest\gtest.h:2238)
base::TestSuite::Run [0x000000014111089C+140]
(d:\chromium\src\base\test\test_suite.cc:246)
base::internal::FunctorTraits<int (__cdecl base::TestSuite::*)(void)
__ptr64,void>::Invoke<cc::CCTestSuite * __ptr64> [0x0000000140E
C36E4+36] (d:\chromium\src\base\bind_internal.h:215)
base::internal::InvokeHelper<0,int>::MakeItSo<int (__cdecl
base::TestSuite::*const & __ptr64)(void) __ptr64,cc::CCTestSuite * __ptr6
4> [0x0000000140EC3777+55] (d:\chromium\src\base\bind_internal.h:287)
base::internal::Invoker<base::internal::BindState<int (__cdecl
base::TestSuite::*)(void) __ptr64,base::internal::UnretainedWrapper<c
c::CCTestSuite> >,int __cdecl(void)>::RunImpl<int (__cdecl
base::TestSuite::*const & __ptr64)(void) __ptr64,std::tuple<base
[0x0000000140EC3
7D9+73] (d:\chromium\src\base\bind_internal.h:365)
base::internal::Invoker<base::internal::BindState<int (__cdecl
base::TestSuite::*)(void) __ptr64,base::internal::UnretainedWrapper<c
c::CCTestSuite> >,int __cdecl(void)>::Run [0x0000000140EC3A73+51]
(d:\chromium\src\base\bind_internal.h:343)
base::internal::RunMixin<base::Callback<int __cdecl(void),1,1> >::Run
[0x000000014111E175+85] (d:\chromium\src\base\callback.h:65)
base::`anonymous namespace'::LaunchUnitTestsInternal
[0x000000014111CE6F+335] (d:\chromium\src\base\test\launcher\unit_test_launcher
.cc:211)
base::LaunchUnitTests [0x000000014111CC0D+125]
(d:\chromium\src\base\test\launcher\unit_test_launcher.cc:453)
main [0x0000000140EC3B55+133]
(d:\chromium\src\cc\test\run_all_unittests.cc:15)
invoke_main [0x000000014138FF64+52]
(f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:65)
__scrt_common_main_seh [0x000000014138FE27+295]
(f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:253)
__scrt_common_main [0x000000014138FCEE+14]
(f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:296)
mainCRTStartup [0x000000014138FF89+9]
(f:\dd\vctools\crt\vcstartup\src\startup\exe_main.cpp:17)
BaseThreadInitThunk [0x000000007742652D+13]
RtlUserThreadStart [0x000000007755C541+33]
[1/1] GLRendererPixelTest.TextureQuadBatching (CRASHED)
1 test crashed:
GLRendererPixelTest.TextureQuadBatching
(d:\chromium\src\cc\output\renderer_pixeltest.cc:3320)
Tests took 2 seconds.

Description:
Reland "Fix HTML5 video blurry"

Original CL: https://codereview.chromium.org/2418173002/

Revert CL: https://codereview.chromium.org/2463103002/
https://codereview.chromium.org/2474073004

Changes: Skip failed gpu pixel tests until baseline is created.
Fix following failures in Win7 dbg
RendererPixelTest/2.PremultipliedTextureWithBackground
GLRendererPixelTest.TextureQuadBatching
RendererPixelTest/0.PremultipliedTextureWithBackground
GLRendererPixelTest.NonPremultipliedTextureWithBackground
IntersectingQuadGLPixelTest/0.Y16VideoQuads
VideoGLRendererPixelTest.TwoColorY16Rect
IntersectingQuadGLPixelTest/1.Y16VideoQuads

TBR=dalec...@chromium.org,ccam...@chromium.org,dpr...@chromium.org,ju...@chromium.org
BUG=615325
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Affected files (+571, -252 lines):
M cc/layers/video_layer_impl.cc
M cc/output/gl_renderer.h
M cc/output/gl_renderer.cc
M cc/output/gl_renderer_draw_cache.h
M cc/output/renderer_pixeltest.cc
M cc/output/shader.h
M cc/output/shader.cc
M cc/resources/resource_provider.h
M cc/resources/resource_provider.cc
M cc/resources/video_resource_updater.h
M cc/resources/video_resource_updater.cc
M cc/test/data/yuv_stripes.png
M cc/test/data/yuv_stripes_alpha.png
M cc/test/data/yuv_stripes_clipped.png
A cc/test/data/yuv_stripes_clipped_rgba.png
M cc/test/data/yuv_stripes_offset.png
A cc/test/data/yuv_stripes_rgba.png
M cc/test/render_pass_test_utils.cc
M cc/test/test_in_process_context_provider.h
M cc/test/test_in_process_context_provider.cc
M components/test_runner/test_plugin.cc
M content/browser/renderer_host/delegated_frame_host.cc
M content/test/gpu/gpu_tests/pixel_expectations.py
M content/test/gpu/gpu_tests/pixel_test_pages.py
M media/renderers/skcanvas_video_renderer.h
M media/renderers/skcanvas_video_renderer.cc
M third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp


enne@chromium.org via codereview.chromium.org

unread,
Dec 2, 2016, 1:17:37 PM12/2/16
to dongseo...@intel.com, ccameron...@chromium.org, dalec...@chromium.org, dpr...@chromium.org, ju...@chromium.org, dongseo...@chromium.org, chromium...@chromium.org, blink-reviews-p...@chromium.org, sche...@chromium.org, miu+...@chromium.org, dsch...@chromium.org, drott+bl...@chromium.org, j...@chromium.org, ju...@chromium.org, jbauma...@chromium.org, poscia...@chromium.org, caba...@adobe.com, dari...@chromium.org, blink-...@chromium.org, einbinder+wat...@chromium.org, xjz+...@chromium.org, rob....@samsung.com, ajuma...@chromium.org, jbr...@chromium.org, feature-me...@chromium.org, pdr+graphi...@chromium.org, kalyan.k...@intel.com, piman...@chromium.org, danakj...@chromium.org, jochen...@chromium.org, mlamouri+watc...@chromium.org, ajuma+wat...@chromium.org, fma...@chromium.org, cc-...@chromium.org
Ok, lgtm if the only change is that added conditional. Thanks for following up
on this. :)

https://codereview.chromium.org/2547683003/

dongseo...@intel.com

unread,
Dec 2, 2016, 1:19:55 PM12/2/16
to ccameron...@chromium.org, dalec...@chromium.org, dpr...@chromium.org, en...@chromium.org, ju...@chromium.org, dongseo...@chromium.org, chromium...@chromium.org, blink-reviews-p...@chromium.org, sche...@chromium.org, miu+...@chromium.org, dsch...@chromium.org, drott+bl...@chromium.org, j...@chromium.org, ju...@chromium.org, jbauma...@chromium.org, poscia...@chromium.org, caba...@adobe.com, dari...@chromium.org, blink-...@chromium.org, einbinder+wat...@chromium.org, xjz+...@chromium.org, rob....@samsung.com, ajuma...@chromium.org, jbr...@chromium.org, feature-me...@chromium.org, pdr+graphi...@chromium.org, kalyan.k...@intel.com, piman...@chromium.org, danakj...@chromium.org, jochen...@chromium.org, mlamouri+watc...@chromium.org, ajuma+wat...@chromium.org, fma...@chromium.org, cc-...@chromium.org
On 2016/12/02 18:17:37, enne wrote:
> Ok, lgtm if the only change is that added conditional. Thanks for following
up
> on this. :)

Thank you for reviewing again. The condition must be there before, but I missed
:)

https://codereview.chromium.org/2547683003/

commit-bot@chromium.org via codereview.chromium.org

unread,
Dec 2, 2016, 1:20:29 PM12/2/16
to dongseo...@intel.com, ccameron...@chromium.org, dalec...@chromium.org, dpr...@chromium.org, en...@chromium.org, ju...@chromium.org, dongseo...@chromium.org, commi...@chromium.org, chromium...@chromium.org, blink-reviews-p...@chromium.org, sche...@chromium.org, miu+...@chromium.org, dsch...@chromium.org, drott+bl...@chromium.org, j...@chromium.org, ju...@chromium.org, jbauma...@chromium.org, poscia...@chromium.org, caba...@adobe.com, dari...@chromium.org, blink-...@chromium.org, einbinder+wat...@chromium.org, xjz+...@chromium.org, rob....@samsung.com, ajuma...@chromium.org, jbr...@chromium.org, feature-me...@chromium.org, pdr+graphi...@chromium.org, kalyan.k...@intel.com, piman...@chromium.org, danakj...@chromium.org, jochen...@chromium.org, mlamouri+watc...@chromium.org, ajuma+wat...@chromium.org, fma...@chromium.org, cc-...@chromium.org

commit-bot@chromium.org via codereview.chromium.org

unread,
Dec 2, 2016, 1:26:39 PM12/2/16
to dongseo...@intel.com, ccameron...@chromium.org, dalec...@chromium.org, dpr...@chromium.org, en...@chromium.org, ju...@chromium.org, dongseo...@chromium.org, commi...@chromium.org, chromium...@chromium.org, blink-reviews-p...@chromium.org, sche...@chromium.org, miu+...@chromium.org, dsch...@chromium.org, drott+bl...@chromium.org, j...@chromium.org, ju...@chromium.org, jbauma...@chromium.org, poscia...@chromium.org, caba...@adobe.com, dari...@chromium.org, blink-...@chromium.org, einbinder+wat...@chromium.org, xjz+...@chromium.org, rob....@samsung.com, ajuma...@chromium.org, jbr...@chromium.org, feature-me...@chromium.org, pdr+graphi...@chromium.org, kalyan.k...@intel.com, piman...@chromium.org, danakj...@chromium.org, jochen...@chromium.org, mlamouri+watc...@chromium.org, ajuma+wat...@chromium.org, fma...@chromium.org, cc-...@chromium.org
Try jobs failed on following builders:
chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/317556)

https://codereview.chromium.org/2547683003/

commit-bot@chromium.org via codereview.chromium.org

unread,
Dec 2, 2016, 6:16:03 PM12/2/16
to dongseo...@intel.com, ccameron...@chromium.org, dalec...@chromium.org, dpr...@chromium.org, en...@chromium.org, ju...@chromium.org, dongseo...@chromium.org, commi...@chromium.org, chromium...@chromium.org, blink-reviews-p...@chromium.org, sche...@chromium.org, miu+...@chromium.org, dsch...@chromium.org, drott+bl...@chromium.org, j...@chromium.org, ju...@chromium.org, jbauma...@chromium.org, poscia...@chromium.org, caba...@adobe.com, dari...@chromium.org, blink-...@chromium.org, einbinder+wat...@chromium.org, xjz+...@chromium.org, rob....@samsung.com, ajuma...@chromium.org, jbr...@chromium.org, feature-me...@chromium.org, pdr+graphi...@chromium.org, kalyan.k...@intel.com, piman...@chromium.org, danakj...@chromium.org, jochen...@chromium.org, mlamouri+watc...@chromium.org, ajuma+wat...@chromium.org, fma...@chromium.org, cc-...@chromium.org

commit-bot@chromium.org via codereview.chromium.org

unread,
Dec 2, 2016, 8:18:43 PM12/2/16
to dongseo...@intel.com, ccameron...@chromium.org, dalec...@chromium.org, dpr...@chromium.org, en...@chromium.org, ju...@chromium.org, dongseo...@chromium.org, commi...@chromium.org, chromium...@chromium.org, blink-reviews-p...@chromium.org, sche...@chromium.org, miu+...@chromium.org, dsch...@chromium.org, drott+bl...@chromium.org, j...@chromium.org, ju...@chromium.org, jbauma...@chromium.org, poscia...@chromium.org, caba...@adobe.com, dari...@chromium.org, blink-...@chromium.org, einbinder+wat...@chromium.org, xjz+...@chromium.org, rob....@samsung.com, ajuma...@chromium.org, jbr...@chromium.org, feature-me...@chromium.org, pdr+graphi...@chromium.org, kalyan.k...@intel.com, piman...@chromium.org, danakj...@chromium.org, jochen...@chromium.org, mlamouri+watc...@chromium.org, ajuma+wat...@chromium.org, fma...@chromium.org, cc-...@chromium.org
Committed patchset #3 (id:40001)

https://codereview.chromium.org/2547683003/

commit-bot@chromium.org via codereview.chromium.org

unread,
Dec 2, 2016, 8:21:01 PM12/2/16
to dongseo...@intel.com, ccameron...@chromium.org, dalec...@chromium.org, dpr...@chromium.org, en...@chromium.org, ju...@chromium.org, dongseo...@chromium.org, commi...@chromium.org, chromium...@chromium.org, blink-reviews-p...@chromium.org, sche...@chromium.org, miu+...@chromium.org, dsch...@chromium.org, drott+bl...@chromium.org, j...@chromium.org, ju...@chromium.org, jbauma...@chromium.org, poscia...@chromium.org, caba...@adobe.com, dari...@chromium.org, blink-...@chromium.org, einbinder+wat...@chromium.org, xjz+...@chromium.org, rob....@samsung.com, ajuma...@chromium.org, jbr...@chromium.org, feature-me...@chromium.org, pdr+graphi...@chromium.org, kalyan.k...@intel.com, piman...@chromium.org, danakj...@chromium.org, jochen...@chromium.org, mlamouri+watc...@chromium.org, ajuma+wat...@chromium.org, fma...@chromium.org, cc-...@chromium.org
Patchset 3 (id:??) landed as
https://crrev.com/60d375f8a6bd7b8c3e3abfd3771a228f3039bf62
Cr-Commit-Position: refs/heads/master@{#436118}

https://codereview.chromium.org/2547683003/

ynov...@chromium.org

unread,
Dec 2, 2016, 11:41:14 PM12/2/16
to dongseo...@intel.com, ccameron...@chromium.org, dalec...@chromium.org, dpr...@chromium.org, en...@chromium.org, ju...@chromium.org, dongseo...@chromium.org, chromium...@chromium.org, blink-reviews-p...@chromium.org, sche...@chromium.org, miu+...@chromium.org, dsch...@chromium.org, drott+bl...@chromium.org, j...@chromium.org, ju...@chromium.org, jbauma...@chromium.org, poscia...@chromium.org, caba...@adobe.com, dari...@chromium.org, blink-...@chromium.org, einbinder+wat...@chromium.org, xjz+...@chromium.org, rob....@samsung.com, ajuma...@chromium.org, jbr...@chromium.org, feature-me...@chromium.org, pdr+graphi...@chromium.org, kalyan.k...@intel.com, piman...@chromium.org, danakj...@chromium.org, jochen...@chromium.org, mlamouri+watc...@chromium.org, ajuma+wat...@chromium.org, fma...@chromium.org, cc-...@chromium.org
A revert of this CL (patchset #3 id:40001) has been created in
https://codereview.chromium.org/2544893006/ by ynov...@chromium.org.

The reason for reverting is: I suspect this breaks
https://build.chromium.org/p/chromium.gpu.fyi/builders/Win7%20Release%20%28AMD%29/builds/565
Error compiling shader: ERROR: 0:7: '[]' : Index expression must be constant.

https://codereview.chromium.org/2547683003/

aleksandar....@intel.com

unread,
Dec 3, 2016, 5:04:09 AM12/3/16
to dongseo...@intel.com, ccameron...@chromium.org, dalec...@chromium.org, dpr...@chromium.org, en...@chromium.org, ju...@chromium.org, dongseo...@chromium.org, chromium...@chromium.org, blink-reviews-p...@chromium.org, sche...@chromium.org, miu+...@chromium.org, dsch...@chromium.org, drott+bl...@chromium.org, j...@chromium.org, ju...@chromium.org, jbauma...@chromium.org, poscia...@chromium.org, caba...@adobe.com, dari...@chromium.org, blink-...@chromium.org, einbinder+wat...@chromium.org, xjz+...@chromium.org, rob....@samsung.com, ajuma...@chromium.org, jbr...@chromium.org, feature-me...@chromium.org, pdr+graphi...@chromium.org, kalyan.k...@intel.com, piman...@chromium.org, danakj...@chromium.org, jochen...@chromium.org, mlamouri+watc...@chromium.org, ajuma+wat...@chromium.org, fma...@chromium.org, cc-...@chromium.org
On 2016/12/03 04:41:13, ynovikov wrote:
> A revert of this CL (patchset #3 id:40001) has been created in

>
> The reason for reverting is: I suspect this breaks
>
https://build.chromium.org/p/chromium.gpu.fyi/builders/Win7%20Release%20%28AMD%29/builds/565
> Error compiling shader: ERROR: 0:7: '[]' : Index expression must be constant.

It seems that none of CQ bots runs webgl_conformance_d3d9_tests.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win_optional_gpu_tests_rel
should help.

https://codereview.chromium.org/2547683003/

aleksandar....@intel.com

unread,
Dec 3, 2016, 6:20:40 AM12/3/16
to dongseo...@intel.com, ccameron...@chromium.org, dalec...@chromium.org, dpr...@chromium.org, en...@chromium.org, ju...@chromium.org, dongseo...@chromium.org, chromium...@chromium.org, blink-reviews-p...@chromium.org, sche...@chromium.org, miu+...@chromium.org, dsch...@chromium.org, drott+bl...@chromium.org, j...@chromium.org, ju...@chromium.org, jbauma...@chromium.org, poscia...@chromium.org, caba...@adobe.com, dari...@chromium.org, blink-...@chromium.org, einbinder+wat...@chromium.org, xjz+...@chromium.org, rob....@samsung.com, ajuma...@chromium.org, jbr...@chromium.org, feature-me...@chromium.org, pdr+graphi...@chromium.org, kalyan.k...@intel.com, piman...@chromium.org, danakj...@chromium.org, jochen...@chromium.org, mlamouri+watc...@chromium.org, ajuma+wat...@chromium.org, fma...@chromium.org, cc-...@chromium.org

https://codereview.chromium.org/2547683003/diff/40001/cc/output/shader.cc
File cc/output/shader.cc (right):

https://codereview.chromium.org/2547683003/diff/40001/cc/output/shader.cc#newcode1303
cc/output/shader.cc:1303: vec2 tex_clamped =
max(tex_clamp_rect[int(v_index)].xy,
Anyway, seems related to using non constant index in accessing uniform
array in fragment shader under angle:
https://cs.chromium.org/chromium/src/third_party/angle/src/compiler/translator/ValidateLimitations.cpp?rcl=0&l=505

https://codereview.chromium.org/2547683003/
Reply all
Reply to author
Forward
0 new messages