Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 75 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,80 @@
# Contributing to vulkan_rust

## Core Principle

Maintainability is the #1 concern for any new code contributed to this project. Every decision, from naming to structure to abstraction, should prioritize making the code easy to read, understand, and change in the future.

---

## Coding Standards

### Architecture

- **High cohesion, low coupling.** Each module has one clear responsibility. Functions and modules that change together live together; functions and modules that do not change together are separated.
- **Directed dependency hierarchy.** Dependencies flow one way: no circular imports, no coupling between sibling modules except through a shared interface.
- **Few top-level components.** Minimise the number of modules at each layer. A flat, wide structure is harder to navigate than a shallow hierarchy with clear boundaries.
- **Module size limit: 400 lines of production code.** Test code (`#[cfg(test)]` blocks, `tests/` files) does not count toward this limit. If a module's non-test code exceeds 400 lines, it is doing too much. Extract cohesive sub-functionality into a dedicated module.
- **No premature generalisation.** Solve the problem in front of you. Do not add abstractions, traits, or configuration knobs for requirements that do not yet exist.

### Functions

- **One level of abstraction per function.** A function that does high-level orchestration delegates all detail to sub-functions. A function that does low-level computation does not also orchestrate.
- **Extract cohesive sub-functionality into named sub-functions.** If a block of code inside a function deserves a comment to explain what it does, it deserves a name and a function boundary instead.
- **Short functions.** A function that does not fit on one screen is too long.
- **No side effects unless the function name says so.** `compute_*` and `get_*` are pure. `store_*`, `update_*`, and `create_*` have side effects.

### Naming

- **Names must be precise and short.** A long identifier is a sign the concept is not well understood.
- **No abbreviations** unless the abbreviation is universally understood in this domain (e.g. `vk`, `cmd`, `gpu`).
- **No redundant context in names.** If the module is `pipeline`, the function is `create`, not `create_pipeline`.

### Constants

- **No magic constants.** Every numeric or string literal that encodes a domain decision belongs in a named constant at the top of the module or in a dedicated `constants` module.
- **Name constants for what they mean, not what they are.** `MIN_API_VERSION: u32 = 1`, not `ONE`.

### Comments

- **No bad comments.** Do not state what the code does; state why it does it if the reason is not obvious.
- **No commented-out code.** Dead code goes in version control history, not in the file.
- **No dead code.** Unused functions, unused imports, unreachable branches, delete them.

### Error Handling

- **No swallowed errors.** Catching a `Result` and discarding the error (or only logging) is only acceptable if you explicitly document why silence is correct.
- **Let errors propagate** to the boundary where they can be meaningfully handled. Use `?` to propagate, do not match and re-wrap without adding context.
- **Fail loudly at invalid inputs.** Use `assert!`, `debug_assert!`, or return a descriptive error at function entry when preconditions are violated.

### Testing

#### Philosophy

- **Tests are first-class code.** They live in `#[cfg(test)]` modules or `tests/`, are subject to the same quality standards as production code, and are committed with the feature they test.
- **Testing shows presence of defects, not absence.** A passing test suite is not a proof of correctness. Design tests to find bugs, not to confirm the code runs.
- **Exhaustive testing is impossible.** Apply risk-based prioritisation: identify the modules most likely to fail and concentrate effort there. 80% of defects cluster in 20% of the code.
- **Test early, test often.** Write tests before or alongside the code they cover, not after.

#### What to test

- **All non-trivial logic must have an automated test.** If a function makes a decision, branches on data, or computes a non-obvious result, it is tested.
- **Trivial code (field access, pass-through delegation) does not need a dedicated test.**
- **Prioritise high-risk modules.** Code generation, Vulkan object lifecycle, and API surface construction are high risk. Re-exports and type aliases are lower risk.
- **Representative sample over exhaustive coverage.** Choose test cases that cover:
- The normal / expected path
- Boundary conditions (empty input, single element, maximum values)
- Known failure modes (null handles, invalid enum variants, missing extensions)

#### How to write tests

- **One behaviour per test function.** A test that checks multiple independent behaviours hides which one failed.
- **Test names describe the scenario and expected outcome.** `test_create_instance_returns_error_when_layer_missing` not `test_instance_1`.
- **No logic in tests.** Loops, conditionals, and complex setup inside a test make it untrustworthy.
- **Shared setup in helper functions.** If three tests need the same fixture, put it in a helper function, not copy-pasted in each test body.
- **Do not mock unless crossing a real boundary** (Vulkan driver, filesystem, network). Do not mock internal helper functions.

---

## Documentation Template

All new public items in `vulkan-rust` must follow this section order:
Expand Down
8 changes: 4 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# vulkan-rust

<img align="right" width="20%" src="logo.png">
<img align="right" width="20%" src="https://raw.githubusercontent.com/Hiddentale/vulkan_rust/main/logo.png">

