Skip to content
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

Crate, Example, Documentation and Test Organisation #170

Closed
XAMPPRocky opened this issue Oct 28, 2020 · 5 comments
Closed

Crate, Example, Documentation and Test Organisation #170

XAMPPRocky opened this issue Oct 28, 2020 · 5 comments
Labels
c: meta Issues specific to the GitHub repository. README, CI, etc. t: enhancement A new feature or improvement to an existing one.

Comments

@XAMPPRocky
Copy link
Member

XAMPPRocky commented Oct 28, 2020

The amount of crates, examples documentation, and tests are expanding pretty rapidly, and some of the layout is already is mildly confusing. I think we should figure out some sort of structure for each.

Crates

Currently we have three dev crates (five with #117) which are at the top level, the top level is already becoming pretty crowded with documentation, and configuration files. I think we should have a crates/ folder that houses the main development crates.

Documentation

Currently we have two directories (assets and rfcs) which are at the top level, but I think since these will be less frequently used I would propose that we should move assets into docs and move rfcs into docs/src and make it part of the rust GPU dev guide. This has the advantage of making the RFCs viewable in the rust gpu guide directly.

Tests

The integration tests for rustc_codegen_spirv are currently located inside spirv-builder in spirv-builder/src/test. This isn't where I would expect these kinds of tests to be. I propose that we create a new separate crate for holding integration/end-to-end tests (e.g. test_suite, codegen_test, etc).

Examples

We have four different example crates at the moment, all on the same level in examples, however it's not really clear which examples are related, or use each other. I would like to propose adding another level to examples that covers the basic domain of example (e.g. a wgpu directory for wgpu examples), and remove the example prefix from the name (e.g. example-runnerrunner).


I've included a tree output of what this proposed directory structure would look like if all of these changes implemented below.

.
├── CODE_OF_CONDUCT.md
├── CONTRIBUTING.md
├── Cargo.lock
├── Cargo.toml
├── LICENSE-APACHE
├── LICENSE-MIT
├── README.md
├── crates
│   ├── rustc_codegen_spirv
│   │   ├── Cargo.toml
│   │   └── src
│   ├── spirv-builder
│   │   ├── Cargo.toml
│   │   └── src
│   ├── spirv-std
│   │   ├── Cargo.toml
│   │   └── src
│   └── test-suite
│       ├── basic.rs
│       └── mod.rs
├── deny.toml
├── docs
│   ├── assets
│   │   └── sky.jpg
│   ├── book.toml
│   └── src
│       ├── SUMMARY.md
│       ├── attributes.md
│       ├── introduction.md
│       └── rfcs
├── examples
│   ├── basic
│   │   ├── cpu-runner
│   │   ├── runner
│   │   └── shader
│   └── wgpu
│       ├── runner
│       └── shader
├── rust-toolchain
├── setup.bat
└── setup.sh

20 directories, 21 files
@XAMPPRocky XAMPPRocky added t: enhancement A new feature or improvement to an existing one. c: meta Issues specific to the GitHub repository. README, CI, etc. labels Oct 28, 2020
@fu5ha
Copy link
Member

fu5ha commented Oct 28, 2020

I support this though I would restructure the example code differently:

  • Top level examples
    • runners
      • wgpu
      • ash
      • cpu
    • shaders
      • hello-world
      • sky

@XAMPPRocky
Copy link
Member Author

XAMPPRocky commented Oct 28, 2020

@termhn That seems good to me, though it's a bit non-obvious that whether cpu depends on hello-world or sky for example. Though really the runners should be able to run any of the shaders anyway right?

@fu5ha
Copy link
Member

fu5ha commented Oct 28, 2020

Yeah exactly. That's basically the goal of #109 (make all of the examples be able to run all the shaders with same output)

@repi
Copy link
Contributor

repi commented Oct 28, 2020

I like @termhn proposed structure, with the added caveat that I think most shaders will not be separate crates, but be a large crate with many of them in, or rather a set of collections of them. so instead of sky it likely will be most (insert good name) :)

Snapshot of Ark shaders likely will be its own crate for ease of use and keep separate and have the old GLSL shaders in also for reference.

Another note about shaders, we'll have many different types of shaders, not just "examples", will also be sets of shaders from other sources (such as Ark) that we experiment with, are porting and maintaining as part of the dev of this, and there will be shaders as in loose separate files that are only there to test specific functionality. Some of them will be embedded in actual tests but larger ones likely will be part of separate standalone crates in the repo. So may want to have shaders/ in the root.

The benefit of having a separate shaders/ directory in general is that we can also have a .cargo/config in it that sets the default target in compilation to spirv-unknown-unknown and sets the RUSTFLAGS to point to the built rustc backend. Then it is easier to do cargo build and clippy and run cargo watch on directly without lots of command-line parameters.

The CPU direct evaluation example will continue to directly link just specific set of shaders and will ever only be able to run very few of them.

I think we should have a crates/ folder that houses the main development crates.

Was leaning towards that as well. Would be the main native binary crates + shader library crates that we maintain, but not our example/test shaders.

Currently we have two directories (assets and rfcs) which are at the top level, but I think since these will be less frequently used I would propose that we should move assets into docs and move rfcs into docs/src

Yup makes sense, assets is really just assets for the documentation/READMEs and shouldn't be other type of assets

@arirawr
Copy link
Contributor

arirawr commented Oct 29, 2020

Seems good to me, at least for the docs and rfc structuring!

fu5ha added a commit that referenced this issue Oct 30, 2020
* get rid of vertex buffers in example

* get rid of unused attributes in example shader

* use negative viewport height to match wgpu

* remove depth buffer

* use SRGB surface

* improve tonemapping

remove 'gamma correction' in favor of hardware srgb support

* make clippy happy

* move tonemapping out of sky(), rename gl_pos to builtin_pos

* rename shaders and use sky shader in wgpu example runner

* apply srgb OETF and invert clip space for cpu example runner

* restructure example directory structure according to #170

* update winit in wgpu example runner

* fix deny.toml example crate refs

* fix ci example name on maOS

* example-shader -> sky-shader in docs

* update sky shader image

* re-enable clippy and make it happy
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: meta Issues specific to the GitHub repository. README, CI, etc. t: enhancement A new feature or improvement to an existing one.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants