Merge pull request #370 from scratchcpp/refactor_graphics_effect_api

Refactor graphics effect API

Author: adazem009

docs/Graphics effects.md

@@ -1,21 +1,10 @@
 \page graphicsEffects Graphics effects
 
-libscratchcpp provides API for implementing custom graphics effects.
-No effects are currently included with libscratchcpp, but that is planned to change.
+libscratchcpp provides API for adding custom graphics effects.
+No effects are included with libscratchcpp and must be implemented by the project player.
 
-In case you've implemented some of the effects yourself, feel free to make pull requests!
-These are the issues (tasks) for the default graphics effects:
-
-- Color: https://github.com/scratchcpp/libscratchcpp/issues/283
-- Fisheye: https://github.com/scratchcpp/libscratchcpp/issues/284
-- Whirl: https://github.com/scratchcpp/libscratchcpp/issues/285
-- Pixelate: https://github.com/scratchcpp/libscratchcpp/issues/286
-- Mosaic: https://github.com/scratchcpp/libscratchcpp/issues/287
-- Brightness: https://github.com/scratchcpp/libscratchcpp/issues/288
-- Ghost: https://github.com/scratchcpp/libscratchcpp/issues/289
-
-# Implementing a graphics effect
-To implement a graphics effect that libscratchcpp doesn't support,
+# Adding a custom graphics effect
+To add a graphics effect that libscratchcpp doesn't support,
 subclass \link libscratchcpp::IGraphicsEffect IGraphicsEffect \endlink
 and override all of the methods.
 
@@ -27,12 +16,14 @@ using namespace libscratchcpp;
 The `name()` method should return the name of the effect which is
 used by the set and change effect blocks, for example `ghost` or `fisheye`.
 
-The `apply()` method is used to apply the effect on a bitmap. The bitmap
-can be accessed using the `bitmap` parameter and is writable.
-
 # Registering the graphics effect
 To register the graphics effect, use \link libscratchcpp::ScratchConfiguration::registerGraphicsEffect() ScratchConfiguration::registerGraphicsEffect() \endlink.
 
 ```cpp
 libscratchcpp::ScratchConfiguration::registerGraphicsEffect(std::make_shared<MyGraphicsEffect>());
 ```
+
+# Why should effects be registered?
+Effects in Scratch are usually identified by their name (`ghost`, `brightness`, etc.), but this isn't effective
+when it comes to running projects. Those "names" are therefore replaced by \link libscratchcpp::IGraphicsEffect IGraphicsEffect \endlink
+pointers which makes working with effects faster.

include/scratchcpp/costume.h

@@ -11,7 +11,6 @@ namespace libscratchcpp
 {
 
 class Broadcast;
-class IGraphicsEffect;
 class CostumePrivate;
 
 /*! \brief The Costume class represents a Scratch costume. */
@@ -41,11 +40,6 @@ class LIBSCRATCHCPP_EXPORT Costume : public Asset
 
         Rgb **bitmap() const;
 
-        double graphicsEffectValue(IGraphicsEffect *effect) const;
-        void setGraphicsEffectValue(IGraphicsEffect *effect, double value);
-
-        void clearGraphicsEffects();
-
         Broadcast *broadcast();
 
     protected:

include/scratchcpp/igraphicseffect.h

@@ -9,17 +9,14 @@
 namespace libscratchcpp
 {
 
-/*! \brief The IGraphicsEffects class is an interface for implementing custom graphics effects. */
+/*! \brief The IGraphicsEffects class is an interface for custom graphics effects. */
 class LIBSCRATCHCPP_EXPORT IGraphicsEffect
 {
     public:
         virtual ~IGraphicsEffect() { }
 
         /*! Returns the name of the graphics effect. */
         virtual std::string name() const = 0;
-
-        /*! Applies the effect on the given bitmap. */
-        virtual void apply(Rgb **bitmap, unsigned int width, unsigned int height, double value) const = 0;
 };
 
 } // namespace libscratchcpp

include/scratchcpp/ispritehandler.h

@@ -41,6 +41,15 @@ class LIBSCRATCHCPP_EXPORT ISpriteHandler
         /*! Called when the rotation style changes. */
         virtual void onRotationStyleChanged(Sprite::RotationStyle rotationStyle) = 0;
 
+        /*!
+         * Called when the value of the given graphics effect changes.
+         * \note This method isn't called when all effects are cleared, use onGraphicsEffectsCleared() for this.
+         */
+        virtual void onGraphicsEffectChanged(IGraphicsEffect *effect, double value) = 0;
+
+        /*! Called when all graphics effects are cleared. */
+        virtual void onGraphicsEffectsCleared() = 0;
+
         /*!
          * Used to get the bounding rectangle of the sprite.
          * \note The rectangle must be relative to the stage, so make sure to use the sprite's coordinates.

include/scratchcpp/istagehandler.h

@@ -28,6 +28,15 @@ class LIBSCRATCHCPP_EXPORT IStageHandler
 
         /*! Called when the video transparency changes. */
         virtual void onVideoTransparencyChanged(int videoTransparency) = 0;
+
+        /*!
+         * Called when the value of the given graphics effect changes.
+         * \note This method isn't called when all effects are cleared, use onGraphicsEffectsCleared() for this.
+         */
+        virtual void onGraphicsEffectChanged(IGraphicsEffect *effect, double value) = 0;
+
+        /*! Called when all graphics effects are cleared. */
+        virtual void onGraphicsEffectsCleared() = 0;
 };
 
 } // namespace libscratchcpp

