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

Issue 1446453003: Generate list of GPU contexts outside SurfaceTest tests (Closed)

Created:
5 years, 1 month ago by Kimmo Kinnunen
Modified:
5 years, 1 month ago
Reviewers:
mtklein, bsalomon
CC:
reviews_skia.org
Base URL:
https://skia.googlesource.com/skia.git@master
Target Ref:
refs/heads/master
Project:
skia
Visibility:
Public.

Description

Generate list of GPU contexts outside SurfaceTest tests Add support for feeding the tests with contexts directly to the unit test framework. This fixes the problem where tests are more complex than needed just in order to run the test code with multiple backends. Also makes it possible to change the logic how contexts are created. Instead of direct numbering, the different testable contexts may be generated from filtered cross-product of context options. For example: currently NVPR is a type of context. However, it could be also an on/off feature of any context. In order to test this kind of context, the enumeration can not be just of context type. It's simpler to move the enumeration out of the tests. A test targeting both normal and GPU backends would look like: static void test_obj_behavior(skiatest::Reporter* reporter, SkObj* obj, [other params] ) { ... test with obj and param .. } DEF_TEST(ObjBehavior, reporter) { for (auto& object : generate_object) { for (auto& other_param : generate_other_variant) { test_obj_behavior(reporter, object, other_param); } } } #if SK_SUPPORT_GPU DEF_GPUTEST_FOR_ALL_CONTEXTS(ObjBehavior_Gpu, reporter, context) { for (auto& object : generate_gpu_object) { for (auto& other_param : generate_other_variant) { test_obj_behavior(reporter, object, other_param); } } } #endif Uses the feature in SurfaceTests as an example. Moves SkSurface -related tests from ImageTest to SurfaceTest. BUG=skia:2992 Committed: https://skia.googlesource.com/skia/+/179a8f5f7feab052e24596d0d04ab5cf2ccab5e0

Patch Set 1 #

Patch Set 2 : #

Patch Set 3 : remove ImageTest modifications #

Total comments: 2

Patch Set 4 : add support for using glContext in the GPU test #

Patch Set 5 : #

Patch Set 6 : #

Patch Set 7 : #

Patch Set 8 : #

Total comments: 1

Patch Set 9 : msvc fixes #

Unified diffs Side-by-side diffs Delta from patch set Stats (+573 lines, -404 lines) Patch
M dm/DM.cpp View 1 2 3 4 5 1 chunk +61 lines, -0 lines 0 comments Download
M src/gpu/GrContextFactory.h View 1 2 3 4 5 6 7 2 chunks +28 lines, -20 lines 0 comments Download
M src/gpu/GrContextFactory.cpp View 1 2 3 4 5 6 7 2 chunks +11 lines, -12 lines 0 comments Download
M tests/ImageTest.cpp View 1 2 2 chunks +0 lines, -70 lines 0 comments Download
M tests/SurfaceTest.cpp View 1 2 3 4 5 6 7 8 8 chunks +356 lines, -302 lines 0 comments Download
M tests/Test.h View 1 2 3 4 5 6 7 8 3 chunks +42 lines, -0 lines 0 comments Download
A tests/TestTest.cpp View 1 2 3 4 1 chunk +75 lines, -0 lines 0 comments Download

Messages

Total messages: 28 (12 generated)
Kimmo Kinnunen
Would this clarify or complexify current tests?
5 years, 1 month ago (2015-11-13 13:31:25 UTC) #2
bsalomon
On 2015/11/13 13:31:25, Kimmo Kinnunen wrote: > Would this clarify or complexify current tests? I ...
5 years, 1 month ago (2015-11-16 14:12:08 UTC) #5
Kimmo Kinnunen
On 2015/11/16 14:12:08, bsalomon wrote: > On 2015/11/13 13:31:25, Kimmo Kinnunen wrote: > > Would ...
5 years, 1 month ago (2015-11-16 17:48:26 UTC) #6
bsalomon
On 2015/11/16 17:48:26, Kimmo Kinnunen wrote: > On 2015/11/16 14:12:08, bsalomon wrote: > > On ...
5 years, 1 month ago (2015-11-17 05:03:27 UTC) #7
Kimmo Kinnunen
On 2015/11/17 05:03:27, bsalomon wrote: > Gotcha, is it possible to leave the tests in ...
5 years, 1 month ago (2015-11-17 10:23:11 UTC) #8
bsalomon
https://codereview.chromium.org/1446453003/diff/40001/dm/DM.cpp File dm/DM.cpp (right): https://codereview.chromium.org/1446453003/diff/40001/dm/DM.cpp#newcode1171 dm/DM.cpp:1171: test(reporter, context); I'm in the process of adding tests ...
5 years, 1 month ago (2015-11-17 14:49:37 UTC) #10
Kimmo Kinnunen
On 2015/11/17 14:49:37, bsalomon wrote: > https://codereview.chromium.org/1446453003/diff/40001/dm/DM.cpp > File dm/DM.cpp (right): > > https://codereview.chromium.org/1446453003/diff/40001/dm/DM.cpp#newcode1171 > ...
5 years, 1 month ago (2015-11-17 17:55:38 UTC) #11
Kimmo Kinnunen
On 2015/11/17 14:49:37, bsalomon wrote: > https://codereview.chromium.org/1446453003/diff/40001/dm/DM.cpp > File dm/DM.cpp (right): > > https://codereview.chromium.org/1446453003/diff/40001/dm/DM.cpp#newcode1171 > ...
5 years, 1 month ago (2015-11-18 09:56:32 UTC) #12
Kimmo Kinnunen
https://codereview.chromium.org/1446453003/diff/140001/tests/TestTest.cpp File tests/TestTest.cpp (right): https://codereview.chromium.org/1446453003/diff/140001/tests/TestTest.cpp#newcode69 tests/TestTest.cpp:69: DEF_GPUTEST_FOR_ALL_CONTEXTS(TestGpuGrContextAndGLContext, reporter, context, glContext) { so would this work ...
5 years, 1 month ago (2015-11-18 13:12:24 UTC) #13
bsalomon
lgtm
5 years, 1 month ago (2015-11-18 15:02:17 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1446453003/140001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1446453003/140001
5 years, 1 month ago (2015-11-19 06:36:34 UTC) #16
commit-bot: I haz the power
Try jobs failed on following builders: Build-Ubuntu-GCC-x86_64-Release-Trybot on client.skia.compile (JOB_FAILED, http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-x86_64-Release-Trybot/builds/4336)
5 years, 1 month ago (2015-11-19 06:37:56 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1446453003/160001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1446453003/160001
5 years, 1 month ago (2015-11-19 09:02:14 UTC) #21
commit-bot: I haz the power
Try jobs failed on following builders: Build-Ubuntu-GCC-x86_64-Release-Trybot on client.skia.compile (JOB_FAILED, http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-x86_64-Release-Trybot/builds/4337)
5 years, 1 month ago (2015-11-19 09:03:42 UTC) #23
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1446453003/160001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1446453003/160001
5 years, 1 month ago (2015-11-20 07:09:48 UTC) #27
commit-bot: I haz the power
5 years, 1 month ago (2015-11-20 21:32:29 UTC) #28
Message was sent while issue was closed.
Committed patchset #9 (id:160001) as
https://skia.googlesource.com/skia/+/179a8f5f7feab052e24596d0d04ab5cf2ccab5e0

Powered by Google App Engine
This is Rietveld 408576698