Made the fTypeMask field non-lazily computed.

With this change, the SkMatrix44 is now trivially copyable. It will
do more work than before if multiple mutations are performed on a
matrix before the getType() method is called.

Bug: 938563
Change-Id: I3faf88e2b72de264457ebff61a26608baf3afd2e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/206176
Commit-Queue: Florin Malita <fmalita@chromium.org>
Auto-Submit: Mason Freed <masonfreed@chromium.org>
Reviewed-by: Florin Malita <fmalita@chromium.org>
This commit is contained in:
Mason Freed 2019-04-05 11:50:23 -07:00 committed by Skia Commit-Bot
parent 3c3b194f3b
commit 559d9eea48
2 changed files with 57 additions and 80 deletions

View File

@ -133,7 +133,6 @@ struct SkVector4 {
The SkMatrix44 class holds a 4x4 matrix.
SkMatrix44 is not thread safe unless you've first called SkMatrix44::getType().
*/
class SK_API SkMatrix44 {
public:
@ -157,23 +156,14 @@ public:
constexpr SkMatrix44() : SkMatrix44{kIdentity_Constructor} {}
SkMatrix44(const SkMatrix44& src) {
memcpy(fMat, src.fMat, sizeof(fMat));
fTypeMask.store(src.fTypeMask, std::memory_order_relaxed);
}
SkMatrix44(const SkMatrix44& src) = default;
SkMatrix44& operator=(const SkMatrix44& src) = default;
SkMatrix44(const SkMatrix44& a, const SkMatrix44& b) {
this->setConcat(a, b);
}
SkMatrix44& operator=(const SkMatrix44& src) {
if (&src != this) {
memcpy(fMat, src.fMat, sizeof(fMat));
fTypeMask.store(src.fTypeMask, std::memory_order_relaxed);
}
return *this;
}
bool operator==(const SkMatrix44& other) const;
bool operator!=(const SkMatrix44& other) const {
return !(other == *this);
@ -196,12 +186,13 @@ public:
*/
static const SkMatrix44& I();
enum TypeMask {
using TypeMask = uint8_t;
enum : TypeMask {
kIdentity_Mask = 0,
kTranslate_Mask = 0x01, //!< set if the matrix has translation
kScale_Mask = 0x02, //!< set if the matrix has any scale != 1
kAffine_Mask = 0x04, //!< set if the matrix skews or rotates
kPerspective_Mask = 0x08 //!< set if the matrix is in perspective
kTranslate_Mask = 1 << 0, //!< set if the matrix has translation
kScale_Mask = 1 << 1, //!< set if the matrix has any scale != 1
kAffine_Mask = 1 << 2, //!< set if the matrix skews or rotates
kPerspective_Mask = 1 << 3, //!< set if the matrix is in perspective
};
/**
@ -211,13 +202,7 @@ public:
* other bits may be set to true even in the case of a pure perspective
* transform.
*/
inline TypeMask getType() const {
if (fTypeMask.load(std::memory_order_relaxed) & kUnknown_Mask) {
fTypeMask.store(this->computeTypeMask(), std::memory_order_relaxed);
}
SkASSERT(!(fTypeMask & kUnknown_Mask));
return (TypeMask)fTypeMask.load(std::memory_order_relaxed);
}
inline TypeMask getType() const { return fTypeMask; }
/**
* Return true if the matrix is identity.
@ -276,7 +261,7 @@ public:
SkASSERT((unsigned)row <= 3);
SkASSERT((unsigned)col <= 3);
fMat[col][row] = value;
this->dirtyTypeMask();
this->recomputeTypeMask();
}
inline double getDouble(int row, int col) const {
@ -447,9 +432,7 @@ public:
private:
/* This is indexed by [col][row]. */
SkMScalar fMat[4][4];
mutable std::atomic<unsigned> fTypeMask;
static constexpr int kUnknown_Mask = 0x80;
TypeMask fTypeMask;
static constexpr int kAllPublic_Masks = 0xF;
@ -468,23 +451,11 @@ private:
SkMScalar perspY() const { return fMat[1][3]; }
SkMScalar perspZ() const { return fMat[2][3]; }
int computeTypeMask() const;
void recomputeTypeMask();
inline void dirtyTypeMask() {
fTypeMask.store(kUnknown_Mask, std::memory_order_relaxed);
}
inline void setTypeMask(int mask) {
SkASSERT(0 == (~(kAllPublic_Masks | kUnknown_Mask) & mask));
fTypeMask.store(mask, std::memory_order_relaxed);
}
/**
* Does not take the time to 'compute' the typemask. Only returns true if
* we already know that this matrix is identity.
*/
inline bool isTriviallyIdentity() const {
return 0 == fTypeMask.load(std::memory_order_relaxed);
inline void setTypeMask(TypeMask mask) {
SkASSERT(0 == (~kAllPublic_Masks & mask));
fTypeMask = mask;
}
inline const SkMScalar* values() const { return &fMat[0][0]; }

View File

@ -6,8 +6,19 @@
*/
#include "SkMatrix44.h"
#include <type_traits>
#include <utility>
// Copying SkMatrix44 byte-wise is performance-critical to Blink. This class is
// contained in several Transform classes, which are copied multiple times
// during the rendering life cycle. See crbug.com/938563 for reference.
#if defined(SK_BUILD_FOR_WIN) || defined(SK_BUILD_FOR_MAC)
// std::is_trivially_copyable is not supported for some older clang versions,
// which (at least as of this patch) are in use for Chromecast.
static_assert(std::is_trivially_copyable<SkMatrix44>::value,
"SkMatrix44 must be trivially copyable");
#endif
static inline bool eq4(const SkMScalar* SK_RESTRICT a,
const SkMScalar* SK_RESTRICT b) {
return (a[0] == b[0]) & (a[1] == b[1]) & (a[2] == b[2]) & (a[3] == b[3]);
@ -18,7 +29,7 @@ bool SkMatrix44::operator==(const SkMatrix44& other) const {
return true;
}
if (this->isTriviallyIdentity() && other.isTriviallyIdentity()) {
if (this->isIdentity() && other.isIdentity()) {
return true;
}
@ -49,14 +60,13 @@ bool SkMatrix44::operator==(const SkMatrix44& other) const {
}
///////////////////////////////////////////////////////////////////////////////
int SkMatrix44::computeTypeMask() const {
unsigned mask = 0;
void SkMatrix44::recomputeTypeMask() {
if (0 != perspX() || 0 != perspY() || 0 != perspZ() || 1 != fMat[3][3]) {
return kTranslate_Mask | kScale_Mask | kAffine_Mask | kPerspective_Mask;
fTypeMask = kTranslate_Mask | kScale_Mask | kAffine_Mask | kPerspective_Mask;
return;
}
TypeMask mask = kIdentity_Mask;
if (0 != transX() || 0 != transY() || 0 != transZ()) {
mask |= kTranslate_Mask;
}
@ -69,8 +79,7 @@ int SkMatrix44::computeTypeMask() const {
0 != fMat[2][0] || 0 != fMat[1][2] || 0 != fMat[2][1]) {
mask |= kAffine_Mask;
}
return mask;
fTypeMask = mask;
}
///////////////////////////////////////////////////////////////////////////////
@ -137,7 +146,7 @@ void SkMatrix44::setColMajorf(const float src[]) {
memcpy(dst, src, 16 * sizeof(float));
#endif
this->dirtyTypeMask();
this->recomputeTypeMask();
}
void SkMatrix44::setColMajord(const double src[]) {
@ -150,7 +159,7 @@ void SkMatrix44::setColMajord(const double src[]) {
}
#endif
this->dirtyTypeMask();
this->recomputeTypeMask();
}
void SkMatrix44::setRowMajorf(const float src[]) {
@ -163,7 +172,7 @@ void SkMatrix44::setRowMajorf(const float src[]) {
src += 4;
dst += 1;
}
this->dirtyTypeMask();
this->recomputeTypeMask();
}
void SkMatrix44::setRowMajord(const double src[]) {
@ -176,7 +185,7 @@ void SkMatrix44::setRowMajord(const double src[]) {
src += 4;
dst += 1;
}
this->dirtyTypeMask();
this->recomputeTypeMask();
}
///////////////////////////////////////////////////////////////////////////////
@ -213,7 +222,7 @@ void SkMatrix44::set3x3(SkMScalar m_00, SkMScalar m_10, SkMScalar m_20,
fMat[1][0] = m_01; fMat[1][1] = m_11; fMat[1][2] = m_21; fMat[1][3] = 0;
fMat[2][0] = m_02; fMat[2][1] = m_12; fMat[2][2] = m_22; fMat[2][3] = 0;
fMat[3][0] = 0; fMat[3][1] = 0; fMat[3][2] = 0; fMat[3][3] = 1;
this->dirtyTypeMask();
this->recomputeTypeMask();
}
void SkMatrix44::set3x3RowMajorf(const float src[]) {
@ -221,7 +230,7 @@ void SkMatrix44::set3x3RowMajorf(const float src[]) {
fMat[1][0] = src[1]; fMat[1][1] = src[4]; fMat[1][2] = src[7]; fMat[1][3] = 0;
fMat[2][0] = src[2]; fMat[2][1] = src[5]; fMat[2][2] = src[8]; fMat[2][3] = 0;
fMat[3][0] = 0; fMat[3][1] = 0; fMat[3][2] = 0; fMat[3][3] = 1;
this->dirtyTypeMask();
this->recomputeTypeMask();
}
void SkMatrix44::set3x4RowMajorf(const float src[]) {
@ -229,7 +238,7 @@ void SkMatrix44::set3x4RowMajorf(const float src[]) {
fMat[0][1] = src[4]; fMat[1][1] = src[5]; fMat[2][1] = src[6]; fMat[3][1] = src[7];
fMat[0][2] = src[8]; fMat[1][2] = src[9]; fMat[2][2] = src[10]; fMat[3][2] = src[11];
fMat[0][3] = 0; fMat[1][3] = 0; fMat[2][3] = 0; fMat[3][3] = 1;
this->dirtyTypeMask();
this->recomputeTypeMask();
}
///////////////////////////////////////////////////////////////////////////////
@ -255,7 +264,7 @@ void SkMatrix44::preTranslate(SkMScalar dx, SkMScalar dy, SkMScalar dz) {
for (int i = 0; i < 4; ++i) {
fMat[3][i] = fMat[0][i] * dx + fMat[1][i] * dy + fMat[2][i] * dz + fMat[3][i];
}
this->dirtyTypeMask();
this->recomputeTypeMask();
}
void SkMatrix44::postTranslate(SkMScalar dx, SkMScalar dy, SkMScalar dz) {
@ -273,7 +282,7 @@ void SkMatrix44::postTranslate(SkMScalar dx, SkMScalar dy, SkMScalar dz) {
fMat[3][0] += dx;
fMat[3][1] += dy;
fMat[3][2] += dz;
this->dirtyTypeMask();
this->recomputeTypeMask();
}
}
@ -305,7 +314,7 @@ void SkMatrix44::preScale(SkMScalar sx, SkMScalar sy, SkMScalar sz) {
fMat[1][i] *= sy;
fMat[2][i] *= sz;
}
this->dirtyTypeMask();
this->recomputeTypeMask();
}
void SkMatrix44::postScale(SkMScalar sx, SkMScalar sy, SkMScalar sz) {
@ -318,7 +327,7 @@ void SkMatrix44::postScale(SkMScalar sx, SkMScalar sy, SkMScalar sz) {
fMat[i][1] *= sy;
fMat[i][2] *= sz;
}
this->dirtyTypeMask();
this->recomputeTypeMask();
}
///////////////////////////////////////////////////////////////////////////////
@ -418,7 +427,7 @@ void SkMatrix44::setConcat(const SkMatrix44& a, const SkMatrix44& b) {
if (useStorage) {
memcpy(fMat, storage, sizeof(storage));
}
this->dirtyTypeMask();
this->recomputeTypeMask();
}
///////////////////////////////////////////////////////////////////////////////
@ -678,8 +687,6 @@ bool SkMatrix44::invert(SkMatrix44* storage) const {
inverse->fMat[3][1] = SkDoubleToMScalar(a00 * b09 - a01 * b07 + a02 * b06);
inverse->fMat[3][2] = SkDoubleToMScalar(a31 * b01 - a30 * b03 - a32 * b00);
inverse->fMat[3][3] = SkDoubleToMScalar(a20 * b03 - a21 * b01 + a22 * b00);
inverse->dirtyTypeMask();
inverse->setTypeMask(this->getType());
if (!is_matrix_finite(*inverse)) {
return false;
@ -693,6 +700,7 @@ bool SkMatrix44::invert(SkMatrix44* storage) const {
///////////////////////////////////////////////////////////////////////////////
void SkMatrix44::transpose() {
if (!this->isIdentity()) {
using std::swap;
swap(fMat[0][1], fMat[1][0]);
swap(fMat[0][2], fMat[2][0]);
@ -700,9 +708,7 @@ void SkMatrix44::transpose() {
swap(fMat[1][2], fMat[2][1]);
swap(fMat[1][3], fMat[3][1]);
swap(fMat[2][3], fMat[3][2]);
if (!this->isTriviallyIdentity()) {
this->dirtyTypeMask();
this->recomputeTypeMask();
}
}
@ -990,7 +996,7 @@ SkMatrix44& SkMatrix44::operator=(const SkMatrix& src) {
if (src.isIdentity()) {
this->setTypeMask(kIdentity_Mask);
} else {
this->dirtyTypeMask();
this->recomputeTypeMask();
}
return *this;
}