Code Style and Conventions {#conventions}#
[TOC]
Here are some general code style guidelines we follow.
Note that we aim to "code with respect", to avoid terminology that may limit our community or hurt those in it, as well as to conform with emerging industry standards. Good guidelines to look to include the Android Coding with Respect policy and the Write Inclusive Documentation page from the Google developer documentation style guide. The latter also links to a word list for clear documentation, which, while not binding on this project, is useful in making sure your code, comments, and docs are understandable by the worldwide Monado community.
Changelog fragments#
In Monado we strongly prefer if all MRs merged into the main Monado repository also includes changelog fragments. A changelog fragment is a small file detailing the changes in the MR on a per area basis. They are slightly more detailed then a commit subject line, but usually just one or two lines, usually providing a little bit of context to the change. Sometimes for "big" changes they are more detailed and provide paragraph of description, such as for adding of new drivers, or large refactors. The changelog fragments are used to generate the @ref CHANGELOG file, updated on each release (and running on the CI). The changelog is generated with proclamation.
The changelog fragments are located in the doc/changes folder, organised into
sub-categories in subfolders. There isn't a 1-to-1 mapping of changelog fragment
to commit, but instead they are per change. A changelog fragment file is named
mr. + MR number + .md, for MR 1234 it would be named mr.1234.md. If a MR
has multiple changes for one sub-category a number is added between the MR
number and file extension, example mr.1234.1.md and mr.1234.2.md. If a
change spans multiple MRs, such as fixing a feature introduced in a earlier MR
we imply the use of YAML headers to mark a changelog fragment applying to
multiple MRs, can also be used to link issues.
---
- mr.1234
- issue.42
- mr.2000
---
Add cool feature X, it gives the answer to life, the universe and everything
else.
Generally the last commit in a MR adds the changelog fragments, as unfortunately MR numbers are allocated when opened, also provides a nice readable separation between MRs in the git history. Examples for commits adding changelog fragments can be seen here, here and here.
APIs#
Internal APIs, when it makes sense, should be C APIs. Headers that define
general communication interfaces between modules (not only use of utilities)
belong in the xrt/include/xrt directory, and should not depend on any other module outside
that directory. (As a historical note: this directory gets its name from a
compressed version of the phrase "XR RunTime", a generic term for Monado and an
early development codename. Also, it's shorter than monado_ and so nicer to
use in code.)
What follows are some basic API usage rules. Note that all the module usage relations must be expressed in the build system, so module usage should form a directed-acyclic-graph.
- Any module can implement or use APIs declared in
xrt/include/xrt - Any module (except the
xrtinterface headers themselves) can (and should!) use APIs declared inxrt/auxiliary/util. - Any module except for
auxiliary/utiland thexrtinterface headers themselves can use APIs declared in otherxrt/auxiliarymodules.
Naming#
- C APIs:
lower_snake_casefor types and functions.UPPER_SNAKE_CASEfor macros. e.g. @ref U_TYPED_CALLOC (which is how all allocations in C code should be performed)- Prefix names with a "namespace" - the library/module where they reside. e.g.
@ref u_var_add_root, @ref math_pose_validate
- Related: only things prefixed by
xrt_belong in thexrt/include/xrtdirectory, and nothing named starting withxrt_should be declared anywhere else. (Interfaces declared inxrt/include/xrtare implemented in other modules.)
- Related: only things prefixed by
- Generally, we do not declare typedefs for
structandenumtypes, but instead refer to them in long form, sayingstructorenumthen the name. The exception to not using typedefs is function pointers used as function arguments as these become very hard to both read and type out. - If a typedef is needed, it should be named ending with
_t. Function pointer typedefs should end with_func_t. - Parameters:
lower_snake_caseor acronyms.- Output parameters should begin with
out_. - Of special note: Structures/types that represent "objects" often have long
type names or "conceptual" names. When a pointer to them is passed to a
function or kept as a local variable, it is typically named by taking the
first letter of each (typically
_-delimited) word in the structure type name. Sometimes, it is an abbreviated form of that name instead. Relevant examples:- @ref xrt_comp_native_create_swapchain() is a member function of the
interface @ref xrt_compositor_native, and takes a pointer to that
interface named
xcn. It creates an @ref xrt_swapchain, which it populates in the parameter namedout_xscn:out_because it's a purely output parameter,xscnfrom @ref xrt_swapchain_native specifically the lettersXrt_SwapChain_Native. @ref xrt_swapchain and related types are a small exception to the rules - there are only 2 words if you go by the_delimiters, but for clarity we treat swapchain as if it were two words when abbreviating. A few other places in thexrtheaders usex+ an abbreviated name form, likexinstfor @ref xrt_instance,xdevfor @ref xrt_device,xsysdsometimes used for @ref xrt_system_devices.
- @ref xrt_comp_native_create_swapchain() is a member function of the
interface @ref xrt_compositor_native, and takes a pointer to that
interface named
- Output parameters should begin with
createanddestroyare used when the functions actually perform allocation and return the new object, or deallocation of the passed-in object.- If some initialization or cleanup is required but the type is not opaque and
is allocated by the caller, the names to use are
initand, if needed, one ofcleanup/fini/teardown. (We are not yet consistent on these names.) One common example is when there is some shared code and a structure partially implementing an interface: a further-derived object may need to call aninitfunction on the shared structure, but it was allocated by the derived object and held by value.
- C++:
- Where a C API is exposed, it should follow the C API naming schemes.
- If only a C++ API is exposed, a fairly conventional C++ naming scheme is used:
- Namespaces: nested to match directory structure, starting with
xrt::.- There are no C++ interfaces in the
xrt/include/xrt, by design, so this is not ambiguous. - Place types that need to be exposed in a header for technical reasons,
but that are still considered implementation details, within a
further-nested
detailnamespace, as seen elsewhere in the C++ ecosystem.
- There are no C++ interfaces in the
- Types/classes:
CamelCase - Methods/functions:
lowerCamelCase - Constants/constexpr values:
kCamelCase - If a header is only usable from C++ code, it should be named with the
extension
.hppto signify this.
- Namespaces: nested to match directory structure, starting with
- Math:
- For different types of transforms
Tbetween two entitiesAandB, try to use variable names likeT_A_Bto express the transform such thatB = T_A_B * A. This is equivalent to "Bexpressed w.r.t.A" and "the transform that converts a point inBcoordinates intoAcoordinates".Tcan be used for 4x4 isometry matrices, but you can use others likePfor poses,Rfor 3x3 rotations,Qfor quaternion rotations,tfor translations, etc.
- For different types of transforms
Patterns and Idioms#
This is an incomplete list of conventional idioms used in the Monado codebase.
C "Inheritance" through first struct member#
Despite being in C, the design is fairly object-oriented. Types implement
interfaces and derive from other types typically by placing a field of that
parent type/interface as their first element, conventionally named base. This
means that a pointer to the derived type, and a pointer to the base type, have
the same value.
For example, consider @ref client_gl_swapchain
- Its first element is named @ref client_gl_swapchain::base and is of type @ref xrt_swapchain_gl - meaning that it implements @ref xrt_swapchain_gl
- @ref xrt_swapchain_gl in turn starts with @ref xrt_swapchain_gl::base which is
@ref xrt_swapchain - meaning that @ref xrt_swapchain_gl extends @ref
xrt_swapchain. (Both @ref xrt_swapchain_gl and @ref xrt_swapchain are abstract
interfaces, as indicated by the
xrt_prefix.)
Structures/types that represent "objects" are often passed as the first
parameter to many functions, which serve as their "member functions". Sometimes,
these types are opaque and not related to other types in the system in a
user-visible way: they should have a _create and _destroy function. See @ref
time_state, @ref time_state_create, @ref time_state_destroy
In other cases, an interface will have function pointers defined as fields in
the interface structure. (A type implementing these may be opaque, but would
begin with a member of the interface/base type.) These interface function
pointers must still take in a self pointer as their first parameter, because
there is no implied this pointer in C. This would result in awkward calls with
repeated, error-prone mentions of the object pointer, such as this example
calling the @ref xrt_device::update_inputs interface:
xdev->update_inputs(xdev). These are typically wrapped by inline free
functions that make the call through the function pointer. Considering again the
@ref xrt_device example, the way you would call @ref xrt_device::update_inputs
is actually @ref xrt_device_update_inputs().
Destroy takes a pointer to a pointer, nulls it out#
Destroy free functions should take a pointer to a pointer, performing null checks before destruction, and setting null. They always succeed (void return): a failure when destroying an object has little meaning in most cases. For a sample, see @ref xrt_images_destroy. It would be used like this:
struct xrt_image_native_allocator *xina = /* created and initialized, or maybe NULL */;
/* ... */
xrt_images_destroy(&xina);
/* here, xina is NULL in all cases, and if it wasn't NULL before, it has been freed. */
Note that this pattern is used in most cases but not all in the codebase: we are gradually migrating those that don't fit this pattern. If you call a destroy function that does not take a pointer-to-a-pointer, make sure to do null checks before calling and set it to null after it returns.
Also note: when an interface includes a "destroy" function pointer, it takes the normal pointer to an object: The free function wrapper is the one that takes a pointer-to-a-pointer and handles the null checks. See for example @ref xrt_instance_destroy takes the pointer-to-a-pointer, while the interface method @ref xrt_instance::destroy takes the single pointer.