include/scratchcpp/sprite.h

@@ -67,10 +67,9 @@ class LIBSCRATCHCPP_EXPORT Sprite : public Target
         Rect boundingRect() const;
         void keepInFence(double newX, double newY, double *fencedX, double *fencedY) const;
 
-        double graphicsEffectValue(IGraphicsEffect *effect) const;
-        void setGraphicsEffectValue(IGraphicsEffect *effect, double value);
+        void setGraphicsEffectValue(IGraphicsEffect *effect, double value) override;
 
-        void clearGraphicsEffects();
+        void clearGraphicsEffects() override;
 
     private:
         Target *dataSource() const override;

include/scratchcpp/stage.h

@@ -47,6 +47,10 @@ class LIBSCRATCHCPP_EXPORT Stage : public Target
         const std::string &textToSpeechLanguage() const;
         void setTextToSpeechLanguage(const std::string &newTextToSpeechLanguage);
 
+        void setGraphicsEffectValue(IGraphicsEffect *effect, double value) override;
+
+        void clearGraphicsEffects() override;
+
     private:
         spimpl::unique_impl_ptr<StagePrivate> impl;
 };

include/scratchcpp/target.h

@@ -17,6 +17,7 @@ class Block;
 class Comment;
 class Costume;
 class Sound;
+class IGraphicsEffect;
 class TargetPrivate;
 
 /*! \brief The Target class is the Stage or a Sprite. */
@@ -77,6 +78,11 @@ class LIBSCRATCHCPP_EXPORT Target
         double volume() const;
         void setVolume(double newVolume);
 
+        double graphicsEffectValue(IGraphicsEffect *effect) const;
+        virtual void setGraphicsEffectValue(IGraphicsEffect *effect, double value);
+
+        virtual void clearGraphicsEffects();
+
         IEngine *engine() const;
         void setEngine(IEngine *engine);
 

src/scratch/costume.cpp

@@ -107,42 +107,6 @@ Rgb **Costume::bitmap() const
     return impl->bitmap;
 }
 
-/*! Returns the value of the given graphics effect. */
-double Costume::graphicsEffectValue(IGraphicsEffect *effect) const
-{
-    auto it = impl->graphicsEffects.find(effect);
-
-    if (it == impl->graphicsEffects.cend())
-        return 0;
-    else
-        return it->second;
-}
-
-/*! Sets the value of the given graphics effect (this is automatically set by the sprite). */
-void Costume::setGraphicsEffectValue(IGraphicsEffect *effect, double value)
-{
-    auto it = impl->graphicsEffects.find(effect);
-    bool update = ((it == impl->graphicsEffects.cend()) || (it->second != value));
-
-    if (value == 0)
-        impl->graphicsEffects.erase(effect);
-    else
-        impl->graphicsEffects[effect] = value;
-
-    if (update)
-        impl->updateImage();
-}
-
-/*! Clears all graphics effects (this is automatically called by the sprite). */
-void Costume::clearGraphicsEffects()
-{
-    bool update = !impl->graphicsEffects.empty();
-    impl->graphicsEffects.clear();
-
-    if (update)
-        impl->updateImage();
-}
-
 /*!
  * Returns the Broadcast linked with this costume.
  * \note This is used by the "switch backdrop to and wait" block.

src/scratch/costume_p.cpp

@@ -1,7 +1,5 @@
 // SPDX-License-Identifier: Apache-2.0
 
-#include <scratchcpp/igraphicseffect.h>
-
 #include "costume_p.h"
 
 using namespace libscratchcpp;
@@ -45,9 +43,6 @@ void CostumePrivate::updateImage()
         for (unsigned int j = 0; j < scaledWidth; j++)
             bitmap[i][j] = image->colorAt(mirrorHorizontally ? (scaledWidth - 1 - j) : j, i, actualScale);
     }
-
-    for (const auto &[effect, value] : graphicsEffects)
-        effect->apply(bitmap, scaledWidth, scaledHeight, value);
 }
 
 void CostumePrivate::freeImage()

src/scratch/costume_p.h

@@ -9,8 +9,6 @@
 namespace libscratchcpp
 {
 
-class IGraphicsEffect;
-
 struct CostumePrivate
 {
         CostumePrivate();
@@ -31,7 +29,6 @@ struct CostumePrivate
         double oldScale = 1;
         double scale = 1;
         bool mirrorHorizontally = false;
-        std::unordered_map<IGraphicsEffect *, double> graphicsEffects;
         Broadcast broadcast;
 };
 

src/scratch/sprite.cpp

@@ -233,9 +233,6 @@ void Sprite::setCostumeIndex(int newCostumeIndex)
     if (costume) {
         costume->setScale(impl->size / 100);
         costume->setMirrorHorizontally(impl->rotationStyle == RotationStyle::LeftRight);
-
-        for (const auto &[effect, value] : impl->graphicsEffects)
-            costume->setGraphicsEffectValue(effect, value);
     }
 
     if (impl->visible) {
@@ -403,51 +400,36 @@ void Sprite::keepInFence(double newX, double newY, double *fencedX, double *fenc
     *fencedY = newY + dy;
 }
 
-/*! Returns the value of the given graphics effect. */
-double Sprite::graphicsEffectValue(IGraphicsEffect *effect) const
-{
-    auto it = impl->graphicsEffects.find(effect);
-
-    if (it == impl->graphicsEffects.cend())
-        return 0;
-    else
-        return it->second;
-}
-
-/*! Sets the value of the given graphics effect. */
+/*! Overrides Target#setGraphicsEffectValue(). */
 void Sprite::setGraphicsEffectValue(IGraphicsEffect *effect, double value)
 {
-    impl->graphicsEffects[effect] = value;
-
-    auto costume = currentCostume();
-
-    if (costume)
-        costume->setGraphicsEffectValue(effect, value);
+    Target::setGraphicsEffectValue(effect, value);
 
     if (impl->visible) {
         IEngine *eng = engine();
 
         if (eng)
             eng->requestRedraw();
     }
+
+    if (impl->iface)
+        impl->iface->onGraphicsEffectChanged(effect, value);
 }
 
