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

Add Image! docs to book #652

Merged
merged 2 commits into from
Jun 9, 2021
Merged

Add Image! docs to book #652

merged 2 commits into from
Jun 9, 2021

Conversation

khyperia
Copy link
Contributor

@khyperia khyperia commented Jun 7, 2021

Fixes #603

@khyperia khyperia requested a review from XAMPPRocky as a code owner June 7, 2021 08:33
@khyperia khyperia requested review from eddyb and removed request for XAMPPRocky June 7, 2021 08:33
Copy link
Contributor

@DJMcNab DJMcNab left a comment

Choose a reason for hiding this comment

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

This looks good, although I might have found the need for a debugging mission into our docs generation. Sorry 😅.

The specific syntax and meaning of the arguments to the `Image!` macro can be found in
[rustdoc](https://embarkstudios.github.io/rust-gpu/api/spirv_std/macro.Image.html).

Some type aliases for common image formats can be found in the `spirv_std::image` module. For
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this also link to the module docs? i.e. something like

Suggested change
Some type aliases for common image formats can be found in the `spirv_std::image` module. For
Some type aliases for common image formats can be found in the [`spirv_std::image`](https://embarkstudios.github.io/rust-gpu/api/spirv_std/index.html) module. For

However, something strange is afoot. That page lists the version as 0.4.0-alpha.9, the latest version. However, it does not list the image module as a seperate module, and lists all the image types as types at the root.

Also, the [src] of Image2d is shown as this, which is afaik not correct.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is because the docs are not generated with the const_generics feature, and so the new Image2d doesn't show up (the new one is in the image module)

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I see sorry. That also solves my other confusion.

what you want if you want a regular old sampled texture.

```rust,no_run
type Image2d = Image!(2D, type=f32, sampled);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
type Image2d = Image!(2D, type=f32, sampled);
// In spirv_std::image
pub type Image2d = Image!(2D, type=f32, sampled);

type Image2d = Image!(2D, type=f32, sampled);
```

Note that the `const-generics` feature must be enabled to use the `Image!` macro at the moment.
Copy link
Contributor

@DJMcNab DJMcNab Jun 7, 2021

Choose a reason for hiding this comment

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

It looks like the feature name is const_generics.

Suggested change
Note that the `const-generics` feature must be enabled to use the `Image!` macro at the moment.
Note that the `const-generics` cargo feature must be enabled to use the `Image!` macro at the moment.
```toml
[dependencies]
spirv-std={..., features=["const-generics"]}
```

Copy link
Contributor

@DJMcNab DJMcNab Jun 7, 2021

Choose a reason for hiding this comment

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

I see now that this is a cargo feature, so this would be a Cargo.toml example instead.

@khyperia khyperia enabled auto-merge (squash) June 9, 2021 10:07
@khyperia khyperia merged commit 64380d6 into main Jun 9, 2021
@khyperia khyperia deleted the image-macro-book-docs branch June 9, 2021 10:18
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.

Document Image! in book
3 participants