|
|
DescriptionEnable SwiftShader fallback on desktop Linux.
SwiftShader is an OpenGL ES implementation which runs on the CPU,
and is used as a fallback for WebGL support when the GPU or its driver
have been blacklisted. It has been added to the Chrome installer for
Windows in https://codereview.chromium.org/2715563002/ This CL enables
the integration in Chromium on Linux.
BUG=160392
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Review-Url: https://codereview.chromium.org/2777193002
Cr-Commit-Position: refs/heads/master@{#463872}
Committed: https://chromium.googlesource.com/chromium/src/+/d85baf0b71c69bbd181aaefc8a803611e03c8eed
Patch Set 1 #Patch Set 2 : Exclude chromeos #Patch Set 3 : Update Linux expectations #Patch Set 4 : Restrict to x86 #Patch Set 5 : Rebase #Patch Set 6 : Ignore accelerated canvas preliminary blacklisting. #Patch Set 7 : Allow GPU process when accelerated 2D canvas blacklisted. #Patch Set 8 : Adjust GPU process expectations for Linux #Patch Set 9 : If GpuAccessAllowed() returns true, reason is empty #Patch Set 10 : Preserve GPU vendor/device IDs. #Patch Set 11 : Fix UpdateActiveGpu unit test. #Patch Set 12 : Fix UpdateActiveGpu again. #Patch Set 13 : Enable SwiftShader fallback on desktop Linux. #Patch Set 14 : Disable SwiftShader for active GPU tests. #Patch Set 15 : Mark all GPUs inactive. #Patch Set 16 : Fix compile. #Patch Set 17 : Fix new UpdateActiveGpu expectation. #
Total comments: 2
Patch Set 18 : Info collected successfully. #
Total comments: 2
Messages
Total messages: 106 (93 generated)
The CQ bit was checked by capn@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: 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_presub...)
The CQ bit was checked by capn@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: 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_presub...)
The CQ bit was checked by capn@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: 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_presub...)
Description was changed from ========== DO NOT SUBMIT This change is to check for Subzero build issues on Linux. CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel BUG= ========== to ========== DO NOT SUBMIT This change is to check for Subzero build issues on Linux. CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel BUG= ==========
capn@chromium.org changed reviewers: + sugoi@chromium.org
FYI
The CQ bit was checked by capn@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: 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_presub...)
The CQ bit was checked by capn@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: 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_presub...)
The CQ bit was checked by capn@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: 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_presub...)
The CQ bit was checked by capn@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: 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_presub...)
The CQ bit was checked by capn@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: 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_presub...)
The CQ bit was checked by capn@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: 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_presub...)
The CQ bit was checked by capn@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: 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_presub...)
The CQ bit was checked by capn@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: 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_presub...)
The CQ bit was checked by capn@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by capn@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: 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_presub...)
The CQ bit was checked by capn@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: 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_presub...)
The CQ bit was checked by capn@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: 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_presub...)
The CQ bit was checked by capn@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== DO NOT SUBMIT This change is to check for Subzero build issues on Linux. CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel BUG= ========== to ========== Enable SwiftShader fallback on desktop Linux. BUG=160392 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ==========
The CQ bit was checked by capn@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Enable SwiftShader fallback on desktop Linux. BUG=160392 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== Enable SwiftShader fallback on desktop Linux. BUG=160392 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ==========
The CQ bit was checked by capn@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== Enable SwiftShader fallback on desktop Linux. BUG=160392 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== Enable SwiftShader fallback on desktop Linux. SwiftShader is an OpenGL ES implementation which runs on the CPU, and is used as a fallback for WebGL support when the GPU or its driver have been blacklisted. It has been added to the Chrome installer for Windows in https://codereview.chromium.org/2715563002/ This CL enables the integration in Chromium on Linux. BUG=160392 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ==========
capn@chromium.org changed reviewers: + kbr@chromium.org
capn@chromium.org changed reviewers: + mmoss@chromium.org
capn@chromium.org changed reviewers: + piman@chromium.org
PTAL
The CQ bit was checked by capn@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by capn@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by capn@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Updated to not leave any GPU as active when using SwiftShader. PTAL.
Dry run: This issue passed the CQ dry run.
Updated to not leave any GPU as active when using SwiftShader. PTAL.
https://codereview.chromium.org/2777193002/diff/320001/content/browser/gpu/gp... File content/browser/gpu/gpu_data_manager_impl_private.cc (right): https://codereview.chromium.org/2777193002/diff/320001/content/browser/gpu/gp... content/browser/gpu/gpu_data_manager_impl_private.cc:1278: // to prevent attempts to use them (crbug.com/702417). Could we add a method on GPUInfo to do these things? I worry that as fields are added to that class, this function will not be maintained. Also, could we have a test that verifies that the values returned by SwiftShader are consistent with the values we hard-code here? https://codereview.chromium.org/2777193002/diff/320001/content/browser/gpu/gp... content/browser/gpu/gpu_data_manager_impl_private.cc:1294: gpu_info_.context_info_state = gpu::kCollectInfoNone; nit: kCollectInfoSuccess for both of these, since we have effectively "collected" the info (by hard-coding it), so we don't want to try to collect it again.
The CQ bit was checked by capn@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
On 2017/04/10 at 23:58:55, piman wrote: > https://codereview.chromium.org/2777193002/diff/320001/content/browser/gpu/gp... > File content/browser/gpu/gpu_data_manager_impl_private.cc (right): > > https://codereview.chromium.org/2777193002/diff/320001/content/browser/gpu/gp... > content/browser/gpu/gpu_data_manager_impl_private.cc:1278: // to prevent attempts to use them (crbug.com/702417). > Could we add a method on GPUInfo to do these things? I worry that as fields are added to that class, this function will not be maintained. We plan on overhauling the GPU info collection to make it more consistent between SwiftShader and other OpenGL implementations. I don't think adding a method to GPUInfo just for setting hard-coded SwiftShader information is a productive step in that direction. Also note that currently GPUInfo is a POD struct with no other methods. Rest assured that we'll turn the info collection into something more elegant and robust as we improve the support multiple OpenGL implementations on a single system. > Also, could we have a test that verifies that the values returned by SwiftShader are consistent with the values we hard-code here? Note that previously we just cleared all the info. So this is just intended to be an incremental improvement on that. We're working toward collecting the info directly from SwiftShader. For now our priority is just to enable SwiftShader on Linux in M59, as it's already enabled on Windows (with blank GPUInfo). > https://codereview.chromium.org/2777193002/diff/320001/content/browser/gpu/gp... > content/browser/gpu/gpu_data_manager_impl_private.cc:1294: gpu_info_.context_info_state = gpu::kCollectInfoNone; > nit: kCollectInfoSuccess for both of these, since we have effectively "collected" the info (by hard-coding it), so we don't want to try to collect it again. Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Any chance this could still make it into M59? We're trying to coordinate having SwiftShader integrated in Chrome on Windows and Linux, as well as in Firefox, in the same release cycle. PTAL
LGTM if you're blocked, but please strongly consider adding a unit test as a follow up.
Sorry for the delay reviewing this. LGTM. Two minor comments. https://codereview.chromium.org/2777193002/diff/340001/content/browser/gpu/gp... File content/browser/gpu/gpu_data_manager_impl_private.cc (right): https://codereview.chromium.org/2777193002/diff/340001/content/browser/gpu/gp... content/browser/gpu/gpu_data_manager_impl_private.cc:1281: gpu_info_.driver_date = "2017/04/07"; Please file a follow-on bug about obtaining this information more automatically. It will fall out of date almost immediately. https://codereview.chromium.org/2777193002/diff/340001/content/browser/gpu/gp... File content/browser/gpu/gpu_data_manager_impl_private_unittest.cc (right): https://codereview.chromium.org/2777193002/diff/340001/content/browser/gpu/gp... content/browser/gpu/gpu_data_manager_impl_private_unittest.cc:812: EXPECT_FALSE(observer.gpu_info_updated()); It's a little confusing to me that the EXPECT_TRUEs which were previously used for these two tests when SwiftShader was active are now changed to EXPECT_FALSEs, but I assume the reason is understood.
lgtm
The CQ bit was checked by capn@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
On 2017/04/11 at 22:03:45, kbr wrote: > Sorry for the delay reviewing this. LGTM. Two minor comments. > > https://codereview.chromium.org/2777193002/diff/340001/content/browser/gpu/gp... > File content/browser/gpu/gpu_data_manager_impl_private.cc (right): > > https://codereview.chromium.org/2777193002/diff/340001/content/browser/gpu/gp... > content/browser/gpu/gpu_data_manager_impl_private.cc:1281: gpu_info_.driver_date = "2017/04/07"; > Please file a follow-on bug about obtaining this information more automatically. It will fall out of date almost immediately. Done: crbug.com/swiftshader/43 > https://codereview.chromium.org/2777193002/diff/340001/content/browser/gpu/gp... > File content/browser/gpu/gpu_data_manager_impl_private_unittest.cc (right): > > https://codereview.chromium.org/2777193002/diff/340001/content/browser/gpu/gp... > content/browser/gpu/gpu_data_manager_impl_private_unittest.cc:812: EXPECT_FALSE(observer.gpu_info_updated()); > It's a little confusing to me that the EXPECT_TRUEs which were previously used for these two tests when SwiftShader was active are now changed to EXPECT_FALSEs, but I assume the reason is understood. It's indeed well understood. The tests were modified to disable falling back to SwiftShader to test the true intent of switching "active" GPUs without depending on the machine the tests are run on. We already have other tests for checking the fallback behavior.
CQ is committing da patch. Bot data: {"patchset_id": 340001, "attempt_start_ts": 1491957819110050, "parent_rev": "8593ecbda0931253208bdd2adfe4deaadb26423c", "commit_rev": "d85baf0b71c69bbd181aaefc8a803611e03c8eed"}
Message was sent while issue was closed.
Description was changed from ========== Enable SwiftShader fallback on desktop Linux. SwiftShader is an OpenGL ES implementation which runs on the CPU, and is used as a fallback for WebGL support when the GPU or its driver have been blacklisted. It has been added to the Chrome installer for Windows in https://codereview.chromium.org/2715563002/ This CL enables the integration in Chromium on Linux. BUG=160392 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== Enable SwiftShader fallback on desktop Linux. SwiftShader is an OpenGL ES implementation which runs on the CPU, and is used as a fallback for WebGL support when the GPU or its driver have been blacklisted. It has been added to the Chrome installer for Windows in https://codereview.chromium.org/2715563002/ This CL enables the integration in Chromium on Linux. BUG=160392 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel Review-Url: https://codereview.chromium.org/2777193002 Cr-Commit-Position: refs/heads/master@{#463872} Committed: https://chromium.googlesource.com/chromium/src/+/d85baf0b71c69bbd181aaefc8a80... ==========
Message was sent while issue was closed.
Committed patchset #18 (id:340001) as https://chromium.googlesource.com/chromium/src/+/d85baf0b71c69bbd181aaefc8a80... |