New issue
Advanced search Search tips

Issue 843346 link

Starred by 2 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on:
issue angleproject:2935



Sign in to add a comment

VK_KHR_xlib_surface and VK_LAYER_LUNARG_standard_validation are not compatible

Project Member Reported by spang@chromium.org, May 15 2018

Issue description

Enable the validation layers under X11 results in errors whenever any VkSurfaceKHR object created via VK_KHR_xlib_surface are used.

(null)(ERROR / SPEC): msgNum: -1 - Instance Extension VK_KHR_xlib_surface is not supported by this layer.  Using this extension may adversely affect validation results and/or produce undefined behavior.

[152346:152346:0515/174651.640546:343954747360:ERROR:vulkan_instance.cc(24)] Object: 0x1ff962de8a20 (Type = 26) | Invalid SurfaceKHR Object 0x1ff962de8a20. The spec valid usage text states 'surface must be a valid VkSurfaceKHR handle' (https://www.khronos.org/registry/vu$
kan/specs/1.0-extensions/html/vkspec.html#VUID-vkGetPhysicalDeviceSurfaceSupportKHR-surface-parameter)

Possible solutions
- Fix VK_LAYER_LUNARG_standard_validation to support xlib
- Switch to VK_KHR_xcb_surface. This is also supposed to improve performance

Until this is solved, we should not enable VK_LAYER_LUNARG_standard_validation under X by default in debug builds.
 
Project Member

Comment 1 by bugdroid1@chromium.org, May 15 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8fcdbf04ac7ff9fe0d0d3dcf125e41810959964c

commit 8fcdbf04ac7ff9fe0d0d3dcf125e41810959964c
Author: Michael Spang <spang@chromium.org>
Date: Tue May 15 23:03:21 2018

vulkan: Don't enable VK_LAYER_LUNARG_standard_validation under X11

Under X11 we use the VK_KHR_xlib_surface extension but this extension is
not supported by VK_LAYER_LUNARG_standard_validation. When these
extensions are used together, the validation layer never observes the
surface creation and fails the first time it is used.

Disable VK_LAYER_LUNARG_standard_validation so that debug builds can be
used with vulkan under X11.

Bug: 843346
Test: vulkan_tests with debug build

Change-Id: I4a42a8adb134a62043a14d416ae30f02e254cbd6
Reviewed-on: https://chromium-review.googlesource.com/1060459
Reviewed-by: Antoine Labour <piman@chromium.org>
Commit-Queue: Michael Spang <spang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558874}
[modify] https://crrev.com/8fcdbf04ac7ff9fe0d0d3dcf125e41810959964c/gpu/vulkan/vulkan_implementation.cc

Owner: spang@chromium.org
Status: Assigned (was: Untriaged)
Cc: penghuang@chromium.org
Project Member

Comment 4 by bugdroid1@chromium.org, Sep 28

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8e6e2a35563181b7ffbf8f9dca6e8d38280c209f

commit 8e6e2a35563181b7ffbf8f9dca6e8d38280c209f
Author: Chris Blume <cblume@chromium.org>
Date: Fri Sep 28 21:11:28 2018

Remove Vulkan validation layers from .gitignore

third_party/vulkan-validation-layers/src used to be populated by a DEPS
file in ANGLE. Because of this, it was added to the .gitignore to
prevent excess code downloading.

However, ANGLE later changed to pull its DEPS into
third_party/angle/third_party. That left
third_party/vulkan-validation-layers/src inside .gitignore without
actually being pulled in by DEPS.

This patch removes that directory from the .gitignore file so we can
later update the directory. This is needed to pull in a fix for a bug.

BUG=843346

Change-Id: I0563deadeef759af11f85dbdb0781238e82b21eb
Reviewed-on: https://chromium-review.googlesource.com/1252622
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Commit-Queue: Chris Blume <cblume@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595216}
[modify] https://crrev.com/8e6e2a35563181b7ffbf8f9dca6e8d38280c209f/third_party/.gitignore

Status update: I've locally tested using a newer copy of the Vulkan validation layers library and saw success. We will be able to re-enable the validation layers.

I tried landing this library update and found that our library moved from third_party/vulkan-validation-layers to third_party/angle/third_party/vulkan-validation-layers.

I've looked into a quick, hacky solution where gpu/vulkan depends on third_party/angle/third_party/vulkan-validation-layers but it seems to be more complex than I had hoped. There seem to be scripts that get run as part of the build and flags that get set as part of ANGLE's build.

I'm talking to some ANGLE folks to see how to best share this dependency or if we should have separate copies.
Project Member

Comment 6 by bugdroid1@chromium.org, Oct 26

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c8ac2e2aa0e6971cfbc60c487a45c0d6532ac10c

commit c8ac2e2aa0e6971cfbc60c487a45c0d6532ac10c
Author: Chris Blume <cblume@chromium.org>
Date: Fri Oct 26 01:03:54 2018

Reenable Vulkan validation

The Vulkan validation layer used to error when a platform-specific
surface (in this case, a X11 surface) was used.

