From 6e87a54ba62685422153cc06441dda6dcf813f0d Mon Sep 17 00:00:00 2001 From: John Stiles Date: Thu, 16 Jun 2022 13:51:49 -0400 Subject: [PATCH] Add method to calculate size of a DataPayloadField. Using a switch lets the compiler flag an error if we add a DataPayloadType and forget to update this function. Despite initial appearances it's quite efficient when compiled: https://godbolt.org/z/vqoaddGn4 Change-Id: I5f650afdf81205942966fb318643ff4af5a29252 Bug: skia:13428 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/550501 Reviewed-by: Robert Phillips Commit-Queue: John Stiles --- src/core/SkPaintParamsKey.cpp | 32 +++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/src/core/SkPaintParamsKey.cpp b/src/core/SkPaintParamsKey.cpp index d85765e873..5697f6aa27 100644 --- a/src/core/SkPaintParamsKey.cpp +++ b/src/core/SkPaintParamsKey.cpp @@ -11,6 +11,9 @@ #include "src/core/SkKeyHelpers.h" #include "src/core/SkShaderCodeDictionary.h" +using DataPayloadType = SkPaintParamsKey::DataPayloadType; +using DataPayloadField = SkPaintParamsKey::DataPayloadField; + //-------------------------------------------------------------------------------------------------- SkPaintParamsKeyBuilder::SkPaintParamsKeyBuilder(const SkShaderCodeDictionary* dict, SkBackend backend) @@ -53,12 +56,12 @@ void SkPaintParamsKeyBuilder::beginBlock(int codeSnippetID) { fStack.back().fNumActualChildren++; } - static const SkPaintParamsKey::DataPayloadField kHeader[2] = { - {"snippetID", SkPaintParamsKey::DataPayloadType::kByte, 1}, - {"blockSize", SkPaintParamsKey::DataPayloadType::kByte, 1}, + static constexpr DataPayloadField kHeader[2] = { + {"snippetID", DataPayloadType::kByte, 1}, + {"blockSize", DataPayloadType::kByte, 1}, }; - static const SkSpan kHeaderExpectations(kHeader, 2); + static const SkSpan kHeaderExpectations(kHeader); #endif SkASSERT(!this->isLocked()); @@ -127,8 +130,7 @@ void SkPaintParamsKeyBuilder::endBlock() { } #ifdef SK_DEBUG -void SkPaintParamsKeyBuilder::checkExpectations(SkPaintParamsKey::DataPayloadType actualType, - uint32_t actualCount) { +void SkPaintParamsKeyBuilder::checkExpectations(DataPayloadType actualType, uint32_t actualCount) { StackFrame& frame = fStack.back(); const auto& expectations = frame.fDataPayloadExpectations; @@ -152,7 +154,7 @@ void SkPaintParamsKeyBuilder::addBytes(uint32_t numBytes, const uint8_t* data) { return; } - SkDEBUGCODE(this->checkExpectations(SkPaintParamsKey::DataPayloadType::kByte, numBytes);) + SkDEBUGCODE(this->checkExpectations(DataPayloadType::kByte, numBytes);) SkASSERT(!this->isLocked()); fData.append(numBytes, data); @@ -169,7 +171,7 @@ void SkPaintParamsKeyBuilder::add(const SkColor4f& color) { return; } - SkDEBUGCODE(this->checkExpectations(SkPaintParamsKey::DataPayloadType::kFloat4, 1);) + SkDEBUGCODE(this->checkExpectations(DataPayloadType::kFloat4, 1);) SkASSERT(!this->isLocked()); fData.append(16, reinterpret_cast(&color)); @@ -377,18 +379,26 @@ SkSpan SkPaintParamsKey::BlockReader::dataPayload() const { return fBlock.subspan(payloadOffset, payloadSize); } +static int field_size(const DataPayloadField& field) { + switch (field.fType) { + case DataPayloadType::kByte: + case DataPayloadType::kPointerIndex: return field.fCount; + case DataPayloadType::kFloat4: return field.fCount * 16; + } + SkUNREACHABLE; +} + SkSpan SkPaintParamsKey::BlockReader::bytes(int fieldIndex) const { SkASSERT(fEntry->fDataPayloadExpectations[fieldIndex].fType == DataPayloadType::kByte); int byteOffsetInPayload = 0; for (int i = 0; i < fieldIndex; ++i) { - SkASSERT(fEntry->fDataPayloadExpectations[i].fType == DataPayloadType::kByte); - byteOffsetInPayload += fEntry->fDataPayloadExpectations[i].fCount; + byteOffsetInPayload += field_size(fEntry->fDataPayloadExpectations[i]); } SkSpan dataPayload = this->dataPayload(); return dataPayload.subspan(byteOffsetInPayload, - fEntry->fDataPayloadExpectations[fieldIndex].fCount); + field_size(fEntry->fDataPayloadExpectations[fieldIndex])); } #ifdef SK_DEBUG