Revert "[skottie] Simplify effect builder lookup"
This reverts commit ef363a9ce692dd0c79d434b5838838280ab1c075.
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>
diff --git a/modules/skottie/src/SkottieTest.cpp b/modules/skottie/src/SkottieTest.cpp
index 8d6f4f2..fbadcde 100644
--- a/modules/skottie/src/SkottieTest.cpp
+++ b/modules/skottie/src/SkottieTest.cpp
@@ -78,7 +78,6 @@
{ "v": { "a": 0, "k": 1 }}
],
"nm": "fill_effect_0",
- "mn": "ADBE Fill",
"ty": 21
}],
"shapes": [
diff --git a/modules/skottie/src/effects/Effects.cpp b/modules/skottie/src/effects/Effects.cpp
index d8b655d..c91cc01 100644
--- a/modules/skottie/src/effects/Effects.cpp
+++ b/modules/skottie/src/effects/Effects.cpp
@@ -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 @@
, 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,
