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.

Sign up for GitHub

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

Jump to bottom

Updated BackendSelector API #7090

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Updated BackendSelector API #7090

wants to merge 1 commit into from
+518 −218

Conversation

tronical
Copy link
Member

Based on API review.

@tronical
Copy link
Member Author

Still marked as draft to also see CI issues.

@tronical tronical force-pushed the simon/backend-selector branch 11 times, most recently from 62cb054 to 3d370e1 Compare December 13, 2024 15:55
@tronical
Revamp BackendSelector API
2b274d5 
Based on API review, PlatformBuilder becomes BackendSelector with
a slightly smaller API surface but more options, such as selecting
Metal or Vulkan rendering.
@tronical tronical force-pushed the simon/backend-selector branch from 3d370e1 to 2b274d5 Compare December 13, 2024 15:56
@tronical tronical marked this pull request as ready for review December 13, 2024 16:03
@tronical tronical requested a review from ogoffart December 13, 2024 16:03
@tronical
Copy link
Member Author

There might be CI issues left, but I think the API is ready for review now.

ogoffart
ogoffart reviewed Dec 13, 2024
View reviewed changes
Copy link
Member

@ogoffart ogoffart left a comment

Choose a reason for hiding this comment

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

Should we select() on Drop ?
I fear that a missing call to select is going to be an unnoticed problem.

@tronical
Copy link
Member Author

We could. I mean, select on drop if select wasn’t called earlier? And panic if there is an error?

So use would be either:

  • end call with select() and possibly handle error
  • don’t call select() but implicitly select on drop and panic on error

Is that correct?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ogoffart ogoffart ogoffart left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@tronical @ogoffart