AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Reland "[skottie] Simplify effect builder lookup"

Browse Source 
This reverts commit 187cd367d3.

Reason for revert: relanding with legacy enum support

Original change's description:
> Revert "[skottie] Simplify effect builder lookup"
> 
> This reverts commit ef363a9ce6.
> 
> Reason for revert: G3 unit tests failing
> 
> Original change's description:
> > [skottie] Simplify effect builder lookup
> > 
> > Layer effects fall into two categories:
> > 
> >   - effects that BM knows about: these get assigned a unique type enum
> >   - effects that BM doesn't know about: these are still exported, but
> >     get assigned a dummy type
> > 
> > To handle effects in the latter case, we rely on their canonical AE
> > name.
> > 
> > The list of supported effects has grown to the point where a) this
> > differentiation doesn't seem valuable anymore and b) the code is quite
> > repetitive.
> > 
> > Consoliate the lookup logic to rely solely on effect names + bsearch
> > table.
> > 
> > Change-Id: Ib5f9b064a373814865da9e8a26037209992e8b9b
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/259997
> > Commit-Queue: Florin Malita <fmalita@chromium.org>
> > Reviewed-by: Mike Reed <reed@google.com>
> > Reviewed-by: Mike Klein <mtklein@google.com>
> 
> TBR=mtklein@google.com,fmalita@chromium.org,reed@google.com
> 
> Change-Id: I3b4c681c260c121e422ade7395c33a77e788ff43
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/260196
> Reviewed-by: Florin Malita <fmalita@chromium.org>
> Commit-Queue: Florin Malita <fmalita@chromium.org>

TBR=mtklein@google.com,fmalita@chromium.org,reed@google.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Change-Id: I2a4360dc8216b8b45e20c6568c0a1d3d069aa56c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/260280
Reviewed-by: Florin Malita <fmalita@chromium.org>
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Florin Malita <fmalita@chromium.org>
This commit is contained in:
Florin Malita 2019-12-16 12:58:52 -05:00 committed by Skia Commit-Bot
parent 26300d64ca
commit 42725ae301
2 changed files with 57 additions and 62 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
modules/skottie/src/SkottieTest.cpp
View File

@ -78,6 +78,7 @@ DEF_TEST(Skottie_Properties, reporter) {
{ "v": { "a": 0, "k": 1 }} { "v": { "a": 0, "k": 1 }}
], ],
"nm": "fill_effect_0", "nm": "fill_effect_0",
"mn": "ADBE Fill",
"ty": 21 "ty": 21
}], }],
"shapes": [ "shapes": [

118
modules/skottie/src/effects/Effects.cpp
View File

@ -11,6 +11,9 @@
#include "modules/sksg/include/SkSGRenderEffect.h" #include "modules/sksg/include/SkSGRenderEffect.h"
#include "src/utils/SkJSON.h" #include "src/utils/SkJSON.h"
#include <algorithm>
#include <iterator>
namespace skottie { namespace skottie {
namespace internal { namespace internal {
@ -19,7 +22,44 @@ EffectBuilder::EffectBuilder(const AnimationBuilder* abuilder, const SkSize& lay
, fLayerSize(layer_size) {} , fLayerSize(layer_size) {}
EffectBuilder::EffectBuilderT EffectBuilder::findBuilder(const skjson::ObjectValue& jeffect) const { EffectBuilder::EffectBuilderT EffectBuilder::findBuilder(const skjson::ObjectValue& jeffect) const {
// First, try assigned types. static constexpr struct BuilderInfo {
const char* fName;
EffectBuilderT fBuilder;
} gBuilderInfo[] = {
{ "ADBE Drop Shadow" , &EffectBuilder::attachDropShadowEffect },
{ "ADBE Easy Levels2" , &EffectBuilder::attachLevelsEffect },
{ "ADBE Fill" , &EffectBuilder::attachFillEffect },
{ "ADBE Gaussian Blur 2", &EffectBuilder::attachGaussianBlurEffect },
{ "ADBE Geometry2" , &EffectBuilder::attachTransformEffect },
{ "ADBE HUE SATURATION" , &EffectBuilder::attachHueSaturationEffect },
{ "ADBE Invert" , &EffectBuilder::attachInvertEffect },
{ "ADBE Linear Wipe" , &EffectBuilder::attachLinearWipeEffect },
{ "ADBE Radial Wipe" , &EffectBuilder::attachRadialWipeEffect },
{ "ADBE Ramp" , &EffectBuilder::attachGradientEffect },
{ "ADBE Shift Channels" , &EffectBuilder::attachShiftChannelsEffect },
{ "ADBE Tile" , &EffectBuilder::attachMotionTileEffect },
{ "ADBE Tint" , &EffectBuilder::attachTintEffect },
{ "ADBE Tritone" , &EffectBuilder::attachTritoneEffect },
{ "ADBE Venetian Blinds", &EffectBuilder::attachVenetianBlindsEffect },
};
const skjson::StringValue* mn = jeffect["mn"];
if (mn) {
const BuilderInfo key { mn->begin(), nullptr };
const auto* binfo = std::lower_bound(std::begin(gBuilderInfo),
std::end (gBuilderInfo),
key,
[](const BuilderInfo& a, const BuilderInfo& b) {
return strcmp(a.fName, b.fName) < 0;
});
if (binfo != std::end(gBuilderInfo) && !strcmp(binfo->fName, key.fName)) {
return binfo->fBuilder;
}
}
// Some legacy clients rely solely on the 'ty' field and generate (non-BM) JSON
// without a valid 'mn' string. TODO: we should update them and remove this fallback.
enum : int32_t { enum : int32_t {
kTint_Effect = 20, kTint_Effect = 20,
kFill_Effect = 21, kFill_Effect = 21,
@ -29,69 +69,23 @@ EffectBuilder::EffectBuilderT EffectBuilder::findBuilder(const skjson::ObjectVal
kGaussianBlur_Effect = 29, kGaussianBlur_Effect = 29,
}; };
const auto ty = ParseDefault<int>(jeffect["ty"], -1); switch (ParseDefault<int>(jeffect["ty"], -1)) {
case kTint_Effect: return &EffectBuilder::attachTintEffect;
switch (ty) { case kFill_Effect: return &EffectBuilder::attachFillEffect;
case kTint_Effect: case kTritone_Effect: return &EffectBuilder::attachTritoneEffect;
return &EffectBuilder::attachTintEffect; case kDropShadow_Effect: return &EffectBuilder::attachDropShadowEffect;
case kFill_Effect: case kRadialWipe_Effect: return &EffectBuilder::attachRadialWipeEffect;
return &EffectBuilder::attachFillEffect; case kGaussianBlur_Effect: return &EffectBuilder::attachGaussianBlurEffect;
case kTritone_Effect: default: break;
return &EffectBuilder::attachTritoneEffect;
case kDropShadow_Effect:
return &EffectBuilder::attachDropShadowEffect;
case kRadialWipe_Effect:
return &EffectBuilder::attachRadialWipeEffect;
case kGaussianBlur_Effect:
return &EffectBuilder::attachGaussianBlurEffect;
default:
break;
} }
// Some effects don't have an assigned type, but the data is still present. #ifdef SKOTTIE_LEGACY_EFFECT_LOGFMT
// Try a name-based lookup. fBuilder->log(Logger::Level::kWarning, nullptr,
"Unsupported layer effect type: %d.", ParseDefault<int>(jeffect["ty"], -1));
static constexpr char kGradientEffectMN[] = "ADBE Ramp", #else
kHueSaturationMN[] = "ADBE HUE SATURATION", fBuilder->log(Logger::Level::kError, &jeffect,
kLevelsEffectMN[] = "ADBE Easy Levels2", "Unsupported layer effect: %s", mn ? mn->begin() : "(unknown)");
kLinearWipeEffectMN[] = "ADBE Linear Wipe", #endif
kMotionTileEffectMN[] = "ADBE Tile",
kTransformEffectMN[] = "ADBE Geometry2",
kVenetianBlindsEffectMN[] = "ADBE Venetian Blinds",
kShiftChannelsEffectMN[] = "ADBE Shift Channels",
kInvertEffectMN[] = "ADBE Invert";
if (const skjson::StringValue* mn = jeffect["mn"]) {
if (!strcmp(mn->begin(), kGradientEffectMN)) {
return &EffectBuilder::attachGradientEffect;
}
if (!strcmp(mn->begin(), kHueSaturationMN)) {
return &EffectBuilder::attachHueSaturationEffect;
}
if (!strcmp(mn->begin(), kLevelsEffectMN)) {
return &EffectBuilder::attachLevelsEffect;
}
if (!strcmp(mn->begin(), kLinearWipeEffectMN)) {
return &EffectBuilder::attachLinearWipeEffect;
}
if (!strcmp(mn->begin(), kMotionTileEffectMN)) {
return &EffectBuilder::attachMotionTileEffect;
}
if (!strcmp(mn->begin(), kTransformEffectMN)) {
return &EffectBuilder::attachTransformEffect;
}
if (!strcmp(mn->begin(), kVenetianBlindsEffectMN)) {
return &EffectBuilder::attachVenetianBlindsEffect;
}
if (!strcmp(mn->begin(), kShiftChannelsEffectMN)) {
return &EffectBuilder::attachShiftChannelsEffect;
}
if (!strcmp(mn->begin(), kInvertEffectMN)) {
return &EffectBuilder::attachInvertEffect;
}
}
fBuilder->log(Logger::Level::kWarning, nullptr, "Unsupported layer effect type: %d.", ty);
return nullptr; return nullptr;
} }