Starting off with an HDR PR I see. This part of a long series of PRs to refactor Bevy's rendering pipelines to allow us to render more, brighter colors on monitors that support it. Doing so breaks a number of critical assumptions, so the refactoring required to do it properly is pretty rough. And the reviewers have been pretty brutal too: "Conversation: 60" is pretty typical.
Still, this looks like it's in a good spot now. The details here are not super interesting: we're moving some code around to make sure we're not wasting work and can support both tonemapped and not tonemapped scenes, even in the same project or scene. The important thing for me as a maintainer is that we have two reviewers who've been brought up to speed on this stuff and fully internalize the new model, and Jasmine's taste in particular here is excellent: I trust that she will push back against pointless complexity.
Merging: let's get one step closer to real HDR support. Maybe my next monitor will even be able to show it!
And step two for today on HDR: tracking how each window wants to have stuff rendered to it, in terms of color.
Really nice use of required components; I was surprised upthread that this goes on Window,
not Monitor, but apparently that's not how the windowing stack works! Really good explanation here.
I think this is a good design, and it's nearly ready. That said, I have some complaints: a doc comment about how Bevy doesn't write to this value is likely to go stale badly later, and there's a couple of tests that are not any more reliable than the underlying code. If it doesn't express an important invariant in a way that's more robust than just writing it right the first time, it's probably not worth including.
Changes requested, but I'll probably merge this tomorrow :)
A small PR that allows for slightly better performance in bevy_solari (Bevy's realtime path-traced lighting) by skipping an unnecessary step. Cool! I think it's really quite neat that Bevy uses a huge amount of shared foundation for several very different rendering solutions. Our standard PBR is classic real-time game engine code: fast approximations that look nice, while bevy_solari is the modern "just simulate the light lol" approach. Super cool that you can just plug that in!
Anyways, simple PR, approved by our resident Solari expert. This makes sense to me, merging.
More information about system ordering edges over BRP! Sweet! Uncontroversially good, simply made, merging.
So, why does this matter? Why would you want this information sent over the wire to a different program? Well: an interactive schedule visualizer is a damn good reason!
This is a really sweet community prototype of something that I've wanted for years and years: allowing you to interactively scroll and filter systems and their relationships to understand logic flow and debug problems. Better believe we're taking notes!
Welp I guess it really is a rendering sort of day. This time, a simple bug fix for environment maps, which are used to efficiently simulate the appearance of a reflective texture using a precomputed texture for what it should look like.
Man, some of the videos / images are real "it's the same picture". Maybe I think the fixed video looks better? Well, the source code of the fixes makes sense to me: this is a pretty clear wiring bug.
Merging.
A break from rendering?? And an exciting PR? Holy shit.
This is the first "we have an entity inspector baby" PR in a very, very long series, across multiple people and prototypes. Read-only, simple tree, but a very very real step. So cool to see this paying off.
Splitting this out into reviewable chunks has been such a pain: this is going to be like 10k+ lines of complex UI and reflection code over the whole series; it's been really good to collaborate with Joe on getting this refined and merged.
enum InspectorSource: "Where the inspector reads its data from" is fun to see. Remote inspection as a first class feature is so cool;
decoupling like that is soo good for crash resistance and to manage recompilation.
Merging with joy. Editing soon!
I should really go finish up my open PRs in this series tomorrow...
Just kidding, back to rendering. It's all rendering sorry! Another edge case bug, tracked down by AI, verified and fixed by a local expert. My feelings about code / docs generation are pretty mixed, but tracking down awful bugs like this? Great! So many hours of expert time saved, and real bugs fixed.
Simple enough, with a very helpful self-review comment by Jasmine explaining how a seperate path did the same thing. Merging.
What it says on the can. Double-sided materials are objects that are rendered on both the front and back side of the material, or more clearly, can be seen from both the inside and out. If you've ever clipped out of bounds in a game and seen the wall disappear, that's why! The PR description says this is needed for leaves, which makes sense: modelling these as fully 2D objects will save you polygons!
Nice little feature, merging.
"This slightly reduces performance (admittedly on a tiny scene), but greatly reduces noise" this line from the description is the core essence of raytracing: you're trying to efficiently estimate some vast, painfully expensive distribution. "Throw more samples at it" only gets you so far, especially in realtime applications like video games. So you need to find tricks that give you a good bang for your buck. Apparently this is one of them!
Yes Jasmine I will merge more of your PRs. What is my purpose in life... I click the button apparently :p
I can't believe it's not rendering!
Serialization bug for bevy_settings: values saved as None weren't getting restored properly.
That's not particularly surprising: a lot of languages and serialization formats don't properly distinguish None from "missing".
Important little bug fix, and helps me feel better about the choice to include bevy_settings upstream in Bevy itself,
even if sometimes it feels too simple / opinionated for that.
Nice regression test, simple change, merging.
This is a nice example of what reflection code looks like in practice BTW!
fn is_option_type(type_info: &TypeInfo) -> bool {
type_info.type_path_table().module_path() == Some("core::option")
&& type_info.type_path_table().ident() == Some("Option")
}Nothing crazy: just a whole bunch of structured data about types at runtime.
I thought surely I was done with Solari PRs today :p Nope, instead we're looking at another simple feature PR.
"Stacked on top of #25930, hence why this is in draft at the moment. It's ready for review though, see the second commit." Well, I'm merging that so the merge queue can handle this if it's ready to go in without a problem.
Supporting UV offsets and transformations seems pretty uncontroversial: these are common fields authored in real models. Why would you use them though? Hmm, let me search to satisfy my curiousity: I am not a working 3D modeller or rendering engineer. Oh!! It's good for procedural animations and variations! Right; just sample a different part of the rock texture.
Normal flipping is apparently a coordinate system fix problem though. Ah conventions in 3D software are a mess...
Anyways, this looks fine. It can go in too :)
Don't crash when you try to clear the text cursor. Yeah man, that does seem like a good thing to fix. Into the 0.20 milestone it goes for backporting; that's nasty.
Simple fix, with a regression test. Merging, love that.
Now, why on earth is a misplaced cursor a crash rather than gracefully recover?
Is that our fault?
Hmm nope, looks like Parley's fault: EditableText::set_text should be clamping to avoid this sort of out-of-bounds crash.
Issue filing time: linebender/parley#849
It's important to be a good citizen!
Aaaaaa yeah this category of bug is hell. Very disruptive, and an absolute nuisance to exterminate. I love floating point rounding!
I'm fine with an epsilon to make the behavior a bit stickier. It fixes the problem empirically, and it's not that hacky. Better fix welcome if someone wants to go hunting, but I'm not going to block on it. I'm especially relieved that ickshonpe agrees. Merging.
A late addition to the merge train, spotted by refreshing the list to make sure I actually remembered to click the button for every PR T_T
This is a nasty new bug in our ECS, where change detection in exclusive systems was broken. Not bueno!
This logic is tricky though; I've spent quite a long time poring over this as a reviewer, and the initial fix still had an edge case where failed systems did not have their change ticks updated appropriately. That regression test is in now and it's green, so this got my approval.
Looks like one of our other erstwhile ECS reviewers (ty hymm!) has had a chance to review this too, so we have the requisite two approvals! Let's go! Merging, but I also spun out an issue for the 7 nanosecond regression caused by not using an associated constant, as spotted by said reviewer :p Every ounce counts as they say!