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

[WIP] A third gfx example #2072

Merged
merged 2 commits into from
Jul 21, 2018
Merged

[WIP] A third gfx example #2072

merged 2 commits into from
Jul 21, 2018

Conversation

goddessfreya
Copy link
Contributor

Depends on #2071 and #2052
PR checklist:

  • make succeeds (on *nix)
  • make reftests succeeds
  • tested examples with the following backends:

@goddessfreya goddessfreya changed the title A third gfx example [WIP] A third gfx example May 28, 2018
@goddessfreya
Copy link
Contributor Author

I'll reopen this when I work on more examples again.

@goddessfreya goddessfreya reopened this Jul 16, 2018
@goddessfreya goddessfreya force-pushed the extra-example branch 2 times, most recently from 58b7c0d to d485efd Compare July 17, 2018 04:19
Copy link
Member

@kvark kvark left a comment

Choose a reason for hiding this comment

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

Can we get around using the takeable-option and manually dropped things?

@goddessfreya
Copy link
Contributor Author

goddessfreya commented Jul 19, 2018

We can get around manually dropping things by ordering the members in the struct definitions in the order we want to drop them. I feel, however, that manually drop helps document the order in which you should be dropping things. I think you'll agree that It would be annoying if changing the order of the members in a struct caused bugs.

As for takeable, no. The hal destroy functions want ownership of the struct members & drop only gets an &mut of the structs.

@kvark
Copy link
Member

kvark commented Jul 19, 2018

We can get around manually dropping things by ordering the members in the struct definitions in the order we want to drop them.

Do you really have to drop things in a specific order with HAL? AFAIK, Vulkan says destruction order can be arbitrary.

@goddessfreya
Copy link
Contributor Author

goddessfreya commented Jul 20, 2018

Other than the swapchain, no. I removed all the ManuallyDrops and just made the swapchain Takeable.

@kvark
Copy link
Member

kvark commented Jul 20, 2018

Thank you!
I'm still not sure we need takeable_option magic though. Would the code be much worse if it had a few regular Option members of structs? In the worst case, we'll have something like as_mut().unwrap() in a place or two, but using a standard construct (Option) is way friendlier.

@goddessfreya
Copy link
Contributor Author

@kvark You can compare the two versions and see which you prefer. I personally like Takeable (else I wouldn't have made it).

@kvark
Copy link
Member

kvark commented Jul 20, 2018

@zegentzy thank you, looks great! Let's fix the CI issues and get this merged.

Signed-off-by: Hal Gentz <[email protected]>
@goddessfreya
Copy link
Contributor Author

@kvark Done.

@kvark
Copy link
Member

kvark commented Jul 21, 2018 via email

bors bot added a commit that referenced this pull request Jul 21, 2018
2072: [WIP] A third gfx example r=kvark a=ZeGentzy

Depends on #2071 and #2052 
PR checklist:
- [ ] `make` succeeds (on *nix)
- [ ] `make reftests` succeeds
- [ ] tested examples with the following backends:


Co-authored-by: Hal Gentz <[email protected]>
@bors
Copy link
Contributor

bors bot commented Jul 21, 2018

@bors bors bot merged commit d5d38e6 into gfx-rs:master Jul 21, 2018
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