-
Notifications
You must be signed in to change notification settings - Fork 106
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
Architecture diagram and description of the project's folders #3053
Conversation
Codecov Report
@@ Coverage Diff @@
## main #3053 +/- ##
==========================================
- Coverage 77.69% 77.69% -0.01%
==========================================
Files 1056 1056
Lines 73409 73449 +40
==========================================
+ Hits 57037 57068 +31
- Misses 16372 16381 +9 see 6 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
doc/sphinx/architecture.md
Outdated
# Overview | ||
This document describes the high-level architecture of *Mir*, including a diagram and a brief introduction of the directory structure. | ||
|
||
# Concepts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think at this level of abstraction there are more concepts. Basically, what you've called "Mir" breaks into several things:
- mircore
- mirplatform
- mirserver
I think "Mir" ought to be the collective. And, maybe the highest level architecture is enumerating the shared libraries and which are "for consumption" and which are "internal"?
doc/sphinx/architecture.md
Outdated
# Concepts | ||
## MirAL | ||
- The "Mir Abstraction Layer" (aka `mirAL` or `miral`) is a layer that makes the core Mir implementation easier for compositor-implementers to work with. | ||
- If one wishes to implement a compositor themselves, they will spend most of their time interacting with the `mirAL` API and only really touch the `mir` API when they require core concepts like a `mir::geometry::Rectangle`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rectangle
is from mircore, and that's very different from the mirserver APIs (that compositor writers should never touch.
doc/sphinx/architecture.md
Outdated
## `/src/platform` (no "s") | ||
- Graphics and input code that is shared between platforms |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment: In principle, we aim to make this a public interface for third party "platforms". As Chris has been reworking these APIs, they are currently private
Co-authored-by: Alan Griffiths <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is too much "flattening" here. There is meaningful hierarchy:
-
mirserver is the "god" library.
-
miral (and miroil) are the support for building compositors with Mir.
-
Platforms are adaptors that allow mirserver to work across different graphics stacks
-
There are support libraries that provide the underpinnings for the above: mircore, mircommon, mircookie, mirplatform & mirwayland.
If you try and present parts of parts of mirserver as top-level concepts you end up with too many things to explain at the same time
doc/sphinx/architecture.md
Outdated
## `/examples` | ||
- A collection of demo *Mir* compositors that are used as examples of how one might build a true compositor. Examples include: | ||
- `miral-app` | ||
- `miral-shel` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- `miral-shel` | |
- `miral-shell` |
doc/sphinx/architecture.md
Outdated
- `mir_demo_server` | ||
|
||
## `/tests` | ||
- Unit tests for the entire project. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not just unit tests
doc/sphinx/architecture.md
Outdated
└── mirserver | ||
├── mircompositor | ||
├── mirinput | ||
├── mirfrontend | ||
├── mirfrontend-wayland | ||
├── mirfrontend-xwayland | ||
├── mirshell | ||
├── mirshelldecoration | ||
├── mirscene | ||
├── mirgraphics | ||
├── mirreport | ||
├── mirlttng | ||
├── mirlogging | ||
├── mirnullreport | ||
├── mirconsole | ||
├── mirgl | ||
└── mirrenderergl |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't expand mirserver here. I think that introduces too many concepts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I expanded it because each concept under mirserver (e.g. compositor, scene, shell, etc.) seem to warrant there own description for someone jumping into the project. Do you have any recommendation on how I can still describe those namespaces?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, they need explaining but we're well past the number of concepts that should be introduced together. (Even Agatha Christie stops at a dozen suspects and she was aiming to confuse!)
First introduce the high level concepts. Then, break down each one in turn: (e.g. "These are the platforms... this one is for X11...", "This is the mirserver... it has these components...").
That gives the reader some structure to organise their understanding.
doc/sphinx/architecture.md
Outdated
## mircookie | ||
- Provides event timestamps for inputs events that are difficult to spoof. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@RAOF I suspect that mircookie isn't actually useful now we're no longer shipping Mir's input events to clients. WDYT?
doc/sphinx/architecture.md
Outdated
- `src/server` | ||
- `src/include/server` | ||
|
||
## mircompositor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like this naming (probably comes from trying to treat it at the same level of abstraction as libraries).
It is "just" a namespace within mirserver.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It could be mirserver::mircompositor
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's worse! How is a reader to associate something in the code (mir::compositor::Foo
, say) with mirserver::mircompositor
?
doc/sphinx/architecture.md
Outdated
## mirfrontend | ||
- TODO: How do I describe this one? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
src/server/frontend
is vestigial - we should probably merge it with src/server/frontend_wayland
doc/sphinx/architecture.md
Outdated
- `src/server/shell` | ||
|
||
## mirshelldecoration | ||
- Manages the decoration of *Mir*'s windows |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Manages the decoration of *Mir*'s windows | |
- Manages *Mir*'s server-side decoration of windows. Currently only used for X11 clients. |
doc/sphinx/architecture.md
Outdated
- Directories: | ||
- `src/server/report` | ||
|
||
## mirlttng |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this naming even less. lttng
is a directory/namespace within report
- it shouldn't be named at the same level as mirwayland
. Similarly, logging
, null
.
doc/sphinx/architecture.md
Outdated
## graphics-gbm-kms | ||
- GBM/KMS display platform for Mir | ||
- This is the main display platform that one would expect to use | ||
- Directories: | ||
- `src/platforms/gbm-kms` | ||
|
||
## graphics-gbm-kms | ||
- EglStreams/KMS display platform for Mir | ||
- This is an Nvidia-specific platform | ||
- Directories: | ||
- `src/platforms/eglstreams-kms` | ||
|
||
## graphics-x11 | ||
- An X11-based display/input platform | ||
- Primary platform used for testing | ||
- Directories: | ||
- `src/platforms/x11` | ||
|
||
## graphics-wayland | ||
- A Wayland-based display/input platform | ||
- Directories: | ||
- `src/platforms/wayland` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suddenly we're not prefixing everything with "mir"! 😀
I think it works better to explain what a platform is, and then list examples.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe I only did this because the final .so
names aren't prefaced with mir here 😂 But yes, agreed
bd35cca
to
8a2eb5e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The architecture.md reads much better now. Please press on updating the rest to match
Should there be a link somewhere? I don't see it. |
Co-authored-by: Alan Griffiths <[email protected]>
7fb4fe0
to
7690c4c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Content works for me.
A couple of tweaks though:
- Could we incorporate the diagrams into the "Architecture" page instead of a separate page?
- The "Closeup" would look tidier with the "Shell" tree and the "Compositor" tree swapped (or the "Scene" and "miral" trees). Is there a way to tweak the layout?
The order that things appear in the document is the order that they will appear in the diagram (loosely). Should be possible! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WFM, but as I've had a lot of input I'll @Saviq do a final review
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, good start! :)
How to test
What's new?