Improve documentation for ash::Device and ash::Instance

[hoj-senna]

Closes #576.

[MarijnS95]

Thanks for contributing this PR to self-resolve your reported issue, but I'm not really fond of the way the text is written.

For starters, render the docs yourself and make sure all intradocs-links are correct and consistent (don't repeat link text, backticks go in the link name, more [currently monospaced] words can be converted to links) and then we can commence the review from there.

Perhaps it's easier if I respin this PR instead of requesting a change on every line, let me know what you prefer 😉

[hoj-senna]

Thank you!

I have attempted to improve the links.

In some places I have not turned monospaced words into links, because I don't think that, for example, the text explaining ash::Device should have (self-referential) links to ash::Device.

If respinning the PR is easier/faster for you, please go ahead with that.

[MarijnS95]

Seems like we're getting somewhere.

In some places I have not turned monospaced words into links, because I don't think that, for example, the text explaining ash::Device should have (self-referential) links to ash::Device.

I'd rather have links here like [`Device`][Self] than even mentioning the ash:: module prefix since you're already reading the documentation inside that module (in other words, this is an unnecessary module qualification), or rewrite the docs to refer to "this" / "the current type being documented" instead of spelling out its name.

[MarijnS95]

Instead of saying what it's not, how about saying what it is? (and don't forget backicks inside [] links).

Suggested change

	/// It is not identical to Vulkan's device object (see [vk::Device]).
	/// The latter, however, can be accessed via the [`handle()`][Self::handle()] method.
	/// In this sense, `ash::Device` is a Vulkan device handle together with all its associated function pointers.
	/// Holds a Vulkan [`vk::Device`] handle together with Vulkan 1.0 - 1.3 device function pointers loaded from it, providing convenient wrappers to call these functions.
	///
	/// The internal handle is available through [`handle()`][Self::handle()] if needed externally.

[MarijnS95]

I don't really like the "whose main purpose is" bit here. The fact that it provides access to Vulkan functions is already described in the next paragraph, and what's the other "non main" purpose you're hinting at here?

[hoj-senna]

Apparrently, there is no other purpose (which I did not know before your question).

[MarijnS95]

This is not entirely correct. First argument can also be a derivative from the current device (ie. associated with one unique device) such as a CommandBuffer, PhysicalDevice, Queue etc. I'd argue just having the paragraph above with the device function pointers loaded from it mention is enough, unless you feel like linking to the Vulkan object model docs and mentioning this device relationship is necessary.

[MarijnS95]

Also not sure if referencing VkDevice from the Vulkan docs is very helpful, pointing to the Rust version here is more type-correct (only reference Vulkan docs if there's something actually useful to be read there, and be explicit about it providing additional information).

And in any case, since this first argument is passed implicitly internally I don't think we should bother the user with that detail - but if you feel like this being necessary, expand on that in the providing convenient wrappers to call these functions sentence.

[hoj-senna]

This is not entirely correct. First argument can also be a derivative from the current device

I'd say that does not contradict the sentence which only talks about the functions with VkDevice as first argument. (It does make my description incomplete; and I have removed this sentence.)

About "bothering the user": You can see this as an internal implementation detail of ash — or as one of the most striking differences between ash and the Vulkan docs to which we often refer, which could (should?) be made explicit somewhere.

[MarijnS95]

They are device functions, no need to mention them explicitly? Otherwise you should also make an exception for all other functions not taking Device but a "derivative" of Device as first argument.

[MarijnS95]

Suggested change

[MarijnS95]

Same comments here as above.

[hoj-senna]

I'd rather have links here like [`Device`][Self] than even mentioning the ash:: module prefix since you're already reading the documentation inside that module (in other words, this is an unnecessary module qualification)

My perspective here was that I wanted to provide a comparison (or "translation guide") between the Vulkan API docs (and Vulkan tutorials etc.) and ash. Since most of ash's documentation (rightly) consists in links to the Vulkan specification and ash::Device is one of the few "additional" things, it seemed to make sense. And in this context, it also seemed reasonable to spell out where I'm talking about ash::Device and where about the VkDevice of the (C) Vulkan API. So, I did not find the module qualification unnecessary.

In this sense, I also disagree with (the second part of) this:

Also not sure if referencing VkDevice from the Vulkan docs is very helpful, pointing to the Rust version here is more type-correct

The objects I was talking about (or at least: wanted to be talking about) where those from the Vulkan docs, not their Rusty ash counterparts.
But the links are not really helpful, that's true.

I have now changed the documentation of Device according to your suggestion (although I have changed "device function pointers" to "device-level function pointers", because I did not find "device function" by searching in the Vulkan docs, and there is at least an entry "Device-Level Command" in the Glossary);

I have chosen a version that expands on "providing convenient wrappers to call these functions" in the documentation of Instance. (The point of "First argument can also be a derivative" etc. shouldn't apply here.)