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

OpenXR: Separate graphics handling from main OpenXR code #243

Merged
merged 3 commits into from
Aug 27, 2024

Conversation

msub2
Copy link
Contributor

@msub2 msub2 commented Aug 27, 2024

This removes the explicit dependency on D3D11 in the OpenXR code and should hopefully lay the groundwork for future support on other platforms like Linux or Android.

pub enum GraphicsProvider {}

pub trait GraphicsProviderMethods<G: Graphics> {
fn enable_graphics_extensions(exts: &mut ExtensionSet);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think it is better if you use associated type, for example:

type GraphicsType: Graphics;

    fn create_session(
        device: &SurfmanDevice,
        instance: &Instance,
        system: SystemId,
    ) -> Result<(Session<Self::GraphicsType>, FrameWaiter, FrameStream<Self::GraphicsType>), Error>;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the PR code is strictly equivalent, and I don't have a good sense for whether we should prefer one style over the other when there's no behaviour difference like this case.

Copy link
Member

@jdm jdm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice! I admit that I find it easy to mix up the GraphicsProvider enum and the GraphicsProviderType typedef, so maybe we should rename the second one to Backend instead?

pub enum GraphicsProvider {}

pub trait GraphicsProviderMethods<G: Graphics> {
fn enable_graphics_extensions(exts: &mut ExtensionSet);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the PR code is strictly equivalent, and I don't have a good sense for whether we should prefer one style over the other when there's no behaviour difference like this case.

@msub2
Copy link
Contributor Author

msub2 commented Aug 27, 2024

Very nice! I admit that I find it easy to mix up the GraphicsProvider enum and the GraphicsProviderType typedef, so maybe we should rename the second one to Backend instead?

That's fair yeah, I'll adjust that today.

@jdm jdm added this pull request to the merge queue Aug 27, 2024
Merged via the queue into servo:main with commit 7656508 Aug 27, 2024
6 checks passed
@msub2 msub2 deleted the openxr/graphics-provider branch August 27, 2024 20:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants