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>
This commit is contained in:
Florin Malita 2019-12-15 03:01:09 +00:00 committed by Skia Commit-Bot
parent ef363a9ce6
commit 187cd367d3
2 changed files with 68 additions and 38 deletions

View File

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

View File

@ -11,9 +11,6 @@
#include "modules/sksg/include/SkSGRenderEffect.h"
#include "src/utils/SkJSON.h"
#include <algorithm>
#include <iterator>
namespace skottie {
namespace internal {
@ -22,47 +19,81 @@ EffectBuilder::EffectBuilder(const AnimationBuilder* abuilder, const SkSize& lay
, fLayerSize(layer_size) {}
EffectBuilder::EffectBuilderT EffectBuilder::findBuilder(const skjson::ObjectValue& jeffect) const {
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 },
// First, try assigned types.
enum : int32_t {
kTint_Effect = 20,
kFill_Effect = 21,
kTritone_Effect = 23,
kDropShadow_Effect = 25,
kRadialWipe_Effect = 26,
kGaussianBlur_Effect = 29,
};
const skjson::StringValue* mn = jeffect["mn"];
if (!mn) {
return nullptr;
const auto ty = ParseDefault<int>(jeffect["ty"], -1);
switch (ty) {
case kTint_Effect:
return &EffectBuilder::attachTintEffect;
case kFill_Effect:
return &EffectBuilder::attachFillEffect;
case kTritone_Effect:
return &EffectBuilder::attachTritoneEffect;
case kDropShadow_Effect:
return &EffectBuilder::attachDropShadowEffect;
case kRadialWipe_Effect:
return &EffectBuilder::attachRadialWipeEffect;
case kGaussianBlur_Effect:
return &EffectBuilder::attachGaussianBlurEffect;
default:
break;
}
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;
});
// Some effects don't have an assigned type, but the data is still present.
// Try a name-based lookup.
if (binfo == std::end(gBuilderInfo) || strcmp(binfo->fName, key.fName)) {
fBuilder->log(Logger::Level::kWarning, nullptr,
"Unsupported layer effect: %s.", mn->begin());
return nullptr;
static constexpr char kGradientEffectMN[] = "ADBE Ramp",
kHueSaturationMN[] = "ADBE HUE SATURATION",
kLevelsEffectMN[] = "ADBE Easy Levels2",
kLinearWipeEffectMN[] = "ADBE Linear Wipe",
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;
}
}
return binfo->fBuilder;
fBuilder->log(Logger::Level::kWarning, nullptr, "Unsupported layer effect type: %d.", ty);
return nullptr;
}
sk_sp<sksg::RenderNode> EffectBuilder::attachEffects(const skjson::ArrayValue& jeffects,