Issue 2547683003: Reland "Fix HTML5 video blurry"

Issue 2547683003: Reland "Fix HTML5 video blurry" (Closed)

Created:
4 years ago by dshwang

Modified:
4 years ago

Reviewers:
Dirk Pranke, aleksandar.stojiljkovic, ccameron, Justin Novosad, enne (OOO), DaleCurtis, inactive_dshwang_plz_cc_intel

CC:
chromium-reviews, blink-reviews-platform-graphics_chromium.org, Stephen Chennney, miu+watch_chromium.org, krit, drott+blinkwatch_chromium.org, jam, Justin Novosad, jbauman+watch_chromium.org, posciak+watch_chromium.org, Rik, darin-cc_chromium.org, blink-reviews, einbinder+watch-test-runner_chromium.org, xjz+watch_chromium.org, rwlbuis, ajuma+watch_chromium.org, jbroman, feature-media-reviews_chromium.org, pdr+graphicswatchlist_chromium.org, kalyank, piman+watch_chromium.org, danakj+watch_chromium.org, jochen+watch_chromium.org, mlamouri+watch-test-runner_chromium.org, ajuma+watch-canvas_chromium.org, f(malita), cc-bugs_chromium.org

Target Ref:
refs/pending/heads/master

Project:
chromium

Visibility:
Public.

More Reviews

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=dalecurtis@chromium.org,ccameron@chromium.org,dpranke@chromium.org,junov@chromium.org BUG=615325 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel Committed: https://crrev.com/60d375f8a6bd7b8c3e3abfd3771a228f3039bf62 Cr-Commit-Position: refs/heads/master@{#436118}

Patch Set 1 #

Patch Set 2 : diff of this CL #

Patch Set 3 : rebase to ToT #

Total comments: 1

Created: 4 years ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+577 lines, -258 lines)			Patch
M	cc/layers/video_layer_impl.cc	View		2 chunks	+7 lines, -12 lines	0 comments	Download
M	cc/output/gl_renderer.h	View		2 chunks	+4 lines, -3 lines	0 comments	Download
M	cc/output/gl_renderer.cc	View	1	7 chunks	+52 lines, -2 lines	0 comments	Download
M	cc/output/gl_renderer_draw_cache.h	View		2 chunks	+2 lines, -0 lines	0 comments	Download
M	cc/output/renderer_pixeltest.cc	View		15 chunks	+199 lines, -75 lines	0 comments	Download
M	cc/output/shader.h	View		3 chunks	+49 lines, -20 lines	0 comments	Download
M	cc/output/shader.cc	View		5 chunks	+89 lines, -36 lines	1 comment	Download
M	cc/resources/resource_provider.h	View		2 chunks	+0 lines, -3 lines	0 comments	Download
M	cc/resources/resource_provider.cc	View		3 chunks	+0 lines, -18 lines	0 comments	Download
M	cc/resources/video_resource_updater.h	View		2 chunks	+4 lines, -0 lines	0 comments	Download
M	cc/resources/video_resource_updater.cc	View		7 chunks	+44 lines, -18 lines	0 comments	Download
M	cc/test/data/yuv_stripes.png	View		Binary file		0 comments	Download
M	cc/test/data/yuv_stripes_alpha.png	View		Binary file		0 comments	Download
M	cc/test/data/yuv_stripes_clipped.png	View		Binary file		0 comments	Download
A	cc/test/data/yuv_stripes_clipped_rgba.png	View		Binary file		0 comments	Download
M	cc/test/data/yuv_stripes_offset.png	View		Binary file		0 comments	Download
A	cc/test/data/yuv_stripes_rgba.png	View		Binary file		0 comments	Download
M	cc/test/render_pass_test_utils.cc	View		1 chunk	+2 lines, -1 line	0 comments	Download
M	cc/test/test_in_process_context_provider.h	View		2 chunks	+10 lines, -0 lines	0 comments	Download
M	cc/test/test_in_process_context_provider.cc	View		1 chunk	+4 lines, -0 lines	0 comments	Download
M	components/test_runner/test_plugin.cc	View		1 chunk	+3 lines, -1 line	0 comments	Download
M	content/browser/renderer_host/delegated_frame_host.cc	View		1 chunk	+2 lines, -1 line	0 comments	Download
M	content/test/gpu/gpu_tests/pixel_expectations.py	View	1 2	1 chunk	+14 lines, -0 lines	0 comments	Download
M	content/test/gpu/gpu_tests/pixel_test_pages.py	View	1 2	3 chunks	+7 lines, -7 lines	0 comments	Download
M	media/renderers/skcanvas_video_renderer.h	View		1 chunk	+2 lines, -0 lines	0 comments	Download
M	media/renderers/skcanvas_video_renderer.cc	View		5 chunks	+79 lines, -60 lines	0 comments	Download
M	third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp	View		1 chunk	+4 lines, -1 line	0 comments	Download

Messages

Total messages: 28 (17 generated)

Expand Messages | Collapse Messages | Show Generated Messages | Hide Generated Messages

dshwang

enne@, could you review this CL again? Sorry for delay of reland. I come back ...

4 years ago (2016-12-02 00:42:10 UTC) #8

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.

enne (OOO)

Ok, lgtm if the only change is that added conditional. Thanks for following up on ...

4 years ago (2016-12-02 18:17:37 UTC) #11

dshwang

On 2016/12/02 18:17:37, enne wrote: > Ok, lgtm if the only change is that added ...

4 years ago (2016-12-02 18:19:55 UTC) #12

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2547683003/20001

4 years ago (2016-12-02 18:20:29 UTC) #14

commit-bot: I haz the power

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)

4 years ago (2016-12-02 18:26:38 UTC) #16

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2547683003/40001

4 years ago (2016-12-02 23:16:02 UTC) #19

commit-bot: I haz the power

Patchset 3 (id:??) landed as https://crrev.com/60d375f8a6bd7b8c3e3abfd3771a228f3039bf62 Cr-Commit-Position: refs/heads/master@{#436118}

4 years ago (2016-12-03 01:21:00 UTC) #24

ynovikov

A revert of this CL (patchset #3 id:40001) has been created in https://codereview.chromium.org/2544893006/ by ynovikov@chromium.org. ...

4 years ago (2016-12-03 04:41:13 UTC) #25

aleksandar.stojiljkovic

On 2016/12/03 04:41:13, ynovikov wrote: > A revert of this CL (patchset #3 id:40001) has ...

4 years ago (2016-12-03 10:04:09 UTC) #26

aleksandar.stojiljkovic

4 years ago (2016-12-03 11:20:39 UTC) #28

Message was sent while issue was closed.

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#new...
cc/output/shader.cc:1303: vec2 tex_clamped =
max(tex_clamp_rect[int(v_index)].xy,
On 2016/12/03 10:04:09, aleksandar.stojiljkovic wrote:
> On 2016/12/03 04:41:13, ynovikov wrote:
> > A revert of this CL (patchset #3 id:40001) has been created in
> > https://codereview.chromium.org/2544893006/ by mailto:ynovikov@chromium.org.
> > 
> > The reason for reverting is: I suspect this breaks
> >
>
https://build.chromium.org/p/chromium.gpu.fyi/builders/Win7%20Release%20%28AM...
> > 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.

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/translato...

Expand Messages | Collapse Messages | Show Generated Messages | Hide Generated Messages