0.15 Retrospective

[Trashtalk217]

The release cycle is quickly nearing it's end. During this cycle, some things went well, while others went poorly. Leave your feedback here, so we can hopefully improve the process for upcoming release cycles!

[Trashtalk217]

Writing release notes / migration guides is currently a huge slog. Especially for @alice-i-cecile and @cart since they have to write the release notes for the feature authors that don't do it themselves (which is quite a lot). Maybe it's an idea to include the writing of release notes before merging PRs? Make it as standard as writing documentation and tests?

[alice-i-cecile]

Moving the release notes / migration guide content into the repo itself (as seperate files, like it currently is) would help make the tracking / coordination / overhead much easier. I've opened #16430 to discuss strategies for that.

I think that requiring release notes (draft, not final) and migration guides before merging is the way. The current process leaves Cart and I with a huge backlog of work to grind through while all other progress is stalled.

[aevyrie]

There have been a few features where I would've preferred if I was asked to write a draft release section as part of the PR.

• It's fresh in my mind
• I'm already motivated and working on it
• It helps point out flaws in the public API if I have to write a short snippet for the release, and I'm not happy with it

I don't think this has to be a huge amount of work, but I'd also understand if some contributors were not comfortable writing that sort of content.

[alice-i-cecile]

Yeah, and I'm fine to help pitch in for the contributors who aren't comfortable with that. I'd much rather write it fresh, without needing a whole separate PR anyways. My motivation here is 50% encouraging contributors to help more, and 50% reducing delay and process pain.

[bushrat011899]

I think it's also important to consider that the in-PR version of release notes and migration guide would be a first draft, with heaps of time to iterate in separate follow-up PRs. While it would be ideal for every PR to have a good set of release notes, even a basic "I added Foo so you can Bar now." is better than nothing.

[NthTensor]

I would much rather write the docs/migration-guide/release-notes while it's fresh in my mind. I also know I absolutely won't manage to do that consistently unless I'm required to. The current level of friction (having to open a second PR in a second repo) doesn't help.

[Trashtalk217]

It's really hard to exhaustively / automatically test rendering features across platforms. Are there any CI solutions that could be implemented. Require more extensive tests from feature authors?

[alice-i-cecile]

Pixel Eagle by @mockersf is the start of a cross-platform screenshot based tool to better test rendering.

I'd also like to build out better tools to test rendering features within a single binary: recompiling for each feature sucks. The testbed examples are a good start towards that.

Simply asking for more tests from feature authors won't work: we need a real framework to write good rendering test code in rather than just saying "figure it out" on an ad hoc basis.