A build flag which enables the validation layer to recognize that
platform-specific surface was missing.

When the Vulkan validation layers moved to ANGLE they also added this
build flag. As a result, we can re-enable the validation layers on X11.

BUG=843346

Change-Id: Ifb634a60a1cd9698b5cf51e03b426a58b4387a5d
Reviewed-on: https://chromium-review.googlesource.com/c/1300335
Reviewed-by: Eric Karl <ericrk@chromium.org>
Commit-Queue: Chris Blume <cblume@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602961}
[modify] https://crrev.com/c8ac2e2aa0e6971cfbc60c487a45c0d6532ac10c/gpu/vulkan/BUILD.gn
[modify] https://crrev.com/c8ac2e2aa0e6971cfbc60c487a45c0d6532ac10c/gpu/vulkan/vulkan_instance.cc

Blockedon: angleproject:2935
Project Member

Comment 8 by bugdroid1@chromium.org, Oct 30

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/da525455f43b2252fd13512d943b2fe54ac74e7d

commit da525455f43b2252fd13512d943b2fe54ac74e7d
Author: Chris Blume <cblume@chromium.org>
Date: Tue Oct 30 22:56:23 2018

Revert "Reenable Vulkan validation"

This reverts commit c8ac2e2aa0e6971cfbc60c487a45c0d6532ac10c.

Reason for revert: It seems like others are running into a problem caused by this. I'll investigate why I didn't see those same issues locally.

Original change's description:
> Reenable Vulkan validation
> 
> The Vulkan validation layer used to error when a platform-specific
> surface (in this case, a X11 surface) was used.
> 
> A build flag which enables the validation layer to recognize that
> platform-specific surface was missing.
> 
> When the Vulkan validation layers moved to ANGLE they also added this
> build flag. As a result, we can re-enable the validation layers on X11.
> 
> BUG=843346
> 
> Change-Id: Ifb634a60a1cd9698b5cf51e03b426a58b4387a5d
> Reviewed-on: https://chromium-review.googlesource.com/c/1300335
> Reviewed-by: Eric Karl <ericrk@chromium.org>
> Commit-Queue: Chris Blume <cblume@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#602961}

TBR=cblume@chromium.org,ericrk@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 843346
Change-Id: I28d17d41426102380c7cb246514c08ba462ae248
Reviewed-on: https://chromium-review.googlesource.com/c/1308770
Reviewed-by: Chris Blume <cblume@chromium.org>
Reviewed-by: Eric Karl <ericrk@chromium.org>
Commit-Queue: Chris Blume <cblume@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604044}
[modify] https://crrev.com/da525455f43b2252fd13512d943b2fe54ac74e7d/gpu/vulkan/BUILD.gn
[modify] https://crrev.com/da525455f43b2252fd13512d943b2fe54ac74e7d/gpu/vulkan/vulkan_instance.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Nov 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/14776798af09d5db8d71edf657ea34c42214e707

commit 14776798af09d5db8d71edf657ea34c42214e707
Author: Jamie Madill <jmadill@chromium.org>
Date: Mon Nov 05 01:19:59 2018

Remove unused README files.

These files were from an obsolete ANGLE integration.

Bug: 843346
Tbr: thakis@chromium.org
Change-Id: I630cf9c5126c979fa28c768f04cafb80dceb2083
Reviewed-on: https://chromium-review.googlesource.com/c/1316594
Commit-Queue: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Chris Blume <cblume@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605220}
[delete] https://crrev.com/8ecf5031139305dadb75c8b1a2b5ac7e080592a5/third_party/glslang-angle/README.chromium
[delete] https://crrev.com/8ecf5031139305dadb75c8b1a2b5ac7e080592a5/third_party/spirv-tools-angle/README.chromium
[delete] https://crrev.com/8ecf5031139305dadb75c8b1a2b5ac7e080592a5/third_party/vulkan-validation-layers/README.chromium

Project Member

Comment 10 by bugdroid1@chromium.org, Dec 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b287a98389eae716e41a3e79c60e4325a2f381ac

commit b287a98389eae716e41a3e79c60e4325a2f381ac
Author: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
Date: Wed Dec 05 22:22:27 2018

Remove glslang-angle and spirv-tools-angle from .gitignore and .gn

Both used to be populated by a DEPS file in ANGLE. Because of this, it was
added to the .gitignore to prevent excess code downloading.

However, ANGLE later changed to pull its DEPS into
third_party/angle/third_party. That left the versions in third_party
dangling for those who had checked them out before.

While here, remove references to those directories from //.gn

Bug: 843346
Change-Id: I5ba82ce74e2b40f4b89081f818eff0ac11fcffff
Reviewed-on: https://chromium-review.googlesource.com/c/1362936
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Chris Blume <cblume@chromium.org>
Commit-Queue: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
Cr-Commit-Position: refs/heads/master@{#614129}
[modify] https://crrev.com/b287a98389eae716e41a3e79c60e4325a2f381ac/.gn
[modify] https://crrev.com/b287a98389eae716e41a3e79c60e4325a2f381ac/third_party/.gitignore

Sign in to add a comment