Fix Vulkan crashes on nexus player
This CL enables Vulkan Test bots on the NexusPlayer
TBR:bsalomon@google.com
BUG=skia:
Change-Id: I0cc8b01107afcc84e0cb0631f361f3615fc8dd9b
Reviewed-on: https://skia-review.googlesource.com/5681
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
diff --git a/include/gpu/GrShaderCaps.h b/include/gpu/GrShaderCaps.h
index e1f5747..918138c 100644
--- a/include/gpu/GrShaderCaps.h
+++ b/include/gpu/GrShaderCaps.h
@@ -164,13 +164,16 @@
bool usesPrecisionModifiers() const { return fUsesPrecisionModifiers; }
- // Returns whether we can use the glsl funciton any() in our shader code.
+ // Returns whether we can use the glsl function any() in our shader code.
bool canUseAnyFunctionInShader() const { return fCanUseAnyFunctionInShader; }
bool canUseMinAndAbsTogether() const { return fCanUseMinAndAbsTogether; }
bool mustForceNegatedAtanParamToFloat() const { return fMustForceNegatedAtanParamToFloat; }
+ // Returns whether a device incorrectly implements atan(y,x) as atan(y/x)
+ bool atan2ImplementedAsAtanYOverX() const { return fAtan2ImplementedAsAtanYOverX; }
+
bool requiresLocalOutputColorForFBFetch() const { return fRequiresLocalOutputColorForFBFetch; }
// Returns the string of an extension that must be enabled in the shader to support
@@ -299,6 +302,7 @@
// Used for specific driver bug work arounds
bool fCanUseMinAndAbsTogether : 1;
bool fMustForceNegatedAtanParamToFloat : 1;
+ bool fAtan2ImplementedAsAtanYOverX : 1;
bool fRequiresLocalOutputColorForFBFetch : 1;
PrecisionInfo fFloatPrecisions[kGrShaderTypeCount][kGrSLPrecisionCount];
diff --git a/infra/bots/gen_tasks.go b/infra/bots/gen_tasks.go
index 61b0900..89a6fe2 100644
--- a/infra/bots/gen_tasks.go
+++ b/infra/bots/gen_tasks.go
@@ -230,9 +230,9 @@
"Test-Android-Clang-NexusPlayer-CPU-Moorefield-x86-Debug-Android",
"Test-Android-Clang-NexusPlayer-CPU-Moorefield-x86-Release-Android",
"Test-Android-Clang-NexusPlayer-GPU-PowerVR-x86-Debug-Android",
- // "Test-Android-Clang-NexusPlayer-GPU-PowerVR-x86-Debug-Android_Vulkan",
+ "Test-Android-Clang-NexusPlayer-GPU-PowerVR-x86-Debug-Android_Vulkan",
"Test-Android-Clang-NexusPlayer-GPU-PowerVR-x86-Release-Android",
- // "Test-Android-Clang-NexusPlayer-GPU-PowerVR-x86-Release-Android_Vulkan",
+ "Test-Android-Clang-NexusPlayer-GPU-PowerVR-x86-Release-Android_Vulkan",
"Test-Android-Clang-PixelC-CPU-TegraX1-arm64-Debug-Android",
"Test-Android-Clang-PixelC-CPU-TegraX1-arm64-Release-Android",
"Test-Android-Clang-PixelXL-GPU-Adreno530-arm64-Debug-Android",
diff --git a/infra/bots/tasks.json b/infra/bots/tasks.json
index de6e96d..e81a5d6 100644
--- a/infra/bots/tasks.json
+++ b/infra/bots/tasks.json
@@ -1142,12 +1142,24 @@
"Upload-Test-Android-Clang-NexusPlayer-GPU-PowerVR-x86-Debug-Android"
]
},
+ "Test-Android-Clang-NexusPlayer-GPU-PowerVR-x86-Debug-Android_Vulkan": {
+ "priority": 0.8,
+ "tasks": [
+ "Upload-Test-Android-Clang-NexusPlayer-GPU-PowerVR-x86-Debug-Android_Vulkan"
+ ]
+ },
"Test-Android-Clang-NexusPlayer-GPU-PowerVR-x86-Release-Android": {
"priority": 0.8,
"tasks": [
"Upload-Test-Android-Clang-NexusPlayer-GPU-PowerVR-x86-Release-Android"
]
},
+ "Test-Android-Clang-NexusPlayer-GPU-PowerVR-x86-Release-Android_Vulkan": {
+ "priority": 0.8,
+ "tasks": [
+ "Upload-Test-Android-Clang-NexusPlayer-GPU-PowerVR-x86-Release-Android_Vulkan"
+ ]
+ },
"Test-Android-Clang-PixelC-CPU-TegraX1-arm64-Debug-Android": {
"priority": 0.8,
"tasks": [
@@ -9524,6 +9536,55 @@
"isolate": "test_skia.isolate",
"priority": 0.8
},
+ "Test-Android-Clang-NexusPlayer-GPU-PowerVR-x86-Debug-Android_Vulkan": {
+ "cipd_packages": [
+ {
+ "name": "skia/bots/skimage",
+ "path": "skimage",
+ "version": "version:19"
+ },
+ {
+ "name": "skia/bots/skp",
+ "path": "skp",
+ "version": "version:33"
+ },
+ {
+ "name": "skia/bots/svg",
+ "path": "svg",
+ "version": "version:3"
+ }
+ ],
+ "dependencies": [
+ "Build-Ubuntu-Clang-x86-Debug-Android_Vulkan"
+ ],
+ "dimensions": [
+ "device_os:N2G10B",
+ "device_type:fugu",
+ "os:Android",
+ "pool:Skia"
+ ],
+ "execution_timeout_ns": 14400000000000,
+ "expiration_ns": 72000000000000,
+ "extra_args": [
+ "--workdir",
+ "../../..",
+ "swarm_test",
+ "repository=<(REPO)",
+ "buildername=Test-Android-Clang-NexusPlayer-GPU-PowerVR-x86-Debug-Android_Vulkan",
+ "mastername=fake-master",
+ "buildnumber=2",
+ "slavename=fake-buildslave",
+ "nobuildbot=True",
+ "swarm_out_dir=${ISOLATED_OUTDIR}",
+ "revision=<(REVISION)",
+ "patch_storage=<(PATCH_STORAGE)",
+ "patch_issue=<(ISSUE)",
+ "patch_set=<(PATCHSET)"
+ ],
+ "io_timeout_ns": 2400000000000,
+ "isolate": "test_skia.isolate",
+ "priority": 0.8
+ },
"Test-Android-Clang-NexusPlayer-GPU-PowerVR-x86-Release-Android": {
"cipd_packages": [
{
@@ -9573,6 +9634,55 @@
"isolate": "test_skia.isolate",
"priority": 0.8
},
+ "Test-Android-Clang-NexusPlayer-GPU-PowerVR-x86-Release-Android_Vulkan": {
+ "cipd_packages": [
+ {
+ "name": "skia/bots/skimage",
+ "path": "skimage",
+ "version": "version:19"
+ },
+ {
+ "name": "skia/bots/skp",
+ "path": "skp",
+ "version": "version:33"
+ },
+ {
+ "name": "skia/bots/svg",
+ "path": "svg",
+ "version": "version:3"
+ }
+ ],
+ "dependencies": [
+ "Build-Ubuntu-Clang-x86-Release-Android_Vulkan"
+ ],
+ "dimensions": [
+ "device_os:N2G10B",
+ "device_type:fugu",
+ "os:Android",
+ "pool:Skia"
+ ],
+ "execution_timeout_ns": 14400000000000,
+ "expiration_ns": 72000000000000,
+ "extra_args": [
+ "--workdir",
+ "../../..",
+ "swarm_test",
+ "repository=<(REPO)",
+ "buildername=Test-Android-Clang-NexusPlayer-GPU-PowerVR-x86-Release-Android_Vulkan",
+ "mastername=fake-master",
+ "buildnumber=2",
+ "slavename=fake-buildslave",
+ "nobuildbot=True",
+ "swarm_out_dir=${ISOLATED_OUTDIR}",
+ "revision=<(REVISION)",
+ "patch_storage=<(PATCH_STORAGE)",
+ "patch_issue=<(ISSUE)",
+ "patch_set=<(PATCHSET)"
+ ],
+ "io_timeout_ns": 2400000000000,
+ "isolate": "test_skia.isolate",
+ "priority": 0.8
+ },
"Test-Android-Clang-PixelC-CPU-TegraX1-arm64-Debug-Android": {
"cipd_packages": [
{
@@ -15147,6 +15257,35 @@
"isolate": "upload_dm_results.isolate",
"priority": 0.8
},
+ "Upload-Test-Android-Clang-NexusPlayer-GPU-PowerVR-x86-Debug-Android_Vulkan": {
+ "dependencies": [
+ "Test-Android-Clang-NexusPlayer-GPU-PowerVR-x86-Debug-Android_Vulkan"
+ ],
+ "dimensions": [
+ "cpu:x86-64-avx2",
+ "gpu:none",
+ "os:Ubuntu-14.04",
+ "pool:Skia"
+ ],
+ "extra_args": [
+ "--workdir",
+ "../../..",
+ "upload_dm_results",
+ "repository=<(REPO)",
+ "buildername=Test-Android-Clang-NexusPlayer-GPU-PowerVR-x86-Debug-Android_Vulkan",
+ "mastername=fake-master",
+ "buildnumber=2",
+ "slavename=fake-buildslave",
+ "nobuildbot=True",
+ "swarm_out_dir=${ISOLATED_OUTDIR}",
+ "revision=<(REVISION)",
+ "patch_storage=<(PATCH_STORAGE)",
+ "patch_issue=<(ISSUE)",
+ "patch_set=<(PATCHSET)"
+ ],
+ "isolate": "upload_dm_results.isolate",
+ "priority": 0.8
+ },
"Upload-Test-Android-Clang-NexusPlayer-GPU-PowerVR-x86-Release-Android": {
"dependencies": [
"Test-Android-Clang-NexusPlayer-GPU-PowerVR-x86-Release-Android"
@@ -15176,6 +15315,35 @@
"isolate": "upload_dm_results.isolate",
"priority": 0.8
},
+ "Upload-Test-Android-Clang-NexusPlayer-GPU-PowerVR-x86-Release-Android_Vulkan": {
+ "dependencies": [
+ "Test-Android-Clang-NexusPlayer-GPU-PowerVR-x86-Release-Android_Vulkan"
+ ],
+ "dimensions": [
+ "cpu:x86-64-avx2",
+ "gpu:none",
+ "os:Ubuntu-14.04",
+ "pool:Skia"
+ ],
+ "extra_args": [
+ "--workdir",
+ "../../..",
+ "upload_dm_results",
+ "repository=<(REPO)",
+ "buildername=Test-Android-Clang-NexusPlayer-GPU-PowerVR-x86-Release-Android_Vulkan",
+ "mastername=fake-master",
+ "buildnumber=2",
+ "slavename=fake-buildslave",
+ "nobuildbot=True",
+ "swarm_out_dir=${ISOLATED_OUTDIR}",
+ "revision=<(REVISION)",
+ "patch_storage=<(PATCH_STORAGE)",
+ "patch_issue=<(ISSUE)",
+ "patch_set=<(PATCHSET)"
+ ],
+ "isolate": "upload_dm_results.isolate",
+ "priority": 0.8
+ },
"Upload-Test-Android-Clang-PixelC-CPU-TegraX1-arm64-Debug-Android": {
"dependencies": [
"Test-Android-Clang-PixelC-CPU-TegraX1-arm64-Debug-Android"
diff --git a/infra/config/recipes.cfg b/infra/config/recipes.cfg
index bf850ca..20c8529 100644
--- a/infra/config/recipes.cfg
+++ b/infra/config/recipes.cfg
@@ -23,5 +23,5 @@
project_id: "skia-recipes"
url: "https://skia.googlesource.com/skia-recipes.git"
branch: "master"
- revision: "7ab5cc6255f6bae0ce71241a26c509f557e9983b"
+ revision: "e2e91bd2def317153737460ec00b4266ff03e4ab"
}
diff --git a/src/effects/gradients/SkSweepGradient.cpp b/src/effects/gradients/SkSweepGradient.cpp
index 1608ae2..64778c3 100644
--- a/src/effects/gradients/SkSweepGradient.cpp
+++ b/src/effects/gradients/SkSweepGradient.cpp
@@ -210,8 +210,18 @@
SkString coords2D = args.fFragBuilder->ensureCoords2D(args.fTransformedCoords[0]);
SkString t;
// 0.1591549430918 is 1/(2*pi), used since atan returns values [-pi, pi]
- t.printf("(atan(- %s.y, - %s.x) * 0.1591549430918 + 0.5)",
- coords2D.c_str(), coords2D.c_str());
+ if (args.fShaderCaps->atan2ImplementedAsAtanYOverX()) {
+ // On some devices they incorrectly implement atan2(y,x) as atan(y/x). In actuality it is
+ // atan2(y,x) = 2 * atan(y / (sqrt(x^2 + y^2) + x)). So to work around this we pass in
+ // (sqrt(x^2 + y^2) + x) as the second parameter to atan2 in these cases. We let the device
+ // handle the undefined behavior of the second paramenter being 0 instead of doing the
+ // divide ourselves and using atan instead.
+ t.printf("(2.0 * atan(- %s.y, length(%s) - %s.x) * 0.1591549430918 + 0.5)",
+ coords2D.c_str(), coords2D.c_str(), coords2D.c_str());
+ } else {
+ t.printf("(atan(- %s.y, - %s.x) * 0.1591549430918 + 0.5)",
+ coords2D.c_str(), coords2D.c_str());
+ }
this->emitColor(args.fFragBuilder,
args.fUniformHandler,
args.fShaderCaps,
diff --git a/src/gpu/GrShaderCaps.cpp b/src/gpu/GrShaderCaps.cpp
index f9db968..54cd3e5 100644
--- a/src/gpu/GrShaderCaps.cpp
+++ b/src/gpu/GrShaderCaps.cpp
@@ -56,6 +56,7 @@
fCanUseAnyFunctionInShader = true;
fCanUseMinAndAbsTogether = true;
fMustForceNegatedAtanParamToFloat = false;
+ fAtan2ImplementedAsAtanYOverX = false;
fRequiresLocalOutputColorForFBFetch = false;
fFlatInterpolationSupport = false;
fNoPerspectiveInterpolationSupport = false;
diff --git a/src/gpu/vk/GrVkCaps.cpp b/src/gpu/vk/GrVkCaps.cpp
index f614ae3..383d3ca 100644
--- a/src/gpu/vk/GrVkCaps.cpp
+++ b/src/gpu/vk/GrVkCaps.cpp
@@ -19,6 +19,7 @@
fMustDoCopiesFromOrigin = false;
fSupportsCopiesAsDraws = false;
fMustSubmitCommandsBeforeCopyOp = false;
+ fMustSleepOnTearDown = false;
/**************************************************************************
* GrDrawTargetCaps fields
@@ -79,6 +80,16 @@
fMustSubmitCommandsBeforeCopyOp = true;
}
+#if defined(SK_BUILD_FOR_WIN)
+ if (kNvidia_VkVendor == properties.vendorID) {
+ fMustSleepOnTearDown = true;
+ }
+#elif defined(SK_BUILD_FOR_ANDROID)
+ if (kImagination_VkVendor == properties.vendorID) {
+ fMustSleepOnTearDown = true;
+ }
+#endif
+
this->applyOptionsOverrides(contextOptions);
fShaderCaps->applyOptionsOverrides(contextOptions);
}
@@ -163,6 +174,10 @@
}
}
+ if (kImagination_VkVendor == properties.vendorID) {
+ shaderCaps->fAtan2ImplementedAsAtanYOverX = true;
+ }
+
// Vulkan is based off ES 3.0 so the following should all be supported
shaderCaps->fUsesPrecisionModifiers = true;
shaderCaps->fFlatInterpolationSupport = true;
diff --git a/src/gpu/vk/GrVkCaps.h b/src/gpu/vk/GrVkCaps.h
index b390a9b..6d030f6 100644
--- a/src/gpu/vk/GrVkCaps.h
+++ b/src/gpu/vk/GrVkCaps.h
@@ -76,6 +76,10 @@
return fMustSubmitCommandsBeforeCopyOp;
}
+ bool mustSleepOnTearDown() const {
+ return fMustSleepOnTearDown;
+ }
+
/**
* Returns both a supported and most prefered stencil format to use in draws.
*/
@@ -87,6 +91,7 @@
enum VkVendor {
kQualcomm_VkVendor = 20803,
kNvidia_VkVendor = 4318,
+ kImagination_VkVendor = 4112,
};
void init(const GrContextOptions& contextOptions, const GrVkInterface* vkInterface,
@@ -136,6 +141,11 @@
// as draws.
bool fMustSubmitCommandsBeforeCopyOp;
+ // Sometimes calls to QueueWaitIdle return before actually signalling the fences
+ // on the command buffers even though they have completed. This causes an assert to fire when
+ // destroying the command buffers. Therefore we add a sleep to make sure the fence signals.
+ bool fMustSleepOnTearDown;
+
typedef GrCaps INHERITED;
};
diff --git a/src/gpu/vk/GrVkGpu.cpp b/src/gpu/vk/GrVkGpu.cpp
index 853e68c..b94a379 100644
--- a/src/gpu/vk/GrVkGpu.cpp
+++ b/src/gpu/vk/GrVkGpu.cpp
@@ -168,12 +168,13 @@
// destroying the command buffers. Currently this ony seems to happen on windows, so we add a
// sleep to make sure the fence signals.
#ifdef SK_DEBUG
+ if (this->vkCaps().mustSleepOnTearDown()) {
#if defined(SK_BUILD_FOR_WIN)
- Sleep(10); // In milliseconds
+ Sleep(10); // In milliseconds
#else
- // Uncomment if above bug happens on non windows build.
- // sleep(1); // In seconds
+ sleep(1); // In seconds
#endif
+ }
#endif
#ifdef SK_DEBUG