-/*! Sets the value of all graphics effects to 0 (clears them). */
+/*! Overrides Target#clearGraphicsEffects(). */
 void Sprite::clearGraphicsEffects()
 {
-    impl->graphicsEffects.clear();
-
-    auto costume = currentCostume();
-
-    if (costume)
-        costume->clearGraphicsEffects();
+    Target::clearGraphicsEffects();
 
     if (impl->visible) {
         IEngine *eng = engine();
 
         if (eng)
             eng->requestRedraw();
     }
+
+    if (impl->iface)
+        impl->iface->onGraphicsEffectsCleared();
 }
 
 Target *Sprite::dataSource() const

src/scratch/sprite_p.h

@@ -31,7 +31,6 @@ struct SpritePrivate
         double direction = 90;
         bool draggable = false;
         Sprite::RotationStyle rotationStyle = Sprite::RotationStyle::AllAround;
-        std::unordered_map<IGraphicsEffect *, double> graphicsEffects;
 };
 
 } // namespace libscratchcpp

src/scratch/stage.cpp

@@ -2,6 +2,7 @@
 
 #include <scratchcpp/stage.h>
 #include <scratchcpp/istagehandler.h>
+#include <scratchcpp/iengine.h>
 #include <cassert>
 
 #include "stage_p.h"
@@ -123,3 +124,29 @@ void Stage::setTextToSpeechLanguage(const std::string &newTextToSpeechLanguage)
 {
     impl->textToSpeechLanguage = newTextToSpeechLanguage;
 }
+
+/*! Overrides Target#setGraphicsEffectValue(). */
+void Stage::setGraphicsEffectValue(IGraphicsEffect *effect, double value)
+{
+    Target::setGraphicsEffectValue(effect, value);
+    IEngine *eng = engine();
+
+    if (eng)
+        eng->requestRedraw();
+
+    if (impl->iface)
+        impl->iface->onGraphicsEffectChanged(effect, value);
+}
+
+/*! Overrides Target#clearGraphicsEffects(). */
+void Stage::clearGraphicsEffects()
+{
+    Target::clearGraphicsEffects();
+    IEngine *eng = engine();
+
+    if (eng)
+        eng->requestRedraw();
+
+    if (impl->iface)
+        impl->iface->onGraphicsEffectsCleared();
+}

src/scratch/target.cpp

@@ -409,6 +409,29 @@ void Target::setVolume(double newVolume)
         impl->volume = newVolume;
 }
 
+/*! Returns the value of the given graphics effect. */
+double Target::graphicsEffectValue(IGraphicsEffect *effect) const
+{
+    auto it = impl->graphicsEffects.find(effect);
+
+    if (it == impl->graphicsEffects.cend())
+        return 0;
+    else
+        return it->second;
+}
+
+/*! Sets the value of the given graphics effect. */
+void Target::setGraphicsEffectValue(IGraphicsEffect *effect, double value)
+{
+    impl->graphicsEffects[effect] = value;
+}
+
+/*! Sets the value of all graphics effects to 0 (clears them). */
+void Target::clearGraphicsEffects()
+{
+    impl->graphicsEffects.clear();
+}
+
 /*! Returns the engine. */
 IEngine *Target::engine() const
 {