Cleanup|libcore: Simplify the Rule classes

There are thousands of rules in a UI widget tree, so avoid excessive memory allocations (no pimpl) and allow subclasses to use the base class flags member to store their own flags. Also allows inlining more of the common methods.
skyjake · Sep 1, 2019 · 89d5ff2 · 89d5ff2
1 parent 9f9d185
commit 89d5ff2
Show file tree

Hide file tree

Showing 9 changed files with 104 additions and 126 deletions.
diff --git a/doomsday/libs/core/include/de/widgets/animationrule.h b/doomsday/libs/core/include/de/widgets/animationrule.h
@@ -45,11 +45,11 @@ class DE_PUBLIC AnimationRule : public Rule, DE_OBSERVES(Clock, TimeChange)
      * @param transition  Transition time to reach the current or future target values.
      * @param style       Animation style.
      */
-    explicit AnimationRule(Rule const &target, TimeSpan transition, Animation::Style style = Animation::EaseOut);
+    explicit AnimationRule(const Rule &target, TimeSpan transition, Animation::Style style = Animation::EaseOut);
 
     void set(float target, TimeSpan transition = 0.0, TimeSpan delay = 0.0);
 
-    void set(Rule const &target, TimeSpan transition = 0.0, TimeSpan delay = 0.0);
+    void set(const Rule &target, TimeSpan transition = 0.0, TimeSpan delay = 0.0);
 
     /**
      * Sets the animation style of the rule.
@@ -61,9 +61,11 @@ class DE_PUBLIC AnimationRule : public Rule, DE_OBSERVES(Clock, TimeChange)
     void setStyle(Animation::Style style, float bounceSpring);
 
     enum Behavior {
-        Singleshot               = 0x1,
-        RestartWhenTargetChanges = 0x2,
-        DontAnimateFromZero      = 0x4,
+        Singleshot               = 0x10,
+        RestartWhenTargetChanges = 0x20,
+        DontAnimateFromZero      = 0x40,
+
+        FlagMask = 0xf0
     };
     using Behaviors = Flags;
 
@@ -94,24 +96,19 @@ class DE_PUBLIC AnimationRule : public Rule, DE_OBSERVES(Clock, TimeChange)
      */
     void shift(float delta);
 
-    void finish();
-
-    void pause();
-
-    void resume();
-
+    void   finish();
+    void   pause();
+    void   resume();
     String description() const;
 
 protected:
     ~AnimationRule();
     void update();
-
     void timeChanged(Clock const &);
 
 private:
     Animation _animation;
-    Rule const *_targetRule;
-    Behaviors _behavior;
+    const Rule *_targetRule;
 };
 
 } // namespace de

diff --git a/doomsday/libs/core/include/de/widgets/constantrule.h b/doomsday/libs/core/include/de/widgets/constantrule.h
@@ -49,7 +49,7 @@ class DE_PUBLIC ConstantRule : public Rule
          * value. Caller is responsible for making sure a reference is taken
          * to the returned rule.
          */
-        operator Rule const &() const;
+        operator const Rule &() const;
 
         operator RefArg<Rule>() const;
 
@@ -69,7 +69,7 @@ class DE_PUBLIC ConstantRule : public Rule
 
     String description() const;
 
-    static ConstantRule const &zero();
+    static const ConstantRule &zero();
 
 protected:
     void update();

diff --git a/doomsday/libs/core/include/de/widgets/indirectrule.h b/doomsday/libs/core/include/de/widgets/indirectrule.h
@@ -53,11 +53,11 @@ class DE_PUBLIC IndirectRule : public Rule
 
     void update();
 
-    Rule const &source() const;
+    const Rule &source() const;
     String description() const;
 
 private:
-    Rule const *_source;
+    const Rule *_source;
 };
 
 } // namespace de

diff --git a/doomsday/libs/core/include/de/widgets/operatorrule.h b/doomsday/libs/core/include/de/widgets/operatorrule.h
@@ -53,7 +53,7 @@ class DE_PUBLIC OperatorRule : public Rule
 
     OperatorRule(Operator op, Rule const &left, Rule const &right);
 
-    OperatorRule(Operator op, Rule const &left, Rule const &right, Rule const &condition);
+    OperatorRule(Operator op, Rule const &left, Rule const &right, Rule const &condition);       
 
 public:
     static OperatorRule &maximum(Rule const &left, Rule const &right) {
@@ -97,12 +97,16 @@ class DE_PUBLIC OperatorRule : public Rule
 protected:
     ~OperatorRule();
 
+    inline Operator op() const
+    {
+        return Operator((_flags >> BaseFlagsShift) & 0xf);
+    }
+
     void update();
 
     String description() const;
 
 private:
-    Operator _operator;
     Rule const *_leftOperand;
     Rule const *_rightOperand;
     Rule const *_condition;

diff --git a/doomsday/libs/core/include/de/widgets/rule.h b/doomsday/libs/core/include/de/widgets/rule.h
@@ -23,6 +23,7 @@
 #include "../libcore.h"
 #include "../Counted"
 #include "../Observers"
+#include "../PointerSet"
 #include "../String"
 
 namespace de {
@@ -60,10 +61,18 @@ class DE_PUBLIC Rule : public Counted, public DE_AUDIENCE_INTERFACE(RuleInvalida
         MAX_SEMANTICS
     };
 
+    enum { Valid = 0x1, BaseFlagsShift = 4 };
+
 public:
-    Rule();
+    Rule()
+        : _flags(0)
+        , _value(0)
+    {}
 
-    explicit Rule(float initialValue);
+    explicit Rule(float initialValue)
+        : _flags(Valid)
+        , _value(initialValue)
+    {}
 
     /**
      * Determines the rule's current value. If it has been marked invalid,
@@ -74,7 +83,7 @@ class DE_PUBLIC Rule : public Counted, public DE_AUDIENCE_INTERFACE(RuleInvalida
     /**
      * Determines the rule's current value (as integer). Otherwise same as value().
      */
-    int valuei() const;
+    inline int valuei() const { return de::floor(value()); }
 
     /**
      * Marks the rule invalid, causing all dependent rules to be invalid, too.
@@ -88,31 +97,38 @@ class DE_PUBLIC Rule : public Counted, public DE_AUDIENCE_INTERFACE(RuleInvalida
      *
      * This is called automatically when needed.
      */
-    virtual void update();
+    virtual void update()
+    {
+        // This is a fixed value, so don't do anything.
+        _flags |= Valid;
+    }
 
     /**
      * Determines if the rule's value is currently valid. A rule becomes
      * invalid if any of its dependencies are invalidated, or invalidate() is
      * called directly on the rule.
      */
-    bool isValid() const;
+    inline bool isValid() const
+    {
+        return (_flags & Valid) != 0;
+    }
 
     /**
      * Links rules together. This rule will depend on @a dependency; if @a
      * dependency becomes invalid, this rule will likewise become invalid.
      * @a dependency will hold a reference to this rule.
      */
-    void dependsOn(Rule const &dependency);
+    void dependsOn(const Rule &dependency);
 
-    void dependsOn(Rule const *dependencyOrNull);
+    void dependsOn(const Rule *dependencyOrNull);
 
     /**
      * Unlinks rules. This rule will no longer depend on @a dependency.
      * @a dependency will release its reference to this rule.
      */
-    void independentOf(Rule const &dependency);
+    void independentOf(const Rule &dependency);
 
-    void independentOf(Rule const *dependencyOrNull);
+    void independentOf(const Rule *dependencyOrNull);
 
     virtual String description() const = 0;
 
@@ -133,22 +149,36 @@ class DE_PUBLIC Rule : public Counted, public DE_AUDIENCE_INTERFACE(RuleInvalida
     static bool invalidRulesExist();
 
 protected:
-    ~Rule(); // Counted
+    ~Rule() override // not public due to being Counted
+    {
+        DE_ASSERT(_dependencies.isEmpty());
+    }
 
     /**
      * Sets the current value of the rule and marks it valid.
      *
      * @param value  New valid value.
      */
-    void setValue(float value);
+    inline void setValue(float value)
+    {
+        _value = value;
+        _flags |= Valid;
+    }
 
-    float cachedValue() const;
+    inline float cachedValue() const
+    {
+        return _value;
+    }
 
     // Implements IRuleInvalidationObserver.
     void ruleInvalidated();
 
+protected:
+    int _flags; // Derived rules use this, too.
+
 private:
-    DE_PRIVATE(d)
+    PointerSetT<Rule> _dependencies; // ref'd
+    float             _value;        // Current value of the rule.
 
     static bool _invalidRulesExist;
 };

diff --git a/doomsday/libs/core/src/widgets/animationrule.cpp b/doomsday/libs/core/src/widgets/animationrule.cpp
@@ -26,15 +26,16 @@ AnimationRule::AnimationRule(float initialValue, Animation::Style style)
     : Rule(initialValue)
     , _animation(initialValue, style)
     , _targetRule(nullptr)
-    , _behavior(Singleshot)
-{}
+{
+    _flags |= Singleshot;
+}
 
-AnimationRule::AnimationRule(Rule const &target, TimeSpan transition, Animation::Style style)
+AnimationRule::AnimationRule(const Rule &target, TimeSpan transition, Animation::Style style)
     : Rule(target.value())
     , _animation(target.value(), style)
     , _targetRule(nullptr)
-    , _behavior(RestartWhenTargetChanges | DontAnimateFromZero)
 {
+    _flags |= RestartWhenTargetChanges | DontAnimateFromZero;
     set(target, transition);
 }
 
@@ -52,7 +53,7 @@ void AnimationRule::set(float target, TimeSpan transition, TimeSpan delay)
     invalidate();
 }
 
-void AnimationRule::set(Rule const &target, TimeSpan transition, TimeSpan delay)
+void AnimationRule::set(const Rule &target, TimeSpan transition, TimeSpan delay)
 {
     set(target.value(), transition, delay);
 
@@ -73,12 +74,13 @@ void AnimationRule::setStyle(Animation::Style style, float bounceSpring)
 
 void AnimationRule::setBehavior(Behaviors behavior)
 {
-    _behavior = behavior;
+    _flags &= ~FlagMask;
+    _flags |= behavior;
 }
 
 AnimationRule::Behaviors AnimationRule::behavior() const
 {
-    return _behavior;
+    return Behaviors(_flags & FlagMask);
 }
 
 void AnimationRule::shift(float delta)
@@ -119,7 +121,7 @@ void AnimationRule::update()
     // When using a rule for the target, keep it updated.
     if (_targetRule)
     {
-        if (_behavior.testFlag(Singleshot) || !_animation.done())
+        if ((_flags & Singleshot) || !_animation.done())
         {
             _animation.adjustTarget(_targetRule->value());
         }
@@ -129,7 +131,7 @@ void AnimationRule::update()
             if (!fequal(_animation.target(), _targetRule->value()))
             {
                 TimeSpan span = _animation.transitionTime();
-                if (_behavior.testFlag(DontAnimateFromZero) && fequal(_animation.target(), 0))
+                if ((_flags & DontAnimateFromZero) && fequal(_animation.target(), 0))
                 {
                     span = 0.0;
                 }

diff --git a/doomsday/libs/core/src/widgets/indirectrule.cpp b/doomsday/libs/core/src/widgets/indirectrule.cpp
@@ -28,7 +28,7 @@ IndirectRule::~IndirectRule()
     independentOf(_source);
 }
 
-void IndirectRule::setSource(Rule const &rule)
+void IndirectRule::setSource(const Rule &rule)
 {
     unsetSource();
     dependsOn(_source = &rule);
@@ -52,7 +52,7 @@ void IndirectRule::update()
     setValue(_source ? _source->value() : 0.f);
 }
 
-Rule const &IndirectRule::source() const
+const Rule &IndirectRule::source() const
 {
     DE_ASSERT(_source);
     return *_source;