tree 772dcfd94008d7c9b0fb223098c6fae4970c1c7e
parent 2790777048d3f00384f68be8081f3a3db615746c
author Michael Ludwig <michaelludwig@google.com> 1713318683 -0400
committer SkCQ <skcq-be@skia-corp.google.com.iam.gserviceaccount.com> 1713371869 +0000

Discard data written to a null BufferWriter

In Ganesh this was written such that a failed buffer mapping would be
detected before it got wrapped in a writer. In the cases where this
wasn't as convenient, a CPU buffer was wrapped in a writer so the
writes went somewhere before the tasks would eventually report the
failure.

In Graphite, we could always employ this same CPU buffer wrapping
approach but maybe it's simpler to just drop the written data if the
mapping failed. Branch prediction should hopefully mean that this
doesn't impact perf. If that's the case, then a follow up CL will just
have BufferManager track if a buffer failed to map so it can trigger
a failed Recording. If there are regressions then we can explore
having BufferManager allocate CPU arrays.

The alternative to checking on each write within the BufferWriter
is to have every call site within Graphite check if it's
writer is valid. This is exactly the same number of if checks and
this seems cleaner to have it be internal.

This should avoid the crashes in b/334461625, although we'll need the
follow-up CL to address correctness from what would now be an a
DrawPass with bind commands pointing to null Buffer resources
or non-null Buffer resources that failed to be mapped and are then
filled with random data.

Bug: b/334461625
Change-Id: Ibbd1c241ac8bd1131c4d58ce7f9d7ea6524636b5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/841836
Reviewed-by: James Godfrey-Kittle <jamesgk@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>