Relatedly, folks are itching to drop webgl2 support, which will help with random breakage (it's very limited), but won't fully resolve things. Rendering will continue to be hard to test (manually and automatically) and prone to platform-specific breakage, and we need to design solutions with that in mind.

[clarfonthey]

I do wonder if it'd be possible to support some kind of headless rendering tests. Even if it can't check driver support, it should be able to at least write to a texture and verify it's output, to make sure that at least rendering isn't catastrophically broken.

[mockersf]

The issue in tests isn't headless, it's easy enough to simulate a window (just works on macOS and Windows, and a virtual frame buffer on Linux) or indeed just run headless.

The difficulty is having either a GPU usable in tests, or CPU rendering.

• Metal: macOS hosts in GitHub CI have a virtual GPU ("Apple Paravirtual device") available that just works
• DX12: Windows has a CPU renderer available ("Microsoft Basic Render Driver") but that has very small limits, probably due to the available memory/cpu in GitHub CI
• Vulkan: it's easy to install a software renderer ("llvmpipe") and it works

Our tests in CI already run a few rendering tests, save the result to a screenshot and check it didn't change.

Coverage is hard to extend because:

• Number of features tested: software rendering is slow (the same example that runs in 7 seconds on my local computer runs in 24 seconds on the macOS runner, 3 minutes on the Linux runner and 4.5 minutes on the Windows runner)
• Number of platforms tested: macOS / Metal and Linux / Vulkan are easy to test. wasm / WebGL2 and wasm / WebGPU are a bit harder but also possible. Windows / DX12 is broken, and mobile is more complicated (and also broken at the moment, at least in CI)

A solution for those difficulties could be to throw money at it, either by hosting dedicated runners somewhere with the hardware we need or by paying for larger runners on GitHub (it should work, but I haven't actually tested that)

The current tests executed are:

• Every 6 hours (if there are commits), every example is executed in CI on linux and macos, and the result can be seen there: https://thebevyflock.github.io/bevy-example-runner/index.html. Some examples are not deterministic and the screenshot will change at every execution. This needs to be manually checked
• Every push to a PR, the testbed examples are run in CI on macOS, and the screenshot are compared here: https://pixel-eagle.com/project/B04F67C0-C054-4A6F-92EC-F599FEC2FD1D. The testbed examples are made to be fully deterministic and can be relied on for automatic comparison
• Every attempt to merge a PR, the testbed examples are run in CI on Linux and Windows, and the screenshot compared (same link as above). Again, fully deterministic, but only on Linux, not on Windows

The last two points currently are still checked manually because they were added recently and I wanted to check their reliability before adding reporting to them. They've been working well for a few weeks, so it's probably time to take them to the next step, which I'll document soon in #15918

[Trashtalk217]

The retained rendering PR turned out to be a bit of a problem. Merged near the end of the cycle, it caused quite a lot of bugs. These bugs had to be ironed out on main which is extremely frustrating for everyone involved.

Since I was pretty heavily involved, I'll give my perspective first, including what I could've done better / how the process could be improved.

The Initial Problem

In 0.14, at the end of every frame, the render world purged all its entities. For complicated ECS reasons which I'm not going to explain (you can get a jist here), this blocks relations. Because I (and a lot of other ecs folks) care about that, I set out to try and fix it.

The Process

I made my first attempt at July 9. The exact way I did it was not really important, because I barely got anywhere before I managed to nerdsnipe @re0312, who created this PR on July 23. I want to take one sentence to say that @re0312 is a much better coder than I am and if it weren't for them I likely wouldn't have gotten anywhere.

This PR went through the review wringer, and while I quite often asked around in the Discord for reviews I was often told that people were busy (which is completely fair, large parts of this were during the summer vacation, so I understand that people had better things to do). People are busy, that's just how it is.

The PR became quiet after a bit, so on September 20 I decided to push it the final stretch. I figured it'd be a shame if all that work went stale. And I should add that this PR grew stale very quickly because it touches almost every single part of the rendering machinery. In any case, I tried to hurry the merging of this PR, in large part because I was done with it. This included a tactic where I tried to only get a few important reviews from SMEs / Alice, because more input would again slow the PR down. When it eventually did merge, I figured that there were probably still bugs in the code. Places I or re0132 forgot, although I hoped that everything would go off without a hitch. Or at least, I hoped that the bugs wouldn't be that bad.

Hope is not a great code review strategy.

In any case, I wrote up a follow-up issue and largely pulled my hands from it, because I was very busy and my attention was needed elsewhere.

What Went Wrong

There turned out to be a bunch of bugs. Some of them I had foreseen (entity leaks, wonky extraction), some of them I had theorized but deemed unlikely (removing only a synced component from an entity instead of removing the entity leaves said entity behind in the render world). By and large I don't think there was much that was technically insurmountable given the design, but it sure was annoying to track down every single bug.

I'm very sorry to and thankfull for the people that did.

On a personal level, I got inpatient, and tried to force through a PR that clearly wasn't ready. On a slightly deeper level, I didn't know how I could ever test the fullness of the change without just pushing it to main and having a thousand eyes create issues. This PR clearly needed a strategy for transitioning to a retained render world, but I don't know how that would've looked like. Please let me know, if you do.

I think that this PR has kind of flew under the radar (in the end that was kind of intentional). This change is important for the ECS folks, but largely involves rendering code. They Rendering people seemed less interested in this refactor. The fact that I took this on without having every played with the rendering internals before, should probably be noted somewhere. Also, because this was a refactor, I think a lot of people who looked at it, underestimated how much could break, right untill everything broke.

I'm out of thoughts for now, if anyone has questions, let me know.

[NthTensor]

For what it's worth, I don't think the issue with this PR was impatience, and I think it was really useful to have a champion to push it through. It was not a mistake to merge this. Yes, it would have been better to land it early in 0.16, but I think there was a good chance it would have gone stale if we had tried to hold it off, maybe burned out the authors, and that would have been worse.

Ideally we should have held off on merging it until after the release (maybe a month) and blocked all other non-essential rendering changes until after it was merged, to minimize churn.

IMO, that's the real issue. What do we do with PRs that are (a) through the review process and "ready-to-merge" but (b) can't/shouldn't be merged now and (c) will quickly go stale? Is there a way we can put them "on ice" and minimize merge conflicts for a while?

[Trashtalk217]

It's an interesting idea, but blocking most rendering PRs untill one specific refactor is finished sounds like it's gonna become an annoyance rather quickly. For exceptional cases, it could maybe be done. In practice this would mean that Alice would have to hold on pressing the button on any rendering PRs. Possible: yes, a bit weird: also yes.

I think the underlying reason why these sort of PRs got finished at the end of a release cycle is because deadlines are a thing that humans respond to. I wouldn't have pushed as much as I did if I knew there was still an entire release cycle left to go. The original plan was to get this merged at the start of 0.15, and back than I didn't (it also wasn't done then, but still).

[alice-i-cecile]

I'm really pleased about the success of working groups. Continuing the trend started with bevy_color, the text rework, bevy_picking, curves and the next generation scene working groups have all been really effective at delivering high quality work in a cohesive and collaborative way.

Having explicit blessing, a design, and a place to talk really helped!

[BD103]

I agree with this! I've had a great experience joining working groups and helping work on larger goals. Working groups are great to find contributors with similar interests and collaborate with them on issues in a semi-official manner.

[Occuros]

Is there currently a way to know which topics are considered for a working group?

It's easy to discover the ones which have already been formed, but not sure how to find potential future ones.

[NthTensor]

I kinda don't think we should have a list? Much prefer that they come about organically because people have been thinking about/struggling with something in the engine. Anyone can propose a group, and any proposed group will get consideration for scope & alignment by the leadership team.

Agree btw, I continue to be really happy with the wg process. I think we are still struggling to get people to write down & document their designs, but maybe that will come with changes to release notes.

[Trashtalk217]

I want to give a little bit of pushback with regards to the relations working group. As far as I can tell we do have a clear goal before us (both an rfc and a hackmd document), but the work has been somewhat slow. I mostly blame this on a lack of manpower. James has been busy at work, same goes for me and periwink.

ECS threads also have a rather unique ability to derail into hypotheticals like: 'could you build a relational database with relations?' or 'How should we handle thing that flecs hasn't even implemted yet and is still at least a year away?' I understand that I can't stop people from talking about things they're passionate about, but it does kind of annoy me sometimes. Still, the relations working group is no worse than the ecs-dev channel, so it's a wash on that point.

I'm kind of wondering if the 'self-contained task' this working group is doing is too large, but I'm not sure. From where I'm standing, landing an MVP for relations is possible in 0.16, but I said the same about 0.15, so I might just be delusional.

[brandon-reinhart]

On the outside, the working group threads were helpful as an early adopter. I had a place I could ask questions related to an implementation detail or bug or give feedback on the feature, etc.

[alice-i-cecile]

Upstreaming bevy_picking went really well overall, and I think it's a model to emulate. This is a really effective way to rapidly expand our capabilities, and it helps share the maintenance burden for vital functionality.

I think in the future we should consider polishing / improving further in the upstream repo (docs and examples especially), and I underestimated the difficulty of the work: it ended up taking fairly experienced engineers to do the integration work effectively.

[alice-i-cecile]

@NthTensor I'd be interested in your thoughts on this briefly, as you ended up largely leading this effort. @Jondolf and @aevyrie as well, since you were also major contributors / the upstreamed crate's author.

[aevyrie]

I thought it went really well. I was away for personal reasons for the majority of the process, but it went smoothly from my perspective.

I get the sense this area of bevy could use a SME; there is a lot of surprising complexity, and even after 0.15 there is still a good amount before we have "finished". It's a bit difficult to feel like the decisions you make and the design you are settling on is going to stick without a dedicated SME present. My biggest worry is that without someone with depth of knowledge, context, and a clear vision, this code will begin to rot from well-meaning but poorly thought out changes.

[NthTensor]

Agree with @aevyrie here about an SME, although I think Alice serves this roll pretty well.

I'd like to see us upstream more stuff. It's a challenge, but a different kind of challenge than greenfield work, which mostly comes from solving problems in the ecosystem crate by modifying the ecs or the other core crates (the for bevy_picking this was adding bubbling to observers). IMO it's something you're usually going to need engine-devs for (which was the issue with the student group, not their fault).

For future up streams, this is probably how I'd work it:

1. Design and implement necessary engine features, and in parallel add polish to the ecosystem crate.
2. Copy all the important files from ecosystem crate verbatim into the engine repo (leave stuff out of the workspace/example tree).
3. Mock the up-streamed crate to get it compiling (even if it's broken) and write specs/tests.
4. Integrate new ecs features and fix broken stuff until it meets the specs/tests.
5. Iterate on the up-streamed api with feedback from early adopters (this was crucial for getting bevy_picking ready for release).

I probably wouldn't try to plan more than one of these stages at a time. It's better to bite off manageable chunks.

[alice-i-cecile]

The new workflow for writing migration guides / PRs with each piece of content in its own file is great (other than being done later + in a different repo). Almost no merge conflicts, super easy to re-organize, and the new PR merging strategy is fantastic. Really pleased with how that design and tooling work has paid off.