Skip to content

Commit

Permalink
Set disabled GL extensions from gpu driver bug list for the test exec…
Browse files Browse the repository at this point in the history
…utables

Apply disabled GL extensions from gpu driver bug list for all the gpu
related executables. This makes it possible to use the disabled extensions
in gpu driver bug list so that they apply to unit tests.

This is needed in order to implement and test features that are
likely to not work at all unless disabled extensions work.

BUG=344330

Review URL: https://codereview.chromium.org/1215063005

Cr-Commit-Position: refs/heads/master@{#372624}
  • Loading branch information
kkinnunen authored and Commit bot committed Feb 1, 2016
1 parent f3ba5db commit 394e596
Show file tree
Hide file tree
Showing 10 changed files with 81 additions and 40 deletions.
4 changes: 4 additions & 0 deletions content/gpu/gpu_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,10 @@ int GpuMain(const MainFunctionParams& parameters) {
// (Chrome OS, Android) or where workarounds may be dependent on GL_VENDOR
// and GL_RENDERER strings which are lazily computed (Linux).
if (!command_line.HasSwitch(switches::kDisableGpuDriverBugWorkarounds)) {
// TODO: this can not affect disabled extensions, since they're already
// initialized in the bindings. This should be moved before bindings
// initialization. However, populating GPUInfo fully works only on
// Android. Other platforms would need the bindings to query GL strings.
gpu::ApplyGpuDriverBugWorkarounds(
gpu_info, const_cast<base::CommandLine*>(&command_line));
}
Expand Down
6 changes: 5 additions & 1 deletion content/test/content_test_suite.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "content/public/common/content_client.h"
#include "content/public/common/content_paths.h"
#include "content/public/test/test_content_client_initializer.h"
#include "gpu/config/gpu_info_collector.h"
#include "gpu/config/gpu_util.h"
#include "testing/gtest/include/gtest/gtest.h"

Expand Down Expand Up @@ -99,8 +100,11 @@ void ContentTestSuite::Initialize() {
bool is_child_process = base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kTestChildProcess);
if (!is_child_process) {
gpu::GPUInfo gpu_info;
gpu::CollectBasicGraphicsInfo(&gpu_info);
gpu::ApplyGpuDriverBugWorkarounds(gpu_info,
base::CommandLine::ForCurrentProcess());
gfx::GLSurfaceTestSupport::InitializeOneOff();
gpu::ApplyGpuDriverBugWorkarounds(base::CommandLine::ForCurrentProcess());
}
#endif
testing::TestEventListeners& listeners =
Expand Down
9 changes: 3 additions & 6 deletions gpu/command_buffer/service/feature_info.cc
Original file line number Diff line number Diff line change
Expand Up @@ -522,10 +522,9 @@ void FeatureInfo::InitializeFeatures() {
// sized formats GL_SRGB8 and GL_SRGB8_ALPHA8. Also, SRGB_EXT isn't a valid
// <format> in this case. So, even with GLES3 explicitly check for
// GL_EXT_sRGB.
if (!workarounds_.disable_ext_srgb &&
(((gl_version_info_->is_es3 ||
extensions.Contains("GL_OES_rgb8_rgba8")) &&
extensions.Contains("GL_EXT_sRGB")) || gfx::HasDesktopGLFeatures())) {
if (((gl_version_info_->is_es3 || extensions.Contains("GL_OES_rgb8_rgba8")) &&
extensions.Contains("GL_EXT_sRGB")) ||
gfx::HasDesktopGLFeatures()) {
AddExtensionString("GL_EXT_sRGB");
validators_.texture_internal_format.AddValue(GL_SRGB_EXT);
validators_.texture_internal_format.AddValue(GL_SRGB_ALPHA_EXT);
Expand Down Expand Up @@ -1188,7 +1187,6 @@ void FeatureInfo::InitializeFeatures() {
}

if (enable_gl_path_rendering_switch_ &&
!workarounds_.disable_gl_path_rendering &&
extensions.Contains("GL_NV_path_rendering")) {
bool has_dsa = gl_version_info_->IsAtLeastGL(4, 5) ||
extensions.Contains("GL_EXT_direct_state_access");
Expand All @@ -1207,7 +1205,6 @@ void FeatureInfo::InitializeFeatures() {
}

if (enable_gl_path_rendering_switch_ &&
!workarounds_.disable_gl_path_rendering &&
extensions.Contains("GL_NV_framebuffer_mixed_samples")) {
AddExtensionString("GL_CHROMIUM_framebuffer_mixed_samples");
feature_flags_.chromium_framebuffer_mixed_samples = true;
Expand Down
6 changes: 5 additions & 1 deletion gpu/command_buffer/tests/gl_tests_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "base/test/launcher/unit_test_launcher.h"
#include "base/test/test_suite.h"
#include "gpu/command_buffer/client/gles2_lib.h"
#include "gpu/config/gpu_info_collector.h"
#include "gpu/config/gpu_util.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "ui/gl/gl_surface.h"
Expand All @@ -29,9 +30,12 @@ int RunHelper(base::TestSuite* testSuite) {
#else
base::MessageLoopForIO message_loop;
#endif
gpu::GPUInfo gpu_info;
gpu::CollectBasicGraphicsInfo(&gpu_info);
gpu::ApplyGpuDriverBugWorkarounds(gpu_info,
base::CommandLine::ForCurrentProcess());
gfx::GLSurface::InitializeOneOff();
::gles2::Initialize();
gpu::ApplyGpuDriverBugWorkarounds(base::CommandLine::ForCurrentProcess());
return testSuite->Run();
}

Expand Down
18 changes: 5 additions & 13 deletions gpu/config/gpu_driver_bug_list_json.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const char kGpuDriverBugListJson[] = LONG_STRING_CONST(
{
"name": "gpu driver bug list",
// Please update the version number whenever you change this file.
"version": "8.41",
"version": "8.42",
"entries": [
{
"id": 1,
Expand Down Expand Up @@ -1439,9 +1439,7 @@ LONG_STRING_CONST(
"op": "<",
"value": "346"
},
"features": [
"disable_gl_path_rendering"
]
"disabled_extensions": ["GL_NV_path_rendering"]
},
{
"id": 124,
Expand Down Expand Up @@ -1582,9 +1580,7 @@ LONG_STRING_CONST(
}
},
"gl_vendor": "Qualcomm.*",
"features": [
"disable_ext_srgb"
]
"disabled_extensions": ["GL_EXT_sRGB"]
},
{
"id": 135,
Expand Down Expand Up @@ -1635,9 +1631,7 @@ LONG_STRING_CONST(
"op": "<",
"value": "346"
},
"features": [
"disable_gl_path_rendering"
]
"disabled_extensions": ["GL_NV_path_rendering"]
},
{
"id": 139,
Expand Down Expand Up @@ -1670,9 +1664,7 @@ LONG_STRING_CONST(
},
"gl_vendor": "Qualcomm",
"gl_renderer": "Adreno \\(TM\\) 4.*", // Originally on 418.
"features": [
"disable_ext_srgb"
]
"disabled_extensions": ["GL_EXT_sRGB"]
},
{
"id": 141,
Expand Down
4 changes: 0 additions & 4 deletions gpu/config/gpu_driver_bug_workaround_type.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,6 @@
disable_discard_framebuffer) \
GPU_OP(DISABLE_EXT_DRAW_BUFFERS, \
disable_ext_draw_buffers) \
GPU_OP(DISABLE_EXT_SRGB, \
disable_ext_srgb) \
GPU_OP(DISABLE_GL_PATH_RENDERING, \
disable_gl_path_rendering) \
GPU_OP(DISABLE_GL_RGB_FORMAT, \
disable_gl_rgb_format) \
GPU_OP(DISABLE_MSAA_ON_NON_WEBGL_CONTEXTS, \
Expand Down
29 changes: 22 additions & 7 deletions gpu/config/gpu_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "base/logging.h"
#include "base/strings/string_number_conversions.h"
#include "base/strings/string_split.h"
#include "base/strings/string_util.h"
#include "gpu/config/gpu_control_list_jsons.h"
#include "gpu/config/gpu_driver_bug_list.h"
#include "gpu/config/gpu_info_collector.h"
Expand Down Expand Up @@ -53,13 +54,6 @@ void MergeFeatureSets(std::set<int>* dst, const std::set<int>& src) {
dst->insert(src.begin(), src.end());
}

void ApplyGpuDriverBugWorkarounds(base::CommandLine* command_line) {
GPUInfo gpu_info;
CollectBasicGraphicsInfo(&gpu_info);

ApplyGpuDriverBugWorkarounds(gpu_info, command_line);
}

void ApplyGpuDriverBugWorkarounds(const GPUInfo& gpu_info,
base::CommandLine* command_line) {
scoped_ptr<GpuDriverBugList> list(GpuDriverBugList::Create());
Expand All @@ -73,6 +67,27 @@ void ApplyGpuDriverBugWorkarounds(const GPUInfo& gpu_info,
command_line->AppendSwitchASCII(switches::kGpuDriverBugWorkarounds,
IntSetToString(workarounds));
}

std::set<std::string> disabled_extensions;
std::vector<std::string> buglist_disabled_extensions =
list->GetDisabledExtensions();
disabled_extensions.insert(buglist_disabled_extensions.begin(),
buglist_disabled_extensions.end());

if (command_line->HasSwitch(switches::kDisableGLExtensions)) {
std::vector<std::string> existing_disabled_extensions = base::SplitString(
command_line->GetSwitchValueASCII(switches::kDisableGLExtensions), " ",
base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
disabled_extensions.insert(existing_disabled_extensions.begin(),
existing_disabled_extensions.end());
}

if (!disabled_extensions.empty()) {
std::vector<std::string> v(disabled_extensions.begin(),
disabled_extensions.end());
command_line->AppendSwitchASCII(switches::kDisableGLExtensions,
base::JoinString(v, " "));
}
}

void StringToFeatureSet(
Expand Down
11 changes: 4 additions & 7 deletions gpu/config/gpu_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,11 @@ struct GPUInfo;
GPU_EXPORT void MergeFeatureSets(
std::set<int>* dst, const std::set<int>& src);

// Collect basic GPUInfo, compute the driver bug workarounds for the current
// system, and append the |command_line|.
GPU_EXPORT void ApplyGpuDriverBugWorkarounds(base::CommandLine* command_line);

// With provided GPUInfo, compute the driver bug workarounds for the current
// system, and append the |command_line|.
// With provided GPUInfo, compute the driver bug workarounds and disabled
// extensions for the current system, and append the |command_line|.
GPU_EXPORT void ApplyGpuDriverBugWorkarounds(
const GPUInfo& gpu_inco, base::CommandLine* command_line);
const GPUInfo& gpu_info,
base::CommandLine* command_line);

// |str| is in the format of "feature1,feature2,...,featureN".
GPU_EXPORT void StringToFeatureSet(
Expand Down
29 changes: 29 additions & 0 deletions gpu/config/gpu_util_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,14 @@
// found in the LICENSE file.

#include "gpu/config/gpu_util.h"
#include "base/memory/scoped_ptr.h"
#include "base/strings/string_split.h"
#include "gpu/config/gpu_control_list_jsons.h"
#include "gpu/config/gpu_driver_bug_list.h"
#include "gpu/config/gpu_info.h"
#include "gpu/config/gpu_info_collector.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "ui/gl/gl_switches.h"

namespace gpu {

Expand Down Expand Up @@ -68,4 +75,26 @@ TEST(GpuUtilTest, StringToFeatureSet) {
}
}

TEST(GpuUtilTest,
ApplyGpuDriverBugWorkarounds_DisabledExtensions) {
GPUInfo gpu_info;
CollectBasicGraphicsInfo(&gpu_info);
scoped_ptr<GpuDriverBugList> list(GpuDriverBugList::Create());
list->LoadList(kGpuDriverBugListJson, GpuControlList::kCurrentOsOnly);
list->MakeDecision(GpuControlList::kOsAny, std::string(), gpu_info);
std::vector<std::string> expected_disabled_extensions =
list->GetDisabledExtensions();
base::CommandLine command_line(base::CommandLine::NO_PROGRAM);
ApplyGpuDriverBugWorkarounds(gpu_info, &command_line);

std::vector<std::string> actual_disabled_extensions = base::SplitString(
command_line.GetSwitchValueASCII(switches::kDisableGLExtensions), ", ;",
base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
sort(expected_disabled_extensions.begin(),
expected_disabled_extensions.end());
sort(actual_disabled_extensions.begin(), actual_disabled_extensions.end());

EXPECT_EQ(expected_disabled_extensions, actual_disabled_extensions);
}

} // namespace gpu
5 changes: 4 additions & 1 deletion gpu/gles2_conform_support/egl/egl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "base/strings/utf_string_conversions.h"
#include "gpu/command_buffer/client/gles2_lib.h"
#include "gpu/command_buffer/service/gpu_switches.h"
#include "gpu/config/gpu_info_collector.h"
#include "gpu/config/gpu_util.h"
#include "gpu/gles2_conform_support/egl/display.h"
#include "ui/gl/gl_context.h"
Expand Down Expand Up @@ -135,7 +136,9 @@ EGLAPI EGLBoolean EGLAPIENTRY eglInitialize(EGLDisplay dpy,
// argc, argv in CommandLine::Init(argc, argv).
command_line->InitFromArgv(argv);
if (!command_line->HasSwitch(switches::kDisableGpuDriverBugWorkarounds)) {
gpu::ApplyGpuDriverBugWorkarounds(command_line);
gpu::GPUInfo gpu_info;
gpu::CollectBasicGraphicsInfo(&gpu_info);
gpu::ApplyGpuDriverBugWorkarounds(gpu_info, command_line);
}

gfx::GLSurface::InitializeOneOff();
Expand Down

0 comments on commit 394e596

Please sign in to comment.