Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove debug ext only when added by loader #129

Merged
merged 4 commits into from Sep 10, 2019

Conversation

@rpavlik
Copy link
Contributor

commented Sep 4, 2019

Fixes #124 without the side-effects of the other approach (that is, this is an alternate to #123)

cc @yl-msft

This can be tested with Monado - the master branch reports the support for the extension (though it's not fully implemented), while https://gitlab.freedesktop.org/monado/monado/commits/no-debug-messenger has it disabled for testing purposes.

To test, you can modify hello_xr to require the debug extension as follows:

Find this code in openxr_program.cpp:

        // Create union of extensions required by platform and graphics plugins.
        std::vector<const char*> extensions;

and change it to this:

        // Create union of extensions required by platform and graphics plugins.
        std::vector<const char*> extensions = {XR_EXT_DEBUG_UTILS_EXTENSION_NAME};

Note that the InstanceCreateInfoManager is intended to be re-usable in other layers, etc that may have a need for pruning the extension list. Also, the current state in this branch is that the extension is removed from the create info even if there are layers that report support for it (none of the built-in ones do so despite actually working with it?) - this might need some fine-tuning. However, it will avoid spurious failures.

rpavlik added 4 commits Sep 3, 2019
loader: Filter out enabled extensions that are provided only by the l…
…oader.

Code is intended to be somewhat re-usable in other layers.
loader: Simplify extension removing.
It's slightly less efficient, but this happens very rarely (just at instance creation)
and the lists of extensions and extensions-to-suppress are both very small,
so readability wins out over efficiency.

@rpavlik rpavlik added the in loader label Sep 4, 2019

@rpavlik rpavlik requested a review from daveh-lunarg Sep 4, 2019

@yl-msft
yl-msft approved these changes Sep 4, 2019
return Update();
}
// Remove the extension named in the parameter and return a pointer to the current state.
const XrInstanceCreateInfo* FilterOutExtension(const char* extension_to_skip) {

This comment has been minimized.

Copy link
@yl-msft

yl-msft Sep 4, 2019

Contributor

Nit: it seems the Get() and FilterOutExtension (w/o 's') functions can be private.

This comment has been minimized.

Copy link
@rpavlik

rpavlik Sep 5, 2019

Author Contributor

Those are intended for use in "re-use" of this class elsewhere. e.g. if there's a layer providing an overlay extension, it would want to filter out just that extension before passing things down the line.

@rpavlik rpavlik merged commit ffcca49 into KhronosGroup:master Sep 10, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
license/cla Contributor License Agreement is signed.
Details

@rpavlik rpavlik deleted the rpavlik:remove-debug-ext-loader branch Sep 10, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.