Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upBatch allocation refactor #3957
Conversation
|
A couple of minor questions in the comments, looks good otherwise! It might be worth taking a quick look at the opaque / alpha sampler counts in the profiler overlay on a few pages before / after this change, just to be sure we're not adding more overdraw due to some weird edge case? |
| @@ -211,102 +369,116 @@ impl AlphaBatchList { | |||
| // The bounding box of everything at this Z plane. We expect potentially | |||
| // multiple primitive segments coming with the same `z_id`. | |||
| z_bounding_rect: &PictureRect, | |||
| z_id: ZBufferId, | |||
| _z_id: ZBufferId, | |||
This comment has been minimized.
This comment has been minimized.
gw3583
May 26, 2020
Collaborator
I guess we don't need this check anymore, but let's remove the param altogether if that's the case?
| } | ||
| } | ||
| } | ||
| // Note: we are able to reorder the opaque batches as needed, theoretically. |
This comment has been minimized.
This comment has been minimized.
gw3583
May 26, 2020
Collaborator
I don't think this will have any effect on fill rate. By definition, batch merging only occurs for non-overlapping regions of a target (separate render tasks). Does that have an effect on what we should do here by default?
|
Thank you! I forgot to mention (here) that the latest code to review is in https://phabricator.services.mozilla.com/D76715 |
|
|
kvark commentedMay 21, 2020
No description provided.