diff --git a/include/core/SkTypes.h b/include/core/SkTypes.h index 4d590c24de..bdd2e930ae 100644 --- a/include/core/SkTypes.h +++ b/include/core/SkTypes.h @@ -300,6 +300,9 @@ static inline bool SkIsU16(long x) { #define SkAlign8(x) (((x) + 7) >> 3 << 3) #define SkIsAlign8(x) (0 == ((x) & 7)) +#define SkAlignPtr(x) (sizeof(void*) == 8 ? SkAlign8(x) : SkAlign4(x)) +#define SkIsAlignPtr(x) (sizeof(void*) == 8 ? SkIsAlign8(x) : SkIsAlign4(x)) + typedef uint32_t SkFourByteTag; #define SkSetFourByteTag(a, b, c, d) (((a) << 24) | ((b) << 16) | ((c) << 8) | (d)) diff --git a/src/core/SkRecord.h b/src/core/SkRecord.h index 96da69b12e..203a16c4e8 100644 --- a/src/core/SkRecord.h +++ b/src/core/SkRecord.h @@ -65,7 +65,8 @@ public: // Here T can be any class, not just those from SkRecords. Throws on failure. template T* alloc(size_t count = 1) { - return (T*)fAlloc.allocThrow(sizeof(T) * count); + // Bump up to the next pointer width if needed, so all allocations start pointer-aligned. + return (T*)fAlloc.allocThrow(SkAlignPtr(sizeof(T) * count)); } // Add a new command of type T to the end of this SkRecord. diff --git a/tests/RecordTest.cpp b/tests/RecordTest.cpp index 2240ae9858..2a0e615516 100644 --- a/tests/RecordTest.cpp +++ b/tests/RecordTest.cpp @@ -78,3 +78,28 @@ DEF_TEST(Record, r) { #undef APPEND +template +static bool is_aligned(const T* p) { + return (((uintptr_t)p) & (sizeof(T) - 1)) == 0; +} + +DEF_TEST(Record_Alignment, r) { + SkRecord record; + + // Of course a byte's always aligned. + REPORTER_ASSERT(r, is_aligned(record.alloc())); + + // (If packed tightly, the rest below here would be off by one.) + + // It happens that the first implementation always aligned to 4 bytes, + // so these two were always correct. + REPORTER_ASSERT(r, is_aligned(record.alloc())); + REPORTER_ASSERT(r, is_aligned(record.alloc())); + + // These two are regression tests (void* only on 64-bit machines). + REPORTER_ASSERT(r, is_aligned(record.alloc())); + REPORTER_ASSERT(r, is_aligned(record.alloc())); + + // We're not testing beyond sizeof(void*), which is where the current implementation will break. +} +