[![CI](https://img.shields.io/github/actions/workflow/status/Hiddentale/vulkan_rust/ci.yml?branch=main&logo=github&label=CI)](https://github.com/Hiddentale/vulkan_rust/actions)
[![docs.rs](https://img.shields.io/docsrs/vulkan-rust?logo=docs.rs&label=docs.rs)](https://docs.rs/vulkan-rust)
Expand All @@ -23,9 +23,9 @@ If you're new to Vulkan, start with the [Hello Triangle tutorial](https://hidden

| | vulkan-rust | ash | vulkanalia |
|---|---|---|---|
| Command style | **Inherent methods** | Trait-based | Trait-based |
| `from_raw_parts` | **Yes, dedicated API** | Yes | No |
| Documentation | **100% coverage, spec links, examples** | Spec links only | Spec links + tutorial |
| Command style | Inherent methods | Trait-based | Trait-based |
| `from_raw_parts` | Yes, dedicated API | Yes | No |
| Documentation | 100% coverage, spec links, examples | Spec links only | Spec links + tutorial |
| Command loading | All enabled extensions | All enabled extensions | All enabled extensions |
| `no_std` (sys crate) | Yes | Yes | Yes |

Expand Down
4 changes: 2 additions & 2 deletions vulkan-rust-codegen/src/emit_aliases.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,8 @@ pub fn emit_func_pointer_stubs(registry: &VkRegistry) -> TokenStream {
quote! { #(#stubs)* }
}

/// Returns true if the StdVideo type is used by value (not pointer) in any struct.
/// By-value StdVideo types are C enums (i32), not opaque structs.
/// By-value StdVideo types are C enums (i32) that can be aliased directly,
/// while pointer-only types are opaque structs that need a stub definition.
fn is_by_value_stdvideo(registry: &VkRegistry, type_name: &str) -> bool {
registry.structs.iter().any(|s| {
s.members
Expand Down
2 changes: 1 addition & 1 deletion vulkan-rust-codegen/src/emit_builders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ fn emit_builder(
}

impl #struct_name {
/// Returns a builder for this struct with sType pre-filled.
/// Start building this struct; `s_type` is already set to the correct variant.
#[inline]
pub fn builder<'a>() -> #builder_name<'a> {
#builder_name {
Expand Down
7 changes: 3 additions & 4 deletions vulkan-rust-codegen/src/emit_enums.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,10 +139,9 @@ pub fn enum_variant_prefix(rust_type_name: &str) -> String {
format!("VK_{}_", base.to_shouty_snake_case())
}

/// Strip a C variant name like `VK_FORMAT_R8G8B8A8_SRGB` given prefix `VK_FORMAT_`.
/// Returns the Rust constant name like `R8G8B8A8_SRGB`.
///
/// Also strips trailing extension suffixes from the result if present.
/// Converts a C enum variant to idiomatic Rust by removing the type prefix
/// and any trailing extension suffix (e.g. `_KHR`), so variants read cleanly
/// when qualified by the Rust type name.
pub fn strip_variant_prefix(c_name: &str, prefix: &str) -> Option<String> {
let stripped = if let Some(s) = c_name.strip_prefix(prefix) {
s
Expand Down
6 changes: 4 additions & 2 deletions vulkan-rust-codegen/src/emit_layout_check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ fn untestable_extensions(registry: &VkRegistry) -> HashSet<String> {
.collect()
}

/// Returns true if this struct is available in standard vulkan.h.
/// Platform-specific and Vulkan SC structs are excluded from layout tests
/// because the C compiler may not have their definitions.
fn is_testable(s: &StructDef, skip_extensions: &HashSet<String>) -> bool {
if s.members.is_empty() || is_opaque(&s.name) {
return false;
Expand All @@ -46,7 +47,8 @@ fn is_testable(s: &StructDef, skip_extensions: &HashSet<String>) -> bool {
}
}

/// Returns true if the struct has any bit-field members.
/// Bitfield structs need manual layout verification since `offset_of!`
/// cannot address individual bitfields.
fn has_bitfields(s: &StructDef) -> bool {
s.members.iter().any(|m| m.is_bitfield)
}
Expand Down
3 changes: 2 additions & 1 deletion vulkan-rust-codegen/src/emit_structs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ pub fn is_base_pnext_struct(name: &str) -> bool {
BASE_PNEXT_STRUCTS.contains(&name)
}

/// Returns true if the struct has both sType and pNext members.
/// Structs with sType/pNext participate in extension chains and need
/// special handling in builders and Default impls.
pub fn has_stype_pnext(def: &StructDef) -> bool {
def.members.iter().any(|m| m.name == "sType") && def.members.iter().any(|m| m.name == "pNext")
}
Expand Down
Loading
Loading