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

Issue 841753002: Remove macros that make it look like it's a good idea to not be able to flatten. (Closed)

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

Description

Remove macros that make it look like it's a good idea to not be able to flatten. There are only a handful of SkFlattenables that are not flattenable. That there are any seems highly illogical. To make this look less like a normal thing, this removes both macros that marked SkFlattenables as non-flattenable (in slightly different ways). The handful of SkFlattenables in our codebase that can't be flattened now assert violently that they can't be flattened. They're internal or part of animator... places where we'll never actually flatten them. TestLooper and DummyRasterizer were so trivial that I just made them flattenable. BUG=skia: Committed: https://skia.googlesource.com/skia/+/7e44bb191633e225fd0455c267dbf67f9ee8633e

Patch Set 1 #

Patch Set 2 : DummyRasterizer too #

Unified diffs Side-by-side diffs Delta from patch set Stats (+17 lines, -19 lines) Patch
M include/core/SkFlattenable.h View 3 chunks +0 lines, -12 lines 0 comments Download
M include/effects/Sk2DPathEffect.h View 1 chunk +0 lines, -2 lines 0 comments Download
M src/animator/SkDrawExtraPathEffect.cpp View 2 chunks +5 lines, -1 line 0 comments Download
M src/core/SkDraw.cpp View 1 chunk +3 lines, -1 line 0 comments Download
M tests/LayerRasterizerTest.cpp View 1 2 chunks +5 lines, -1 line 0 comments Download
M tests/QuickRejectTest.cpp View 2 chunks +4 lines, -2 lines 0 comments Download

Messages

Total messages: 7 (2 generated)
mtklein
5 years, 11 months ago (2015-01-07 16:15:27 UTC) #2
reed1
no callers in chrome? lgtm
5 years, 11 months ago (2015-01-07 16:20:27 UTC) #3
mtklein
On 2015/01/07 16:20:27, reed1 wrote: > no callers in chrome? > > lgtm None I ...
5 years, 11 months ago (2015-01-07 16:55:56 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/841753002/20001
5 years, 11 months ago (2015-01-07 16:56:23 UTC) #6
commit-bot: I haz the power
5 years, 11 months ago (2015-01-07 17:06:11 UTC) #7
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as
https://skia.googlesource.com/skia/+/7e44bb191633e225fd0455c267dbf67f9ee8633e

Powered by Google App Engine
This is Rietveld 408576698