Today's (full-time, paid!) #opesource work for @bevy is tackling work in the release milestone. We're nearly due for our next train release (look, the train is North American: you can't expect reliability) so I'm going to be going through the milestone, culling, reviewing and fixing issues.
At this stage, I'm only going to leave critical bugs and nearly complete high-impact work in the milestone. Other PRs can and will be merged if they're in there before the release candidate ships.
Some of these are double-counted, as there's both an issue and a linked PR. Let's go through them one at a time together, and see what each of them needs. It helps me think, makes the work a bit less lonely, and might even be fun (or motivating) for you to read :)
Rest of this thread will be set to unlisted, so I spam the poor gamedev.place timeline less.
A bit of cleanup from the color work that I did: one of the fields (emissive, which makes objects glow) should be stored as a nice simple LinearRgba rather than the polymorphic Color type.
Easy change, but a bit of cleanup needed from the author. Labelling as S-Waiting-On-Author and leaving for now: next week I will be adopting or cutting these, so if you want your work in the release get it done this weekend <3
Winit 0.30 upgrade. Generally really valuable due to bug fixes, but particularly needed this time for Android fixes.
These are always gnarly: lots of complex logic, tons of platform-specific testing. This one has generally gone shockingly smooth, but there's an outstanding challenge with how we initialize render plugins that I think is driven by limitations of WebGL. If this is something you have ideas on, please chime in!
As part of a soundness fix, we ended up restricting the access requested by WorldQuery::get_state. This was great, but ended up with weird inconsistencies with its init_state sibling. It was non-trivial to resolve, so we punted it to future work on the condition that this is fixed up in the milestone.
The original author claimed this: we'll see if they get the time to do it this weekend. Otherwise, no worries, I'd appreciate an excuse to code.
Nannou, a creative coding tool (used for things like art installations), is moving to Bevy! They've been wonderful to work with, and one of the critical limitations they've been running into is that they need waay more render layers than the 32 we provide. One of the authors made a nice PR to do that: which is now a bit bogged down in perf discussions. https://discord.com/channels/691052431525675048/743663924229963868/1240676270380027945
Rust recently shipped support for on_unimplented diagnostic hints to let crates define their own error messages for traits that, as the name implies, aren't implemented.
This is a pretty basic implementation that I think is still an improvement. I'd like to see much fancier things (which args weren't SystemParam?) but this is a good first step. The key improvement (hiding the stupid tuple pyramid suggestions) has not shipped. Please review and bikeshed!
Apparently we're incorrectly handling emissive (glowy) materials, by multiplying it with the camera's exposure value. This ends up breaking imports, leading to a mysterious discrepancy between how Blender and Bevy render models with this property.
Great link to the ever-useful filament docs, I agree with this fix. Approved, but needs a second review. Easy enough and useful enough to stay in the milestone IMO.
In an earlier PR, we merged a change to how we compute normals for indexed meshes. It was incorrect and confusing, so when this was pointed out I opened an issue to revert it before the release.
The author did better, and added a proper fix with the preliminary ability to generate smooth normals. Hype! Looks good now, approving, cleaning up the PR title and merging. While cool, doesn't deserve a release note section (attention is scarce) so removing the label.
Ah, here's the "Android is broken issue". Great writeup. The winit upgrade is required to fix this, which I've already discussed. Linking this issue there in the PR description so it doesn't stay alive as a phantom issue and moving on.
I love platform fragmentation 🙃 Here, low-end Android device are crashing following a rendering change. Important to fix in some form, so it stays. Thank you thank you Francois for being so dilligent about this work. There's fresh contention in the comments about the best way to detect the missing feature, so this gets kicked back to the author to respond. Shippable as is though IMO, so I'm not super worried.
A nice code quality / cleanup for how we handle the window handles (which track access to the OS-level resources). Seems fine, but it's crashing on Android and not vital. Cutting from the milestone.
A change to how commands are applied when working with the shiny new hooks and observers. WrongShoe, who's both diligent and very skilled with ECS, found some non-trivial concerns that I think should block a merge despite approvals (including my own!). Labelling as Waiting-for-Author: I'll adopt this next week if needed.
If we lose access to the context of the window (and thus cannot render to it) on web, we should pause rendering rather than crashing. Very sensible, but I don't have the expertise to review this. It doesn't meet the criteria to be kept in the release milestone, but let me ping the authors of the issues and see if they have the skill and interest to review.
A bug with rendering cameras on top of each other. I initially thought this was related to the 2D clear color bug we just fixed, but it predates the introduction of that. This has been bumped from 0.13.1 to 0.13.2 to 0.13.3 to 0.14. No fix is forthcoming and there's no outcry to fix this immediately, so this gets cut.
Alright, next up is dynamic-plugin-yeet. This is an neat but old and completely unloved feature to support Rust hot reloading. It's wildly unsafe, not particularly well-documented, hasn't been improved in years and seems to have active UB.
As other experiments (like dextrous_developer) have been created, I've increasingly been of an opinion that this is an attractive nuisance. Others agree! Cart's the last holdout, so I'll bring it up next time we chat :)
A neat little enhancement to asset loading for working with async asset loading. Seems handy and well-designed, but I'm not confident on how it fits in to the bigger picture. Requesting an example and voicing support. Regardless, definitely doesn't meet milestone criteria. Cutting.
A sizable performance improvement to meshlets. Needs review from folks with more expertise than me. Seems fine to include for now: it's cleanup for one of our major features of the release, but this will be cut soon <3
Well, I've added more labels at least! There's some joy in that, and I have been finding the new difficulty and status labels very useful to quickly evaluate and communicate about work.
We've merged a PR to make asset readers seekable, letting you skim through the file.
Cart had a justified concern that this won't play nice with things like HTTP requests for assets. I spun this out into a follow-up issue to make sure it didn't get lost. While it would be good to improve this, I don't think it's vital and there's no easy win / clear design here. The merged PR has real benefits so reverting isn't the clear winner. Well, cutting from 0.14...
A bug fix for cube array textures on iOS. Uncontroversially good, but nowhere near high enough impact for the milestone. Niche feature x niche platform means that we shouldn't block other more valuable work on it. Removing, but I'll give it some nice labels while I'm here.
I would really, really like more graceful failures for rendering in general: the platform fragmentation sucks. If you have ideas for how to improve that please suggest or make changes!
Exposes better information about what naga features in Bevy. naga is two levels up from bevy_render: it's the shader translation library used by wgpu, which is the mid-level rendering abstraction crate that we build on.
Seems like a fine PR, but not critical to ship now and may be obsoleted by the next wgpu upgrade anyways. I've asked Jasmine what to do with it, but either way it's not a good milestone candidate right now.
We've refactored how animations work in 0.13 to a shiny new animation graph approach. Super useful and welcome, but there's evidently a regression with how blending from an animation to itself is handled. This is a non-trivial new bug with no workaround: bah!
mweatherley has a great analysis in the comments: it looks like this is non-trivial to properly fix. Hmm. I'll ping the SME-Animation: this isn't something I can bang out in a day next week.
We merged a nice little debug overlay for UI layouts, much like the inspector that you get in web browsers. Super useful, but it's missing an example.
I'm a bit loathe to call attention to the mess that is dev tools right now, but I think that this is an essential bit of the release. Users will want to use this, and we should teach them! Easier testing is also key.
Great first issue if you're interested! Otherwise I'll tackle this next week.
Observers! This is on the critical path towards relations, and one of the star ECS features of the coming release.
I'm broadly confident that these should exist, but haven't devoted an afternoon to a thorough review yet. That's mostly on me, although we'll need another SME-ECS (or Cart, who's effectively an SME-Everything). It gets the new S-Needs-SME tag and stays. No getting nerd-sniped! There's more triage to do!
A PR for a frustrum gizmo. Super handy, but the author hasn't touched this since October despite pings. I've slapped an Adopt-Me on this and cutting from the milestone.
Life happens and interest will naturally ebb and flow in projects. It's really important to make sure that your open source project is robust to this: it's not the original author's responsibility to finish this. Bevy just has to make the best of the gifts given <3
Oh wow, someone adopted the old command error handling work?? I totally missed this. I desperately want this, but now is not the right time. Apologizing to the author, labelling more and moving to the 0.15 milestone.
Okay, a nice hooks API change: support using closures (which can capture state) rather than just function pointers. I really like this ergonomically but there's discussion about the performance costs and tradeoffs. I'll bother james7132, the author and SME-ECS about what he wants to do and how I can help.
IMO it's good to try and get this sort of polish work in for the big new features of the release, so it stays for now.
An interesting performance PR that consolidates Bevy's task pools into a single pool for more flexibility. Neat, but the comments hint at complex problems. This isn't a critical bug, nearly ready or polish on a major feature, so this gets cut from the milestone. Labelling it as we go at least.
Hooks are designed as a building block for relations, primarily to enforce structural invariants like "every child entity has a parent". We're currently doing this manually with a super locked down interface. Which... mostly works. We should use the shiny new tool as designed.
However, this is a non-trivial bit of work with lots of performance and correctness concerns. This can absolutely wait (both for work management and just out of precaution). Cutting.
More gizmos! This time, rounded boxes. Cute :3 The linked tracking issue is so so good.
I like fleshing this out, and can see where this would be valuable for visualization and prototyping purposes. Just needs one review, let me do that quick...
Clean, simple boring code, render looks solid, I think this should exist. That's an easy LGTM: approving and merging.
"Look into sorting rendering pipelines by bindgroups". Like the issue says, this could make Bevy's rendering faster! Neat and a reasonable idea, but not milestone material at this stage. There's no linked PR, so out it goes <3
A neat little PR that adds more flexibility to how users can render UI by letting them pass in their own draw function. There's a bit of healthy debate about whether or not this is the best idiom. Definitely not milestone material, cutting. Neat though, and I'll label it fully while I'm here.
A panicking asset loading bug that seems to be the result of a race condition. Multiple users want a fix for this, but no fix is outstanding. Definitely high enough impact, but may not get a fix in time :(
I'll leave it for now and see about trying to reproduce and investigate this. Thankfully there's a (stochastic) Minimum Reproducible Example to start with.
A rendering crash due to missing cubemap support. Oh hey, another of those nasty platform fragmentation bugs. We should do better about checking for capabilities, as Ida says in the comments.
Without anyone who can currently test this fix, this is going to be tricky. Cutting from the milestone due to niche impact and difficulty verifying a fix, but like always I'm happy to merge fixes like this if they happen to make it.
Blending High Dynamic Range and normal cameras breaks rendering. This is a relatively old bug with moderate impact, and now unblocked. Would be great to get in, but not something that I'm willing to block a release on.
I'll leave it in for the weekend: Jasmine did a pass on the rendering milestone and folks may have plans.
Naming confusion and bikeshedding between view and inverse view matrixes.
> I've added this to the 0.13 milestone as I do think its worth resolving this in a timely fashion. Worth re-evaluating what naming conventions we want (ex: the long descriptive names vs the short "common" names).
cart
We're now another release beyond that: this gets added to my list of "things to bug Cart about". This is probably getting closed as won't-fix.
God rays! This is an awesome feature that just needs merge conflict resolution. There's a PR to the branch with the resolution ready, the author just needs to double-check it and press the button.
This stays, but I'll ping the author on Discord <3 Very easy for this sort of thing to slip one's mind.
@bevy Alright, I've done a full pass now! I don't know where the 4 remaining items went: they may have been issues that were closed as I merged PRs 😅
We've gone from 42 items in the milestone down to a much more manageable 22, now with beautiful color coded labels on everything ;) If you'd like to help out with the #bevy 0.14 release, tackle one of those items: reviewing, implementing, testing and designing are all welcome. Now, it's time for lunch <3
After that PR is merged the attribute still needs to be move to the diagnostic namespace. After that we hopefully can talk about adding this attribute to stable rust.
Add comment