vello_hybrid: Remove command annotation logic#1438
vello_hybrid: Remove command annotation logic#1438laurenz-canva wants to merge 13 commits intolinebender:mainfrom
Conversation
# Conflicts: # sparse_strips/vello_hybrid/src/schedule.rs
| /// | ||
| /// TODO: Can be triggered via a const generic on coarse draw cmd generation which will avoid | ||
| /// a linear scan. | ||
| fn prepare_cmds(cmds: &[Cmd], state: &mut SchedulerState) { |
There was a problem hiding this comment.
I frequently see this function show up in browser performance profiles, so I wouldn't be surprised if we see non-trivial performance improvements in Wasm 🎉
There was a problem hiding this comment.
E.g., this frame took 300 microseconds in graphics. 100 of which are in this function!
|
|
||
| let next_round: bool = depth.is_multiple_of(2) && depth > 2; | ||
| let round = nos.round.max(tos.round + usize::from(next_round)); | ||
| #[inline(always)] |
There was a problem hiding this comment.
does it make sense to do inline always compared to just inline?
There was a problem hiding this comment.
I think it makes sense if we really want to be sure, but both would probably work in this case. Just did it because of Andrew's suggestion.
There was a problem hiding this comment.
ok, I was just thinking if there are cases were a compiler might not want to inline it because it might be faster not to?
| wide.pop_buf(); | ||
|
|
||
| assert_eq!(wide.cmds.len(), 4); | ||
| assert_eq!(wide.cmds.len(), 5); |
There was a problem hiding this comment.
should we test that the Start cmd is actually the first entry?
# Conflicts: # sparse_strips/vello_common/src/coarse.rs
5 participants
This PR is stacked on top of #1436, so that should land first.It removes the command annotation logic that is needed to handle blending correctly, and instead moves the bookkeeping into vello_common during coarse rasterization. Basically, we add a new
Startcommand that tells us whether the original surface is a target of blending operations, and eachPushBufcommand has a similar flag. In vello_hybrid, we simply need to query this flag and add some additional processing if the root layer is the target of blending. I've considered putting the logic behind the const generic ofWide, but I think it's probably not worth as it makes the code less straightforward, and I think the performance overhead is negligible.While not as big of a performance improvement as I had hoped, it's still something:
Before:
After:
