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

Issue 1284333002: Revert of Refactor to put SkXfermode_opts inside SK_OPTS_NS. (Closed)

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

Description

Revert of Refactor to put SkXfermode_opts inside SK_OPTS_NS. (patchset #1 id:1 of https://codereview.chromium.org/1286093004/ ) Reason for revert: Maybe causing test / gold problems? Original issue's description: > Refactor to put SkXfermode_opts inside SK_OPTS_NS. > > Without this refactor I was getting warnings previously about having code > inside namespace SK_OPTS_NS (e.g. namespace sse2, namespace neon) referring to > code inside an anonymous namespace (Sk4px, SkPMFloat, Sk4f, etc) [1]. > > That low-level code was in an anonymous namespace to allow multiple independent > copies of its methods to be instantiated without the linker getting confused / > offended about violating the One Definition Rule. This was only happening in > Debug mode where the methods were not being inlined. > > To fix this all, I've force-inlined the methods of the low-level code and > removed the anonymous namespace. > > BUG=skia:4117 > > > [1] Here is what those errors looked like: > > In file included from ../../../../src/core/SkOpts.cpp:18:0: > ../../../../src/opts/SkXfermode_opts.h:193:7: error: 'portable::Sk4pxXfermode' has a field 'portable::Sk4pxXfermode::fProc4' whose type uses the anonymous namespace [-Werror] > class Sk4pxXfermode : public SkProcCoeffXfermode { > ^ > ../../../../src/opts/SkXfermode_opts.h:193:7: error: 'portable::Sk4pxXfermode' has a field 'portable::Sk4pxXfermode::fAAProc4' whose type uses the anonymous namespace [-Werror] > ../../../../src/opts/SkXfermode_opts.h:235:7: error: 'portable::SkPMFloatXfermode' has a field 'portable::SkPMFloatXfermode::fProcF' whose type uses the anonymous namespace [-Werror] > class SkPMFloatXfermode : public SkProcCoeffXfermode { > ^ > cc1plus: all warnings being treated as errors > > Committed: https://skia.googlesource.com/skia/+/b07bee3121680b53b98b780ac08d14d374dd4c6f TBR=djsollen@google.com,mtklein@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=skia:4117 Committed: https://skia.googlesource.com/skia/+/082e329887a8f1efe4e1020f0a0a6ea09961712d

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+149 lines, -97 lines) Patch
M src/core/Sk4px.h View 2 chunks +9 lines, -0 lines 0 comments Download
M src/core/SkNx.h View 2 chunks +9 lines, -0 lines 0 comments Download
M src/core/SkOpts.cpp View 1 chunk +1 line, -1 line 0 comments Download
M src/core/SkPMFloat.h View 2 chunks +9 lines, -0 lines 0 comments Download
M src/opts/Sk4px_NEON.h View 5 chunks +29 lines, -27 lines 0 comments Download
M src/opts/Sk4px_SSE2.h View 6 chunks +31 lines, -33 lines 0 comments Download
M src/opts/Sk4px_none.h View 5 chunks +27 lines, -23 lines 0 comments Download
M src/opts/SkNx_neon.h View 2 chunks +4 lines, -0 lines 0 comments Download
M src/opts/SkNx_sse.h View 2 chunks +5 lines, -0 lines 0 comments Download
M src/opts/SkOpts_neon.cpp View 1 chunk +1 line, -1 line 0 comments Download
M src/opts/SkOpts_sse2.cpp View 1 chunk +1 line, -1 line 0 comments Download
M src/opts/SkPMFloat_neon.h View 3 chunks +7 lines, -3 lines 0 comments Download
M src/opts/SkPMFloat_none.h View 3 chunks +7 lines, -3 lines 0 comments Download
M src/opts/SkPMFloat_sse.h View 3 chunks +7 lines, -3 lines 0 comments Download
M src/opts/SkXfermode_opts.h View 2 chunks +2 lines, -2 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
mtklein
Created Revert of Refactor to put SkXfermode_opts inside SK_OPTS_NS.
5 years, 4 months ago (2015-08-12 18:56:25 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1284333002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1284333002/1
5 years, 4 months ago (2015-08-12 18:56:39 UTC) #2
commit-bot: I haz the power
5 years, 4 months ago (2015-08-12 18:56:49 UTC) #3
Message was sent while issue was closed.
Committed patchset #1 (id:1) as
https://skia.googlesource.com/skia/+/082e329887a8f1efe4e1020f0a0a6ea09961712d

Powered by Google App Engine
This is Rietveld 408576698