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

Added a GLFW/Vulkan backend. #4

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

grahamreeds
Copy link

Note: I developed this in a container as not to pollute my wider WSL installation.

This may not work as intended as I am currently having trouble with the project that intends to consume this having trouble building and having it exist in its workspace (another docker container).

@giladreich
Copy link
Owner

Hi @grahamreeds, thanks for the PR! :) I'll try to have a look sometime this week or weekend and hopefully merge it if all good.

Copy link
Owner

@giladreich giladreich left a comment

Choose a reason for hiding this comment

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

Hi @grahamreeds, I just briefly had a look at your changes.

First off, many thanks for your efforts! It looks great for the most part.

Secondly, I'm afraid there is a problem with my initial approach of how I implemented the dependent cmake options (cmake_dependent_option). This will lead to other problems as we add more platforms and renderers (backends) in the long run.

I believe since we're externalizing this library and there could be so many combinations between the variant of Platforms and Renderers (e.g. SDL2 platform could run with dx{9,10,11,12}, vulkan, opengl{2,3} or sdlrenderer{2,3} etc.. renderers), then the existing approach won't scale. Therefore we need to completely separate the examples from the main build. This means if I know now I'm going to use ImGui with SDL2 and DX11, then the scope of the build should only be for what I chose.

I also remember I added myself a note here:

# TODO: Validate configurations based on the given input combination

to add validation for the given options, since I probably realized at the time that this doesn't scale.

Let me think about it more. Maybe tomorrow in the evening I may have some time to look or weekend and update this repo a little and add more examples.

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.

2 participants