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

rewrite: Absolute basics #713

Merged
merged 1 commit into from
Mar 13, 2023
Merged

rewrite: Absolute basics #713

merged 1 commit into from
Mar 13, 2023

Conversation

Friz64
Copy link
Contributor

@Friz64 Friz64 commented Mar 2, 2023

This PR intends to lay a groundwork for the rewrite.

Here are my changes

  • Added an analysis crate, so that in the future code other than ash's generator can work with the API registry.
  • Other small miscellaneous changes.

@Friz64 Friz64 force-pushed the rewrite branch 3 times, most recently from 1ab5e3d to c45953c Compare March 2, 2023 20:58
@Friz64 Friz64 changed the base branch from update to master March 2, 2023 20:59
@Friz64
Copy link
Contributor Author

Friz64 commented Mar 2, 2023

Responding to early feedback about this PR, I've changed it to keep the old generator in the code base as a reference point and I've changed the base branch to master so that we have a working version of the current generator to build off of. Now we even might not even need to have a rewrite branch.

@Jerrody
Copy link

Jerrody commented Mar 4, 2023

@Friz64
Very cool!
vibe-rabbit (1)

Copy link
Collaborator

@MarijnS95 MarijnS95 left a comment

Choose a reason for hiding this comment

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

Nice and simple, this should allow us to more easily compare the new generated version to the old one. Which of course only makes sense if the new one looks similar to the old one... and that is exactly the feedback I have heard/received over time: ash works well for many users as a low-level no-frills and they really wouldn't want to deal with (much) more API breakage nor see any need for it. In other words, and as said before I really don't intend to rewrite/redesign ash, only the generator needs to be rewritten to deal with increasing complexity in vk.xml resulting in an unmaintainable hunk of code. Changes to ash itself should mostly be incremental, such as perhaps autogenerating the hand-written helpers, including more/better documentation, properly guarding experimental APIs behind cfg and so forth.

For this PR that mainly comes down to leaving README.md intact, especially if we merge this to master instead of instantiating a special rewrite branch.

And if folks don't align with my view here, let's discuss. Incremental changes/improvements to ash is still rather broad, and I am open to breaking changes.

.github/workflows/ci.yml Outdated Show resolved Hide resolved
@Friz64 Friz64 marked this pull request as ready for review March 8, 2023 05:56
@Friz64
Copy link
Contributor Author

Friz64 commented Mar 8, 2023

Okay! Now it probably makes more sense to directly work from master, so: Reverted the README.md change.

@Friz64
Copy link
Contributor Author

Friz64 commented Mar 8, 2023

Looks like CI broke because raw-window-handle 0.5.1 got released with rust-windowing/raw-window-handle#107 which bumps its MSRV to 1.64.

@MarijnS95
Copy link
Collaborator

Ugh, I'm also running into that MSRV break on the ndk crate. Don't feel like bumping the MSRV for a patch dependency update (that we "could" opt out of) but raw-window-handle 0.5.1 provides some new features and drops cty in favour of the stabilized ffi types.

@MarijnS95
Copy link
Collaborator

@Friz64 I'm tackling the MSRV bump in #716 for just ash-window. In fact I think we might bump ash back down to 1.59 as I only bumped it to 1.60 for winit 0.28 initially which isn't used in ash's crate graph anyway (and 1.59 is still enough for #715 🥳).

@Friz64
Copy link
Contributor Author

Friz64 commented Mar 13, 2023

FYI I rebased this.

@MarijnS95
Copy link
Collaborator

Let's get it in then and get this show on the road!

@MarijnS95 MarijnS95 merged commit eaf140f into ash-rs:master Mar 13, 2023
@MarijnS95 MarijnS95 changed the title rewrite: Absolute basics rewrite: Absolute basics Mar 27, 2023
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