mirror of
https://github.com/hyprwm/wlroots-hyprland.git
synced 2024-12-23 10:29:49 +01:00
5cde35923c
Some globals are static and it doesn't make sense to destroy them before the wl_display. For instance, wl_compositor should be created before the display is started and shouldn't be destroyed. For these globals, we can simplify the code by removing the destructor and stop keeping track of wl_resources (these will be destroyed with the wl_display by libwayland).
355 lines
12 KiB
Markdown
355 lines
12 KiB
Markdown
# Contributing to wlroots
|
|
|
|
Contributing just involves sending a pull request. You will probably be more
|
|
successful with your contribution if you visit
|
|
[#sway-devel](https://webchat.freenode.net/?channels=sway-devel) on
|
|
irc.freenode.net upfront and discuss your plans.
|
|
|
|
Note: rules are made to be broken. Adjust or ignore any/all of these as you see
|
|
fit, but be prepared to justify it to your peers.
|
|
|
|
## Pull Requests
|
|
|
|
If you already have your own pull request habits, feel free to use them. If you
|
|
don't, however, allow me to make a suggestion: feature branches pulled from
|
|
upstream. Try this:
|
|
|
|
1. Fork wlroots
|
|
2. `git clone https://github.com/username/wlroots && cd wlroots`
|
|
3. `git remote add upstream https://github.com/swaywm/wlroots`
|
|
|
|
You only need to do this once. You're never going to use your fork's master
|
|
branch. Instead, when you start working on a feature, do this:
|
|
|
|
1. `git fetch upstream`
|
|
2. `git checkout -b add-so-and-so-feature upstream/master`
|
|
3. Add and commit your changes
|
|
4. `git push -u origin add-so-and-so-feature`
|
|
5. Make a pull request from your feature branch
|
|
|
|
When you submit your pull request, your commit log should do most of the talking
|
|
when it comes to describing your changes and their motivation. In addition to
|
|
this, your pull request's comments will ideally include a test plan that the
|
|
reviewers can use to (1) demonstrate the problem on master, if applicable and
|
|
(2) verify that the problem no longer exists with your changes applied (or that
|
|
your new features work correctly). Document all of the edge cases you're aware
|
|
of so we can adequately test them - then verify the test plan yourself before
|
|
submitting.
|
|
|
|
## Commit Messages
|
|
|
|
Please strive to write good commit messages. Here's some guidelines to follow:
|
|
|
|
The first line should be limited to 50 characters and should be a sentence that
|
|
completes the thought [When applied, this commit will...] *"Implement
|
|
cmd_move"* or *"Fix #742"* or *"Improve performance of arrange_windows on ARM"*
|
|
or similar.
|
|
|
|
The subsequent lines should be separated from the subject line by a single
|
|
blank line, and include optional details. In this you can give justification
|
|
for the change, [reference Github
|
|
issues](https://help.github.com/articles/closing-issues-via-commit-messages/),
|
|
or explain some of the subtler details of your patch. This is important because
|
|
when someone finds a line of code they don't understand later, they can use the
|
|
`git blame` command to find out what the author was thinking when they wrote
|
|
it. It's also easier to review your pull requests if they're separated into
|
|
logical commits that have good commit messages and justify themselves in the
|
|
extended commit description.
|
|
|
|
As a good rule of thumb, anything you might put into the pull request
|
|
description on Github is probably fair game for going into the extended commit
|
|
message as well.
|
|
|
|
See [here](https://chris.beams.io/posts/git-commit/) for more details.
|
|
|
|
## Code Review
|
|
|
|
When your changes are submitted for review, one or more core committers will
|
|
look over them. Smaller changes might be merged with little fanfare, but larger
|
|
changes will typically see review from several people. Be prepared to receive
|
|
some feedback - you may be asked to make changes to your work. Our code review
|
|
process is:
|
|
|
|
1. **Triage** the pull request. Do the commit messages make sense? Is a test
|
|
plan necessary and/or present? Add anyone as reviewers that you think should
|
|
be there (using the relevant GitHub feature, if you have the permissions, or
|
|
with an @mention if necessary).
|
|
2. **Review** the code. Look for code style violations, naming convention
|
|
violations, buffer overflows, memory leaks, logic errors, non-portable code
|
|
(including GNU-isms), etc. For significant changes to the public API, loop in
|
|
a couple more people for discussion.
|
|
3. **Execute** the test plan, if present.
|
|
4. **Merge** the pull request when all reviewers approve.
|
|
5. **File** follow-up tickets if appropriate.
|
|
|
|
## Style Reference
|
|
|
|
wlroots is written in C with a style similar to the [kernel
|
|
style](https://www.kernel.org/doc/Documentation/process/coding-style.rst), but
|
|
with a few notable differences.
|
|
|
|
Try to keep your code conforming to C11 and POSIX as much as possible, and do
|
|
not use GNU extensions.
|
|
|
|
### Brackets
|
|
|
|
Brackets always go on the same line, including in functions.
|
|
Always include brackets for if/while/for, even if it's a single statement.
|
|
```c
|
|
void function(void) {
|
|
if (condition1) {
|
|
do_thing1();
|
|
}
|
|
|
|
if (condition2) {
|
|
do_thing2();
|
|
} else {
|
|
do_thing3();
|
|
}
|
|
}
|
|
```
|
|
|
|
### Indentation
|
|
|
|
Indentations are a single tab.
|
|
|
|
For long lines that need to be broken, the continuation line should be indented
|
|
with an additional tab.
|
|
If the line being broken is opening a new block (functions, if, while, etc.),
|
|
the continuation line should be indented with two tabs, so they can't be
|
|
misread as being part of the block.
|
|
```c
|
|
really_long_function(argument1, argument2, ...,
|
|
argument3, argument4);
|
|
|
|
if (condition1 && condition2 && ...
|
|
condition3 && condition4) {
|
|
do_thing();
|
|
}
|
|
```
|
|
|
|
Try to break the line in the place which you think is the most appropriate.
|
|
|
|
|
|
### Line Length
|
|
|
|
Try to keep your lines under 80 columns, but you can go up to 100 if it
|
|
improves readability. Don't break lines indiscriminately, try to find nice
|
|
breaking points so your code is easy to read.
|
|
|
|
### Names
|
|
|
|
Global function and type names should be prefixed with `wlr_submodule_` (e.g.
|
|
`struct wlr_output`, `wlr_output_set_cursor`). For static functions and
|
|
types local to a file, the names chosen aren't as important. Local function
|
|
names shouldn't have a `wlr_` prefix.
|
|
|
|
For include guards, use the header's filename relative to include. Uppercase
|
|
all of the characters, and replace any invalid characters with an underscore.
|
|
|
|
### Construction/Destruction Functions
|
|
|
|
For functions that are responsible for constructing and destructing an object,
|
|
they should be written as a pair of one of two forms:
|
|
* `init`/`finish`: These initialize/deinitialize a type, but are **NOT**
|
|
responsible for allocating it. They should accept a pointer to some
|
|
pre-allocated memory (e.g. a member of a struct).
|
|
* `create`/`destroy`: These also initialize/deinitialize, but will return a
|
|
pointer to a `malloc`ed chunk of memory, and will `free` it in `destroy`.
|
|
|
|
A destruction function should always be able to accept a NULL pointer or a
|
|
zeroed value and exit cleanly; this simplifies error handling a lot.
|
|
|
|
### Error Codes
|
|
|
|
For functions not returning a value, they should return a (stdbool.h) bool to
|
|
indicated if they succeeded or not.
|
|
|
|
### Macros
|
|
|
|
Try to keep the use of macros to a minimum, especially if a function can do the
|
|
job. If you do need to use them, try to keep them close to where they're being
|
|
used and `#undef` them after.
|
|
|
|
### Example
|
|
|
|
```c
|
|
struct wlr_backend *wlr_backend_autocreate(struct wl_display *display) {
|
|
struct wlr_backend *backend;
|
|
if (getenv("WAYLAND_DISPLAY") || getenv("_WAYLAND_DISPLAY")) {
|
|
backend = attempt_wl_backend(display);
|
|
if (backend) {
|
|
return backend;
|
|
}
|
|
}
|
|
|
|
const char *x11_display = getenv("DISPLAY");
|
|
if (x11_display) {
|
|
return wlr_x11_backend_create(display, x11_display);
|
|
}
|
|
|
|
// Attempt DRM+libinput
|
|
|
|
struct wlr_session *session = wlr_session_create(display);
|
|
if (!session) {
|
|
wlr_log(WLR_ERROR, "Failed to start a DRM session");
|
|
return NULL;
|
|
}
|
|
|
|
int gpu = wlr_session_find_gpu(session);
|
|
if (gpu == -1) {
|
|
wlr_log(WLR_ERROR, "Failed to open DRM device");
|
|
goto error_session;
|
|
}
|
|
|
|
backend = wlr_multi_backend_create(session);
|
|
if (!backend) {
|
|
goto error_gpu;
|
|
}
|
|
|
|
struct wlr_backend *libinput = wlr_libinput_backend_create(display, session);
|
|
if (!libinput) {
|
|
goto error_multi;
|
|
}
|
|
|
|
struct wlr_backend *drm = wlr_drm_backend_create(display, session, gpu);
|
|
if (!drm) {
|
|
goto error_libinput;
|
|
}
|
|
|
|
wlr_multi_backend_add(backend, libinput);
|
|
wlr_multi_backend_add(backend, drm);
|
|
return backend;
|
|
|
|
error_libinput:
|
|
wlr_backend_destroy(libinput);
|
|
error_multi:
|
|
wlr_backend_destroy(backend);
|
|
error_gpu:
|
|
wlr_session_close_file(session, gpu);
|
|
error_session:
|
|
wlr_session_destroy(session);
|
|
return NULL;
|
|
}
|
|
```
|
|
|
|
## Wayland protocol implementation
|
|
|
|
Each protocol generally lives in a file with the same name, usually containing
|
|
at least one struct for each interface in the protocol. For instance,
|
|
`xdg_shell` lives in `types/wlr_xdg_shell.h` and has a `wlr_xdg_surface` struct.
|
|
|
|
### Globals
|
|
|
|
Global interfaces generally have public constructors and destructors. Their
|
|
struct has a field holding the `wl_global` itself, a destroy signal and a
|
|
`wl_display` destroy listener. Example:
|
|
|
|
```c
|
|
struct wlr_compositor {
|
|
struct wl_global *global;
|
|
…
|
|
|
|
struct wl_listener display_destroy;
|
|
|
|
struct {
|
|
struct wl_signal new_surface;
|
|
struct wl_signal destroy;
|
|
} events;
|
|
};
|
|
```
|
|
|
|
When the destructor is called, it should emit the destroy signal, remove the
|
|
display destroy listener, destroy the `wl_global` and then destroy the struct.
|
|
The destructor can assume all clients and resources have been already
|
|
destroyed.
|
|
|
|
### Resources
|
|
|
|
Resources are the representation of Wayland objects on the compositor side. They
|
|
generally have an associated struct, called the _object struct_, stored in their
|
|
`user_data` field.
|
|
|
|
Object structs can be retrieved from resources via `wl_resource_get_data`. To
|
|
prevent bad casts, a safe helper function checking the type of the resource is
|
|
used:
|
|
|
|
```c
|
|
static const struct wl_surface_interface surface_impl;
|
|
|
|
struct wlr_surface *wlr_surface_from_resource(struct wl_resource *resource) {
|
|
assert(wl_resource_instance_of(resource, &wl_surface_interface,
|
|
&surface_impl));
|
|
return wl_resource_get_user_data(resource);
|
|
}
|
|
```
|
|
|
|
### Destroying resources
|
|
|
|
Object structs should only be destroyed when their resource is destroyed, ie.
|
|
in the resource destroy handler (set with `wl_resource_set_implementation`).
|
|
Destructor requests should only call `wl_resource_destroy`.
|
|
|
|
The compositor should not destroy resources on its own.
|
|
|
|
### Inert resources
|
|
|
|
Some resources can become inert in situations described in the protocol or when
|
|
the compositor decides to get rid of them. All requests made to inert resources
|
|
should be ignored, except the destructor. This is achieved by:
|
|
|
|
1. When the resource becomes inert: destroy the object struct and call
|
|
`wl_resource_set_user_data(resource, NULL)`. Do not destroy the resource.
|
|
2. For each request made to a resource that can be inert: add a NULL check to
|
|
ignore the request if the resource is inert.
|
|
3. When the client calls the destructor request on the resource: call
|
|
`wl_resource_destroy(resource)` as usual.
|
|
4. When the resource is destroyed, if the resource isn't inert, destroy the
|
|
object struct.
|
|
|
|
Example:
|
|
|
|
```c
|
|
// Handles the destroy request
|
|
static void subsurface_handle_destroy(struct wl_client *client,
|
|
struct wl_resource *resource) {
|
|
wl_resource_destroy(resource);
|
|
}
|
|
|
|
// Handles a regular request
|
|
static void subsurface_set_position(struct wl_client *client,
|
|
struct wl_resource *resource, int32_t x, int32_t y) {
|
|
struct wlr_subsurface *subsurface = subsurface_from_resource(resource);
|
|
if (subsurface == NULL) {
|
|
return;
|
|
}
|
|
|
|
…
|
|
}
|
|
|
|
// Destroys the wlr_subsurface struct
|
|
static void subsurface_destroy(struct wlr_subsurface *subsurface) {
|
|
if (subsurface == NULL) {
|
|
return;
|
|
}
|
|
|
|
…
|
|
|
|
wl_resource_set_user_data(subsurface->resource, NULL);
|
|
free(subsurface);
|
|
}
|
|
|
|
// Resource destroy listener
|
|
static void subsurface_handle_resource_destroy(struct wl_resource *resource) {
|
|
struct wlr_subsurface *subsurface = subsurface_from_resource(resource);
|
|
subsurface_destroy(subsurface);
|
|
}
|
|
|
|
// Makes the resource inert
|
|
static void subsurface_handle_surface_destroy(struct wl_listener *listener,
|
|
void *data) {
|
|
struct wlr_subsurface *subsurface =
|
|
wl_container_of(listener, subsurface, surface_destroy);
|
|
subsurface_destroy(subsurface);
|
|
}
|
|
```
|