Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(96)

Issue 1309523003: Remove SK_OFFSETOF from SkTypes, clean up offsetof usage. (Closed)

Created:
5 years, 4 months ago by bungeman-skia
Modified:
5 years, 4 months ago
Reviewers:
mtklein
CC:
reviews_skia.org
Base URL:
https://skia.googlesource.com/skia.git@master
Target Ref:
refs/heads/master
Project:
skia
Visibility:
Public.

Description

Remove SK_OFFSETOF from SkTypes, clean up offsetof usage. The motivation for this was to remove SK_OFFSETOF from SkTypes, but this CL is mostly about cleaning up our use of offsetof generally. SK_OFFSETOF is removed to SkTypes and added to the two places it is actually used (for the non standard behavior of finding the offset of fields in types which are not standard layout). Older versions of gcc required POD for offsetof to be used without warning. Newer versions require the more relaxed standard layout. Now that we no longer build on older versions of gcc, remove the old warning suppressions. PODMatrix is renamed to AggregateMatrix. SkMatrix is already POD (trivial and standard layout). The PODMatrix name implies that the POD-ness is needed for the offsetof, but it is actually the aggregate attribute which is needed for compile time constant initialization. This makes it more obvious that this can be revisited after we can rely on constexpr constructors. This also adds skstd::declval since this allows removal of existing awkward code which casts a constant to a pointer to find the size of a field. TBR=reed@google.com No API change, only removes unused macro. Committed: https://skia.googlesource.com/skia/+/afd7c749724ccceb88761c52c196ba371be5fb1c

Patch Set 1 #

Total comments: 2

Patch Set 2 : Speling #

Unified diffs Side-by-side diffs Delta from patch set Stats (+51 lines, -32 lines) Patch
M gyp/common_conditions.gypi View 3 chunks +0 lines, -3 lines 0 comments Download
M include/core/SkTypes.h View 1 chunk +0 lines, -3 lines 0 comments Download
M include/private/SkTLogic.h View 1 chunk +28 lines, -5 lines 0 comments Download
M include/private/SkTemplates.h View 1 chunk +2 lines, -0 lines 0 comments Download
M include/svg/parser/SkSVGAttribute.h View 1 chunk +2 lines, -1 line 0 comments Download
M src/animator/SkDisplayInclude.cpp View 1 chunk +0 lines, -7 lines 0 comments Download
M src/animator/SkMemberInfo.h View 1 chunk +7 lines, -2 lines 0 comments Download
M src/core/SkMatrix.cpp View 1 1 chunk +12 lines, -11 lines 0 comments Download

Messages

Total messages: 10 (4 generated)
bungeman-skia
5 years, 4 months ago (2015-08-25 17:59:20 UTC) #2
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1309523003/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1309523003/1
5 years, 4 months ago (2015-08-25 17:59:54 UTC) #4
mtklein
lgtm https://codereview.chromium.org/1309523003/diff/1/src/core/SkMatrix.cpp File src/core/SkMatrix.cpp (right): https://codereview.chromium.org/1309523003/diff/1/src/core/SkMatrix.cpp#newcode1589 src/core/SkMatrix.cpp:1589: // SkMatrix is C++11 POD (trivial and standard-layout), ...
5 years, 4 months ago (2015-08-25 18:04:36 UTC) #5
bungeman-skia
https://codereview.chromium.org/1309523003/diff/1/src/core/SkMatrix.cpp File src/core/SkMatrix.cpp (right): https://codereview.chromium.org/1309523003/diff/1/src/core/SkMatrix.cpp#newcode1589 src/core/SkMatrix.cpp:1589: // SkMatrix is C++11 POD (trivial and standard-layout), but ...
5 years, 4 months ago (2015-08-25 18:07:15 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1309523003/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1309523003/20001
5 years, 4 months ago (2015-08-25 18:07:37 UTC) #9
commit-bot: I haz the power
5 years, 4 months ago (2015-08-25 19:05:58 UTC) #10
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as
https://skia.googlesource.com/skia/+/afd7c749724ccceb88761c52c196ba371be5fb1c

Powered by Google App Engine
This is Rietveld 408576698