Browse Source

Decorators and their instancers now use smart pointers instead of manual reference counting

Michael Ragazzon 6 years ago
parent
commit
cc0b4a459a
33 changed files with 88 additions and 302 deletions
  1. 1 5
      Include/Rocket/Core/Decorator.h
  2. 3 13
      Include/Rocket/Core/DecoratorInstancer.h
  3. 0 14
      Include/Rocket/Core/Element.h
  4. 2 2
      Include/Rocket/Core/Factory.h
  5. 1 0
      Include/Rocket/Core/Property.h
  6. 3 3
      Include/Rocket/Core/StyleSheet.h
  7. 8 4
      Samples/assets/invader.rcss
  8. 4 18
      Samples/invaders/src/DecoratorInstancerDefender.cpp
  9. 1 7
      Samples/invaders/src/DecoratorInstancerDefender.h
  10. 3 17
      Samples/invaders/src/DecoratorInstancerStarfield.cpp
  11. 1 7
      Samples/invaders/src/DecoratorInstancerStarfield.h
  12. 2 7
      Samples/invaders/src/main.cpp
  13. 0 9
      Source/Core/Decorator.cpp
  14. 0 6
      Source/Core/DecoratorInstancer.cpp
  15. 2 14
      Source/Core/DecoratorNoneInstancer.cpp
  16. 1 7
      Source/Core/DecoratorNoneInstancer.h
  17. 3 17
      Source/Core/DecoratorTiledBoxInstancer.cpp
  18. 1 6
      Source/Core/DecoratorTiledBoxInstancer.h
  19. 3 5
      Source/Core/DecoratorTiledHorizontalInstancer.cpp
  20. 1 1
      Source/Core/DecoratorTiledHorizontalInstancer.h
  21. 3 20
      Source/Core/DecoratorTiledImageInstancer.cpp
  22. 1 7
      Source/Core/DecoratorTiledImageInstancer.h
  23. 2 7
      Source/Core/DecoratorTiledInstancer.cpp
  24. 3 17
      Source/Core/DecoratorTiledVerticalInstancer.cpp
  25. 1 6
      Source/Core/DecoratorTiledVerticalInstancer.h
  26. 0 6
      Source/Core/Element.cpp
  27. 4 26
      Source/Core/ElementDecoration.cpp
  28. 2 11
      Source/Core/ElementDecoration.h
  29. 8 1
      Source/Core/ElementDefinition.cpp
  30. 1 1
      Source/Core/ElementDefinition.h
  31. 17 23
      Source/Core/Factory.cpp
  32. 4 13
      Source/Core/StyleSheet.cpp
  33. 2 2
      Source/Core/StyleSheetParser.cpp

+ 1 - 5
Include/Rocket/Core/Decorator.h

@@ -28,7 +28,6 @@
 #ifndef ROCKETCOREDECORATOR_H
 #define ROCKETCOREDECORATOR_H
 
-#include "ReferenceCountable.h"
 #include <vector>
 #include "Header.h"
 #include "Texture.h"
@@ -50,7 +49,7 @@ class TextureResource;
 	@author Peter Curry
  */
 
-class ROCKETCORE_API Decorator : public ReferenceCountable
+class ROCKETCORE_API Decorator
 {
 public:
 	Decorator();
@@ -89,9 +88,6 @@ public:
 	static const DecoratorDataHandle INVALID_DECORATORDATAHANDLE = 0;
 
 protected:
-	/// Releases the decorator through its instancer.
-	virtual void OnReferenceDeactivate() override;
-
 	/// Attempts to load a texture into the list of textures in use by the decorator.
 	/// @param[in] texture_name The name of the texture to load.
 	/// @param[in] rcss_path The RCSS file the decorator definition was loaded from; this is used to resolve relative paths.

+ 3 - 13
Include/Rocket/Core/DecoratorInstancer.h

@@ -28,7 +28,6 @@
 #ifndef ROCKETCOREDECORATORINSTANCER_H
 #define ROCKETCOREDECORATORINSTANCER_H
 
-#include "ReferenceCountable.h"
 #include "Header.h"
 #include "PropertyDictionary.h"
 #include "PropertySpecification.h"
@@ -47,23 +46,17 @@ class Decorator;
 	@author Peter Curry
  */
 
-class ROCKETCORE_API DecoratorInstancer : public ReferenceCountable
+class ROCKETCORE_API DecoratorInstancer
 {
 public:
 	DecoratorInstancer();
 	virtual ~DecoratorInstancer();
 
 	/// Instances a decorator given the property tag and attributes from the RCSS file.
-	/// @param[in] name The type of decorator desired. For example, "background-decorator: simple;" is declared as type "simple".
+	/// @param[in] name The type of decorator desired. For example, "decorator: simple(...);" is declared as type "simple".
 	/// @param[in] properties All RCSS properties associated with the decorator.
 	/// @return The decorator if it was instanced successfully, NULL if an error occured.
-	virtual Decorator* InstanceDecorator(const String& name, const PropertyDictionary& properties, const StyleSheet& style_sheet) = 0;
-	/// Releases the given decorator.
-	/// @param[in] decorator Decorator to release. This is guaranteed to have been constructed by this instancer.
-	virtual void ReleaseDecorator(Decorator* decorator) = 0;
-
-	/// Releases the instancer.
-	virtual void Release() = 0;
+	virtual std::shared_ptr<Decorator> InstanceDecorator(const String& name, const PropertyDictionary& properties, const StyleSheet& style_sheet) = 0;
 
 	/// Returns the property specification associated with the instancer.
 	const PropertySpecification& GetPropertySpecification() const;
@@ -81,9 +74,6 @@ protected:
 	/// @param True if all the property names exist, false otherwise.
 	ShorthandId RegisterShorthand(const String& shorthand_name, const String& property_names, ShorthandType type);
 
-	// Releases the instancer.
-	virtual void OnReferenceDeactivate();
-
 private:
 	PropertySpecification properties;
 };

+ 0 - 14
Include/Rocket/Core/Element.h

@@ -323,20 +323,6 @@ public:
 	int GetNumAttributes() const;
 	//@}
 
-	/** @name Decorators
-	 */
-	//@{
-	/// Iterates over all decorators attached to the element. Note that all decorators are iterated
-	/// over, not just active ones.
-	/// @param[inout] index Index to fetch. This is incremented after the fetch.
-	/// @param[out] pseudo_classes The pseudo-classes the decorator required to be active before it renders.
-	/// @param[out] name The name of the decorator at the specified index.
-	/// @param[out] decorator The decorator at the specified index.
-	/// @param[out] decorator_data This element's handle to any data is has stored against the decorator.
-	/// @return True if a decorator was successfully fetched, false if not.
-	bool IterateDecorators(int& index, PseudoClassList& pseudo_classes, String& name, Decorator*& decorator, DecoratorDataHandle& decorator_data);
-	//@}
-
 	/// Gets the outer-most focus element down the tree from this node.
 	/// @return Outer-most focus element.
 	Element* GetFocusLeafNode();

+ 2 - 2
Include/Rocket/Core/Factory.h

@@ -115,7 +115,7 @@ public:
 	/// @param[in] name The name of the decorator the instancer will be called for.
 	/// @param[in] instancer The instancer to call when the decorator name is encountered.
 	/// @return The added instancer if the registration was successful, NULL otherwise.
-	static DecoratorInstancer* RegisterDecoratorInstancer(const String& name, DecoratorInstancer* instancer);
+	static void RegisterDecoratorInstancer(const String& name, std::unique_ptr<DecoratorInstancer> instancer);
 	/// Retrieves the property specification of a decorator registered with the factory.
 	/// @param[in] name The name of the decorator.
 	/// @return The property specification if the decorator exists, NULL otherwise.
@@ -124,7 +124,7 @@ public:
 	/// @param[in] name The name of the desired decorator type.
 	/// @param[in] properties The properties associated with the decorator.
 	/// @return The newly instanced decorator, or NULL if the decorator could not be instanced.
-	static Decorator* InstanceDecorator(const String& name, const PropertyDictionary& properties, const StyleSheet& style_sheet);
+	static std::shared_ptr<Decorator> InstanceDecorator(const String& name, const PropertyDictionary& properties, const StyleSheet& style_sheet);
 
 	/// Registers an instancer that will be used to instance font effects.
 	/// @param[in] name The name of the font effect the instancer will be called for.

+ 1 - 0
Include/Rocket/Core/Property.h

@@ -77,6 +77,7 @@ public:
 		TRANSFORM = 1 << 17,			// transform; fetch as < TransformRef >, may be empty
 		TRANSITION = 1 << 18,           // transition; fetch as < TransitionList >
 		ANIMATION = 1 << 19,            // animation; fetch as < AnimationList >
+		DECORATOR = 1 << 20,            // decorator; fetch as < DecoratorList >
 
 		LENGTH = PX | DP | PPI_UNIT | EM | REM,
 		LENGTH_PERCENT = LENGTH | PERCENT,

+ 3 - 3
Include/Rocket/Core/StyleSheet.h

@@ -56,7 +56,7 @@ typedef UnorderedMap<String, Keyframes> KeyframesMap;
 struct DecoratorSpecification {
 	String decorator_type;
 	PropertyDictionary properties;
-	Decorator* decorator = nullptr;
+	std::shared_ptr<Decorator> decorator;
 };
 
 struct Sprite {
@@ -208,11 +208,11 @@ public:
 	Keyframes* GetKeyframes(const String& name);
 
 	/// Returns the Decorator of the given name, or null if it does not exist.
-	Decorator* GetDecorator(const String& name) const;
+	std::shared_ptr<Decorator> GetDecorator(const String& name) const;
 
 	const Sprite* GetSprite(const String& name) const;
 
-	Decorator* GetOrInstanceDecorator(const String& decorator_value, const String& source_file, int source_line_number);
+	std::shared_ptr<Decorator> GetOrInstanceDecorator(const String& decorator_value, const String& source_file, int source_line_number);
 
 	/// Returns the compiled element definition for a given element hierarchy. A reference count will be added for the
 	/// caller, so another should not be added. The definition should be released by removing the reference count.

+ 8 - 4
Samples/assets/invader.rcss

@@ -61,6 +61,10 @@ div#title_bar div#icon
 	window-bl: 0px 140px 11px 11px;
 	window-b:  11px 140px 1px 11px;
 	window-br: 136px 140px 10px 11px;
+	
+	button: 247px 0px 159px 45px;
+	button-hover:  247px 45px 159px 45px;
+	button-active: 247px 90px 159px 45px;
 }
 
 @decorator title_bar : tiled-horizontal {
@@ -182,7 +186,7 @@ input.submit
 	text-align: center;
 	tab-index: auto;
 
-	decorator: image(invader.tga 247px 0px 406px 45px);
+	decorator: image(button);
 }
 
 button:focus,
@@ -198,18 +202,18 @@ input.submit:focus
 button:hover,
 input.submit:hover
 {
-	decorator: image(invader.tga 247px 45px 406px 90px);
+	decorator: image(button-hover);
 }
 
 button:active,
 input.submit:active
 {
-	decorator: image(invader.tga 247px 90px 406px 135px);
+	decorator: image(button-active);
 }
 
 input.submit:disabled
 {
-	decorator: image(invader.tga 247px 0px 406px 45px);
+	decorator: image(button);
 	image-color: rgba(50, 150, 150, 120);
 	cursor: unavailable;
 }

+ 4 - 18
Samples/invaders/src/DecoratorInstancerDefender.cpp

@@ -41,30 +41,16 @@ DecoratorInstancerDefender::~DecoratorInstancerDefender()
 }
 
 // Instances a decorator given the property tag and attributes from the RCSS file.
-Rocket::Core::Decorator* DecoratorInstancerDefender::InstanceDecorator(const Rocket::Core::String& ROCKET_UNUSED_PARAMETER(name), const Rocket::Core::PropertyDictionary& properties, const Rocket::Core::StyleSheet& style_sheet)
+std::shared_ptr<Rocket::Core::Decorator> DecoratorInstancerDefender::InstanceDecorator(const Rocket::Core::String& ROCKET_UNUSED_PARAMETER(name), const Rocket::Core::PropertyDictionary& properties, const Rocket::Core::StyleSheet& style_sheet)
 {
 	ROCKET_UNUSED(name);
 
 	const Rocket::Core::Property* image_source_property = properties.GetProperty(id_image_src);
 	Rocket::Core::String image_source = image_source_property->Get< Rocket::Core::String >();
 
-	DecoratorDefender* decorator = new DecoratorDefender();
+	auto decorator = std::make_shared<DecoratorDefender>();
 	if (decorator->Initialise(image_source, image_source_property->source))
 		return decorator;
-
-	decorator->RemoveReference();
-	ReleaseDecorator(decorator);
-	return NULL;
-}
-
-// Releases the given decorator.
-void DecoratorInstancerDefender::ReleaseDecorator(Rocket::Core::Decorator* decorator)
-{
-	delete decorator;
-}
-
-// Releases the instancer.
-void DecoratorInstancerDefender::Release()
-{
-	delete this;
+	
+	return nullptr;
 }

+ 1 - 7
Samples/invaders/src/DecoratorInstancerDefender.h

@@ -44,13 +44,7 @@ public:
 	/// @param[in] name The type of decorator desired. For example, "background-decorator: simple;" is declared as type "simple".
 	/// @param[in] properties All RCSS properties associated with the decorator.
 	/// @return The decorator if it was instanced successful, NULL if an error occured.
-	Rocket::Core::Decorator* InstanceDecorator(const Rocket::Core::String& name, const Rocket::Core::PropertyDictionary& properties, const Rocket::Core::StyleSheet& style_sheet) override;
-	/// Releases the given decorator.
-	/// @param[in] decorator Decorator to release. This is guaranteed to have been constructed by this instancer.
-	void ReleaseDecorator(Rocket::Core::Decorator* decorator) override;
-
-	/// Releases the instancer.
-	void Release() override;
+	std::shared_ptr<Rocket::Core::Decorator> InstanceDecorator(const Rocket::Core::String& name, const Rocket::Core::PropertyDictionary& properties, const Rocket::Core::StyleSheet& style_sheet) override;
 
 private:
 	Rocket::Core::PropertyId id_image_src;

+ 3 - 17
Samples/invaders/src/DecoratorInstancerStarfield.cpp

@@ -45,7 +45,7 @@ DecoratorInstancerStarfield::~DecoratorInstancerStarfield()
 }
 
 // Instances a decorator given the property tag and attributes from the RCSS file.
-Rocket::Core::Decorator* DecoratorInstancerStarfield::InstanceDecorator(const Rocket::Core::String& ROCKET_UNUSED_PARAMETER(name), const Rocket::Core::PropertyDictionary& properties, const Rocket::Core::StyleSheet& style_sheet)
+std::shared_ptr<Rocket::Core::Decorator> DecoratorInstancerStarfield::InstanceDecorator(const Rocket::Core::String& ROCKET_UNUSED_PARAMETER(name), const Rocket::Core::PropertyDictionary& properties, const Rocket::Core::StyleSheet& style_sheet)
 {
 	ROCKET_UNUSED(name);
 
@@ -57,23 +57,9 @@ Rocket::Core::Decorator* DecoratorInstancerStarfield::InstanceDecorator(const Ro
 	int top_density = Rocket::Core::Math::RealToInteger(properties.GetProperty(id_top_density)->Get< float >());
 	int bottom_density = Rocket::Core::Math::RealToInteger(properties.GetProperty(id_bottom_density)->Get< float >());
 
-	DecoratorStarfield* decorator = new DecoratorStarfield();
+	auto decorator = std::make_shared<DecoratorStarfield>();
 	if (decorator->Initialise(num_layers, top_colour, bottom_colour, top_speed, bottom_speed, top_density, bottom_density))
 		return decorator;
 
-	decorator->RemoveReference();
-	ReleaseDecorator(decorator);
-	return NULL;
-}
-
-// Releases the given decorator.
-void DecoratorInstancerStarfield::ReleaseDecorator(Rocket::Core::Decorator* decorator)
-{
-	delete decorator;
-}
-
-// Releases the instancer.
-void DecoratorInstancerStarfield::Release()
-{
-	delete this;
+	return nullptr;
 }

+ 1 - 7
Samples/invaders/src/DecoratorInstancerStarfield.h

@@ -45,13 +45,7 @@ public:
 	/// @param name The type of decorator desired. For example, "background-decorator: simple;" is declared as type "simple".
 	/// @param properties All RCSS properties associated with the decorator.
 	/// @return The decorator if it was instanced successful, NULL if an error occured.
-	Rocket::Core::Decorator* InstanceDecorator(const Rocket::Core::String& name, const Rocket::Core::PropertyDictionary& properties, const Rocket::Core::StyleSheet& style_sheet) override;
-	/// Releases the given decorator.
-	/// @param decorator Decorator to release. This is guaranteed to have been constructed by this instancer.
-	void ReleaseDecorator(Rocket::Core::Decorator* decorator) override;
-
-	/// Releases the instancer.
-	void Release() override;
+	std::shared_ptr<Rocket::Core::Decorator> InstanceDecorator(const Rocket::Core::String& name, const Rocket::Core::PropertyDictionary& properties, const Rocket::Core::StyleSheet& style_sheet) override;
 
 private:
 	Rocket::Core::PropertyId id_num_layers, id_top_colour, id_bottom_colour, id_top_speed, id_bottom_speed, id_top_density, id_bottom_density;

+ 2 - 7
Samples/invaders/src/main.cpp

@@ -129,13 +129,8 @@ int main(int ROCKET_UNUSED_PARAMETER(argc), char** ROCKET_UNUSED_PARAMETER(argv)
 	Rocket::Core::Factory::RegisterElementInstancer("game", element_instancer);
 	element_instancer->RemoveReference();
 
-	Rocket::Core::DecoratorInstancer* decorator_instancer = new DecoratorInstancerStarfield();
-	Rocket::Core::Factory::RegisterDecoratorInstancer("starfield", decorator_instancer);
-	decorator_instancer->RemoveReference();
-
-	decorator_instancer = new DecoratorInstancerDefender();
-	Rocket::Core::Factory::RegisterDecoratorInstancer("defender", decorator_instancer);
-	decorator_instancer->RemoveReference();
+	Rocket::Core::Factory::RegisterDecoratorInstancer("starfield", std::make_unique<DecoratorInstancerStarfield>());
+	Rocket::Core::Factory::RegisterDecoratorInstancer("defender", std::make_unique<DecoratorInstancerDefender>());
 
 	// Register Invader's data formatters
 	HighScoresNameFormatter name_formatter;

+ 0 - 9
Source/Core/Decorator.cpp

@@ -29,7 +29,6 @@
 #include "../../Include/Rocket/Core/Decorator.h"
 #include "TextureDatabase.h"
 #include "TextureResource.h"
-#include "../../Include/Rocket/Core/DecoratorInstancer.h"
 #include "../../Include/Rocket/Core/PropertyDefinition.h"
 
 namespace Rocket {
@@ -37,7 +36,6 @@ namespace Core {
 
 Decorator::Decorator()
 {
-	instancer = nullptr;
 	z_index = 0;
 	specificity = -1;
 }
@@ -70,13 +68,6 @@ int Decorator::GetSpecificity() const
 	return specificity;
 }
 
-// Releases the decorator through its instancer.
-void Decorator::OnReferenceDeactivate()
-{
-	if (instancer)
-		instancer->ReleaseDecorator(this);
-}
-
 // Attempts to load a texture into the list of textures in use by the decorator.
 int Decorator::LoadTexture(const String& texture_name, const String& rcss_path)
 {

+ 0 - 6
Source/Core/DecoratorInstancer.cpp

@@ -57,11 +57,5 @@ ShorthandId DecoratorInstancer::RegisterShorthand(const String& shorthand_name,
 	return properties.RegisterShorthand(shorthand_name, property_names, type);
 }
 
-// Releases the instancer.
-void DecoratorInstancer::OnReferenceDeactivate()
-{
-	Release();
-}
-
 }
 }

+ 2 - 14
Source/Core/DecoratorNoneInstancer.cpp

@@ -37,25 +37,13 @@ DecoratorNoneInstancer::~DecoratorNoneInstancer()
 }
 
 // Instances a decorator given the property tag and attributes from the RCSS file.
-Decorator* DecoratorNoneInstancer::InstanceDecorator(const String& ROCKET_UNUSED_PARAMETER(name), const PropertyDictionary& ROCKET_UNUSED_PARAMETER(properties), const StyleSheet& ROCKET_UNUSED_PARAMETER(style_sheet))
+std::shared_ptr<Decorator> DecoratorNoneInstancer::InstanceDecorator(const String& ROCKET_UNUSED_PARAMETER(name), const PropertyDictionary& ROCKET_UNUSED_PARAMETER(properties), const StyleSheet& ROCKET_UNUSED_PARAMETER(style_sheet))
 {
 	ROCKET_UNUSED(name);
 	ROCKET_UNUSED(properties);
 	ROCKET_UNUSED(style_sheet);
 
-	return new DecoratorNone();
-}
-
-// Releases the given decorator.
-void DecoratorNoneInstancer::ReleaseDecorator(Decorator* decorator)
-{
-	delete decorator;
-}
-
-// Releases the instancer.
-void DecoratorNoneInstancer::Release()
-{
-	delete this;
+	return std::make_shared<DecoratorNone>();
 }
 
 }

+ 1 - 7
Source/Core/DecoratorNoneInstancer.h

@@ -48,13 +48,7 @@ public:
 	/// @param name The type of decorator desired. For example, "background-decorator: simple;" is declared as type "simple".
 	/// @param properties All RCSS properties associated with the decorator.
 	/// @return The decorator if it was instanced successful, NULL if an error occured.
-	Decorator* InstanceDecorator(const String& name, const PropertyDictionary& properties, const StyleSheet& style_sheet) override;
-	/// Releases the given decorator.
-	/// @param decorator Decorator to release. This is guaranteed to have been constructed by this instancer.
-	void ReleaseDecorator(Decorator* decorator) override;
-
-	/// Releases the instancer.
-	void Release() override;
+	std::shared_ptr<Decorator> InstanceDecorator(const String& name, const PropertyDictionary& properties, const StyleSheet& style_sheet) override;
 };
 
 }

+ 3 - 17
Source/Core/DecoratorTiledBoxInstancer.cpp

@@ -54,7 +54,7 @@ DecoratorTiledBoxInstancer::~DecoratorTiledBoxInstancer()
 }
 
 // Instances a box decorator.
-Decorator* DecoratorTiledBoxInstancer::InstanceDecorator(const String& ROCKET_UNUSED_PARAMETER(name), const PropertyDictionary& properties, const StyleSheet& style_sheet)
+std::shared_ptr<Decorator>DecoratorTiledBoxInstancer::InstanceDecorator(const String& ROCKET_UNUSED_PARAMETER(name), const PropertyDictionary& properties, const StyleSheet& style_sheet)
 {
 	ROCKET_UNUSED(name);
 
@@ -67,25 +67,11 @@ Decorator* DecoratorTiledBoxInstancer::InstanceDecorator(const String& ROCKET_UN
 	for(size_t i = 0; i < num_tiles; i++)
 		GetTileProperties(i, tiles[i], texture_names[i], rcss_paths[i], properties, style_sheet);
 
-	DecoratorTiledBox* decorator = new DecoratorTiledBox();
+	auto decorator = std::make_shared<DecoratorTiledBox>();
 	if (decorator->Initialise(tiles, texture_names, rcss_paths))
 		return decorator;
 
-	decorator->RemoveReference();
-	ReleaseDecorator(decorator);
-	return NULL;
-}
-
-// Releases the given decorator.
-void DecoratorTiledBoxInstancer::ReleaseDecorator(Decorator* decorator)
-{
-	delete decorator;
-}
-
-// Releases the instancer.
-void DecoratorTiledBoxInstancer::Release()
-{
-	delete this;
+	return nullptr;
 }
 
 }

+ 1 - 6
Source/Core/DecoratorTiledBoxInstancer.h

@@ -44,12 +44,7 @@ public:
 	~DecoratorTiledBoxInstancer();
 
 	/// Instances a box decorator.
-	Decorator* InstanceDecorator(const String& name, const PropertyDictionary& properties, const StyleSheet& style_sheet) override;
-	/// Releases the given decorator.
-	void ReleaseDecorator(Decorator* decorator) override;
-
-	/// Releases the instancer.
-	void Release() override;
+	std::shared_ptr<Decorator> InstanceDecorator(const String& name, const PropertyDictionary& properties, const StyleSheet& style_sheet) override;
 };
 
 }

+ 3 - 5
Source/Core/DecoratorTiledHorizontalInstancer.cpp

@@ -45,7 +45,7 @@ DecoratorTiledHorizontalInstancer::~DecoratorTiledHorizontalInstancer()
 }
 
 // Instances a box decorator.
-Decorator* DecoratorTiledHorizontalInstancer::InstanceDecorator(const String& ROCKET_UNUSED_PARAMETER(name), const PropertyDictionary& properties, const StyleSheet& style_sheet)
+std::shared_ptr<Decorator> DecoratorTiledHorizontalInstancer::InstanceDecorator(const String& ROCKET_UNUSED_PARAMETER(name), const PropertyDictionary& properties, const StyleSheet& style_sheet)
 {
 	ROCKET_UNUSED(name);
 
@@ -58,13 +58,11 @@ Decorator* DecoratorTiledHorizontalInstancer::InstanceDecorator(const String& RO
 	for (size_t i = 0; i < num_tiles; i++)
 		GetTileProperties(i, tiles[i], texture_names[i], rcss_paths[i], properties, style_sheet);
 
-	DecoratorTiledHorizontal* decorator = new DecoratorTiledHorizontal();
+	auto decorator = std::make_shared<DecoratorTiledHorizontal>();
 	if (decorator->Initialise(tiles, texture_names, rcss_paths))
 		return decorator;
 
-	decorator->RemoveReference();
-	ReleaseDecorator(decorator);
-	return NULL;
+	return nullptr;
 }
 
 // Releases the given decorator.

+ 1 - 1
Source/Core/DecoratorTiledHorizontalInstancer.h

@@ -44,7 +44,7 @@ public:
 	~DecoratorTiledHorizontalInstancer();
 
 	/// Instances a horizontal decorator.
-	Decorator* InstanceDecorator(const String& name, const PropertyDictionary& properties, const StyleSheet& style_sheet);
+	std::shared_ptr<Decorator> InstanceDecorator(const String& name, const PropertyDictionary& properties, const StyleSheet& style_sheet);
 	/// Releases the given decorator.
 	void ReleaseDecorator(Decorator* decorator);
 

+ 3 - 20
Source/Core/DecoratorTiledImageInstancer.cpp

@@ -43,7 +43,7 @@ DecoratorTiledImageInstancer::~DecoratorTiledImageInstancer()
 }
 
 
-Decorator* DecoratorTiledImageInstancer::InstanceDecorator(const String& ROCKET_UNUSED_PARAMETER(name), const PropertyDictionary& properties, const StyleSheet& style_sheet)
+std::shared_ptr<Decorator> DecoratorTiledImageInstancer::InstanceDecorator(const String& ROCKET_UNUSED_PARAMETER(name), const PropertyDictionary& properties, const StyleSheet& style_sheet)
 {
 	ROCKET_UNUSED(name);
 
@@ -53,29 +53,12 @@ Decorator* DecoratorTiledImageInstancer::InstanceDecorator(const String& ROCKET_
 
 	GetTileProperties(0, tile, texture_name, rcss_path, properties, style_sheet);
 	
-	if (texture_name == "window-c" || (texture_name == "invader.tga" && tile.texcoords[0].x == 11))
-	{
-		int i = 0;
-	}
-
-	DecoratorTiledImage* decorator = new DecoratorTiledImage();
+	auto decorator = std::make_shared<DecoratorTiledImage>();
 
 	if (decorator->Initialise(tile, texture_name, rcss_path))
 		return decorator;
 
-	decorator->RemoveReference();
-	ReleaseDecorator(decorator);
-	return NULL;
-}
-
-void DecoratorTiledImageInstancer::ReleaseDecorator(Decorator* decorator)
-{
-	delete decorator;
-}
-
-void DecoratorTiledImageInstancer::Release()
-{
-	delete this;
+	return nullptr;
 }
 
 }

+ 1 - 7
Source/Core/DecoratorTiledImageInstancer.h

@@ -44,13 +44,7 @@ public:
 	~DecoratorTiledImageInstancer();
 
 	/// Instances an image decorator.
-	Decorator* InstanceDecorator(const String& name, const PropertyDictionary& properties, const StyleSheet& style_sheet) override;
-
-	/// Releases the given decorator.
-	void ReleaseDecorator(Decorator* decorator) override;
-
-	/// Releases the instancer.
-	void Release() override;
+	std::shared_ptr<Decorator> InstanceDecorator(const String& name, const PropertyDictionary& properties, const StyleSheet& style_sheet) override;
 };
 
 }

+ 2 - 7
Source/Core/DecoratorTiledInstancer.cpp

@@ -73,19 +73,14 @@ void DecoratorTiledInstancer::GetTileProperties(size_t tile_index, DecoratorTile
 	const Property* texture_property = properties.GetProperty(ids.src);
 	texture_name = texture_property->Get< String >();
 	rcss_path = texture_property->source;
-
-	if (texture_name == "window-c")
-	{
-		int i = 0;
-	}
-
+	
 	// Declaring the name 'auto' is the same as an empty string. This gives an easy way to skip certain
 	// tiles in a shorthand since we can't always declare an empty string.
 	static const String none_texture_name = "auto";
 	if (texture_name == none_texture_name)
 		texture_name.clear();
 
-	// @performance / @todo: We want some way to determine sprite or image instead of always do the lookup up as a sprite name.
+	// @performance / @todo: We want some way to determine sprite or image instead of always doing the lookup as a sprite name.
 	// @performance: We already have the texture loaded in the spritesheet, very unnecessary to return as name and then convert to texture again.
 	if (const Sprite * sprite = style_sheet.GetSprite(texture_name))
 	{

+ 3 - 17
Source/Core/DecoratorTiledVerticalInstancer.cpp

@@ -45,7 +45,7 @@ DecoratorTiledVerticalInstancer::~DecoratorTiledVerticalInstancer()
 }
 
 // Instances a box decorator.
-Decorator* DecoratorTiledVerticalInstancer::InstanceDecorator(const String& ROCKET_UNUSED_PARAMETER(name), const PropertyDictionary& properties, const StyleSheet& style_sheet)
+std::shared_ptr<Decorator> DecoratorTiledVerticalInstancer::InstanceDecorator(const String& ROCKET_UNUSED_PARAMETER(name), const PropertyDictionary& properties, const StyleSheet& style_sheet)
 {
 	ROCKET_UNUSED(name);
 
@@ -58,25 +58,11 @@ Decorator* DecoratorTiledVerticalInstancer::InstanceDecorator(const String& ROCK
 	for (size_t i = 0; i < num_tiles; i++)
 		GetTileProperties(i, tiles[i], texture_names[i], rcss_paths[i], properties, style_sheet);
 
-	DecoratorTiledVertical* decorator = new DecoratorTiledVertical();
+	auto decorator = std::make_shared<DecoratorTiledVertical>();
 	if (decorator->Initialise(tiles, texture_names, rcss_paths))
 		return decorator;
 
-	decorator->RemoveReference();
-	ReleaseDecorator(decorator);
-	return NULL;
-}
-
-// Releases the given decorator.
-void DecoratorTiledVerticalInstancer::ReleaseDecorator(Decorator* decorator)
-{
-	delete decorator;
-}
-
-// Releases the instancer.
-void DecoratorTiledVerticalInstancer::Release()
-{
-	delete this;
+	return nullptr;
 }
 
 }

+ 1 - 6
Source/Core/DecoratorTiledVerticalInstancer.h

@@ -44,12 +44,7 @@ public:
 	~DecoratorTiledVerticalInstancer();
 
 	/// Instances a vertical decorator.
-	Decorator* InstanceDecorator(const String& name, const PropertyDictionary& properties, const StyleSheet& style_sheet) override;
-	/// Releases the given decorator.
-	void ReleaseDecorator(Decorator* decorator) override;
-
-	/// Releases the instancer.
-	void Release() override;
+	std::shared_ptr<Decorator> InstanceDecorator(const String& name, const PropertyDictionary& properties, const StyleSheet& style_sheet) override;
 };
 
 }

+ 0 - 6
Source/Core/Element.cpp

@@ -1006,12 +1006,6 @@ int Element::GetNumAttributes() const
 	return (int)attributes.size();
 }
 
-// Iterates over all decorators attached to the element.
-bool Element::IterateDecorators(int& index, PseudoClassList& pseudo_classes, String& name, Decorator*& decorator, DecoratorDataHandle& decorator_data)
-{
-	return decoration->IterateDecorators(index, pseudo_classes, name, decorator, decorator_data);
-}
-
 // Gets the name of the element.
 const String& Element::GetTagName() const
 {

+ 4 - 26
Source/Core/ElementDecoration.cpp

@@ -67,22 +67,21 @@ bool ElementDecoration::ReloadDecorators()
 
 	for (const String& name : decorator_list)
 	{
-		Decorator* decorator = stylesheet->GetOrInstanceDecorator(name, property->source, property->source_line_number);
+		std::shared_ptr<Decorator> decorator = stylesheet->GetOrInstanceDecorator(name, property->source, property->source_line_number);
 
 		if (decorator)
-			LoadDecorator(decorator);
+			LoadDecorator(std::move(decorator));
 	}
 
 	return true;
 }
 
 // Loads a single decorator and adds it to the list of loaded decorators for this element.
-int ElementDecoration::LoadDecorator(Decorator* decorator)
+int ElementDecoration::LoadDecorator(std::shared_ptr<Decorator> decorator)
 {
 	DecoratorHandle element_decorator;
-	element_decorator.decorator = decorator;
-	element_decorator.decorator->AddReference();
 	element_decorator.decorator_data = decorator->GenerateElementData(element);
+	element_decorator.decorator = std::move(decorator);
 
 	decorators.push_back(element_decorator);
 	return (int) (decorators.size() - 1);
@@ -95,8 +94,6 @@ void ElementDecoration::ReleaseDecorators()
 	{
 		if (decorators[i].decorator_data)
 			decorators[i].decorator->ReleaseElementData(decorators[i].decorator_data);
-
-		decorators[i].decorator->RemoveReference();
 	}
 
 	decorators.clear();
@@ -126,24 +123,5 @@ void ElementDecoration::DirtyDecorators()
 	decorators_dirty = true;
 }
 
-// Iterates over all active decorators attached to the decoration's element.
-bool ElementDecoration::IterateDecorators(int& index, PseudoClassList& pseudo_classes, String& name, Decorator*& decorator, DecoratorDataHandle& decorator_data) const
-{
-	if (index < 0)
-		return false;
-
-	if (index < (int)decorators.size())
-	{
-		decorator = decorators[index].decorator;
-		decorator_data = decorators[index].decorator_data;
-		name = ":not implemented:";
-
-		index += 1;
-		return true;
-	}
-
-	return false;
-}
-
 }
 }

+ 2 - 11
Source/Core/ElementDecoration.h

@@ -56,18 +56,9 @@ public:
 	/// Mark decorators as dirty and force them to reset themselves.
 	void DirtyDecorators();
 
-	/// Iterates over all active decorators attached to the decoration's element.
-	/// @param[inout] index Index to fetch. This is incremented after the fetch.
-	/// @param[out] pseudo_classes The pseudo-classes the decorator required to be active before it renders.
-	/// @param[out] name The name of the decorator at the specified index.
-	/// @param[out] decorator The decorator at the specified index.
-	/// @param[out] decorator_data This element's handle to any data is has stored against the decorator.
-	/// @return True if a decorator was successfully fetched, false if not.
-	bool IterateDecorators(int& index, PseudoClassList& pseudo_classes, String& name, Decorator*& decorator, DecoratorDataHandle& decorator_data) const;
-
 private:
 	// Loads a single decorator and adds it to the list of loaded decorators for this element.
-	int LoadDecorator(Decorator* decorator);
+	int LoadDecorator(std::shared_ptr<Decorator> decorator);
 	// Releases existing decorators and loads all decorators required by the element's definition.
 	bool ReloadDecorators();
 	// Releases all existing decorators and frees their data.
@@ -75,7 +66,7 @@ private:
 
 	struct DecoratorHandle
 	{
-		Decorator* decorator;
+		std::shared_ptr<Decorator> decorator;
 		DecoratorDataHandle decorator_data;
 	};
 

+ 8 - 1
Source/Core/ElementDefinition.cpp

@@ -44,7 +44,7 @@ ElementDefinition::~ElementDefinition()
 }
 
 // Initialises the element definition from a list of style sheet nodes.
-void ElementDefinition::Initialise(const std::vector< const StyleSheetNode* >& style_sheet_nodes, const PseudoClassList& volatile_pseudo_classes, bool _structurally_volatile)
+void ElementDefinition::Initialise(const std::vector< const StyleSheetNode* >& style_sheet_nodes, const PseudoClassList& volatile_pseudo_classes, bool _structurally_volatile, const StyleSheet& style_sheet)
 {
 	// Set the volatile structure flag.
 	structurally_volatile = _structurally_volatile;
@@ -105,6 +105,13 @@ void ElementDefinition::Initialise(const std::vector< const StyleSheetNode* >& s
 			}
 		}
 	}
+
+	// Turn the decorator properties from String to DecoratorList.
+	// This is essentially an optimization, we could do this just as well in e.g. ComputeValues() or ElementDecoration, but when we do it here, we only need to do it once.
+	// Note, since the user may set a new decorator through its style, we we need to do it again in ComputeValues.
+
+
+
 }
 
 // Returns a specific property from the element definition's base properties.

+ 1 - 1
Source/Core/ElementDefinition.h

@@ -55,7 +55,7 @@ public:
 	virtual ~ElementDefinition();
 
 	/// Initialises the element definition from a list of style sheet nodes.
-	void Initialise(const std::vector< const StyleSheetNode* >& style_sheet_nodes, const PseudoClassList& volatile_pseudo_classes, bool structurally_volatile);
+	void Initialise(const std::vector< const StyleSheetNode* >& style_sheet_nodes, const PseudoClassList& volatile_pseudo_classes, bool structurally_volatile, const StyleSheet& style_sheet);
 
 	/// Returns a specific property from the element definition's base properties.
 	/// @param[in] name The name of the property to return.

+ 17 - 23
Source/Core/Factory.cpp

@@ -60,7 +60,7 @@ typedef UnorderedMap< String, ElementInstancer* > ElementInstancerMap;
 static ElementInstancerMap element_instancers;
 
 // Decorator instancers.
-typedef UnorderedMap< String, DecoratorInstancer* > DecoratorInstancerMap;
+typedef UnorderedMap< String, std::unique_ptr<DecoratorInstancer> > DecoratorInstancerMap;
 static DecoratorInstancerMap decorator_instancers;
 
 // Font effect instancers.
@@ -106,11 +106,11 @@ bool Factory::Initialise()
 	RegisterElementInstancer("body", new ElementInstancerGeneric< ElementDocument >())->RemoveReference();
 
 	// Bind the default decorator instancers
-	RegisterDecoratorInstancer("tiled-horizontal", new DecoratorTiledHorizontalInstancer())->RemoveReference();
-	RegisterDecoratorInstancer("tiled-vertical", new DecoratorTiledVerticalInstancer())->RemoveReference();
-	RegisterDecoratorInstancer("tiled-box", new DecoratorTiledBoxInstancer())->RemoveReference();
-	RegisterDecoratorInstancer("image", new DecoratorTiledImageInstancer())->RemoveReference();
-	RegisterDecoratorInstancer("none", new DecoratorNoneInstancer())->RemoveReference();
+	RegisterDecoratorInstancer("tiled-horizontal", std::make_unique<DecoratorTiledHorizontalInstancer>());
+	RegisterDecoratorInstancer("tiled-vertical", std::make_unique<DecoratorTiledVerticalInstancer>());
+	RegisterDecoratorInstancer("tiled-box", std::make_unique<DecoratorTiledBoxInstancer>());
+	RegisterDecoratorInstancer("image", std::make_unique<DecoratorTiledImageInstancer>());
+	RegisterDecoratorInstancer("none", std::make_unique<DecoratorNoneInstancer>());
 
 	RegisterFontEffectInstancer("shadow", new FontEffectShadowInstancer())->RemoveReference();
 	RegisterFontEffectInstancer("outline", new FontEffectOutlineInstancer())->RemoveReference();
@@ -131,8 +131,6 @@ void Factory::Shutdown()
 		(*i).second->RemoveReference();
 	element_instancers.clear();
 
-	for (DecoratorInstancerMap::iterator i = decorator_instancers.begin(); i != decorator_instancers.end(); ++i)
-		(*i).second->RemoveReference();
 	decorator_instancers.clear();
 
 	for (FontEffectInstancerMap::iterator i = font_effect_instancers.begin(); i != font_effect_instancers.end(); ++i)
@@ -328,19 +326,15 @@ ElementDocument* Factory::InstanceDocumentStream(Rocket::Core::Context* context,
 	return document;
 }
 
+
 // Registers an instancer that will be used to instance decorators.
-DecoratorInstancer* Factory::RegisterDecoratorInstancer(const String& name, DecoratorInstancer* instancer)
+void Factory::RegisterDecoratorInstancer(const String& name, std::unique_ptr<DecoratorInstancer> instancer)
 {
-	String lower_case_name = ToLower(name);
-	instancer->AddReference();
+	if (!instancer)
+		return;
 
-	// Check if an instancer for this tag is already defined. If so, release it.
-	DecoratorInstancerMap::iterator iterator = decorator_instancers.find(lower_case_name);
-	if (iterator != decorator_instancers.end())
-		(*iterator).second->RemoveReference();
-
-	decorator_instancers[lower_case_name] = instancer;
-	return instancer;
+	String lower_case_name = ToLower(name);
+	decorator_instancers[lower_case_name] = std::move(instancer);
 }
 
 const PropertySpecification* Factory::GetDecoratorPropertySpecification(const String& name)
@@ -353,7 +347,7 @@ const PropertySpecification* Factory::GetDecoratorPropertySpecification(const St
 }
 
 // Attempts to instance a decorator from an instancer registered with the factory.
-Decorator* Factory::InstanceDecorator(const String& name, const PropertyDictionary& properties, const StyleSheet& style_sheet)
+std::shared_ptr<Decorator> Factory::InstanceDecorator(const String& name, const PropertyDictionary& properties, const StyleSheet& style_sheet)
 {
 	// TODO: z-index, specificity no longer part of decorator
 	float z_index = 0;
@@ -363,10 +357,10 @@ Decorator* Factory::InstanceDecorator(const String& name, const PropertyDictiona
 	if (iterator == decorator_instancers.end())
 		return nullptr;
 
-	DecoratorInstancer* instancer = iterator->second;
+	DecoratorInstancer& instancer = *iterator->second;
 
 	// Turn the generic, un-parsed properties we've got into a properly parsed dictionary.
-	const PropertySpecification& property_specification = instancer->GetPropertySpecification();
+	const PropertySpecification& property_specification = instancer.GetPropertySpecification();
 
 	// Verify all properties set
 	if((size_t)properties.GetNumProperties() != property_specification.GetRegisteredProperties().size())
@@ -376,13 +370,13 @@ Decorator* Factory::InstanceDecorator(const String& name, const PropertyDictiona
 		return nullptr;
 	}
 
-	Decorator* decorator = instancer->InstanceDecorator(name, properties, style_sheet);
+	std::shared_ptr<Decorator> decorator = instancer.InstanceDecorator(name, properties, style_sheet);
 	if (!decorator)
 		return nullptr;
 
 	decorator->SetZIndex(z_index);
 	decorator->SetSpecificity(specificity);
-	decorator->instancer = instancer;
+	decorator->instancer = &instancer;
 	return decorator;
 }
 

+ 4 - 13
Source/Core/StyleSheet.cpp

@@ -62,10 +62,6 @@ StyleSheet::~StyleSheet()
 {
 	delete root;
 
-	// Release decorators
-	for (auto& pair : decorator_map)
-		pair.second.decorator->RemoveReference();
-
 	// Release our reference count on the cached element definitions.
 	for (ElementDefinitionCache::iterator cache_iterator = address_cache.begin(); cache_iterator != address_cache.end(); ++cache_iterator)
 		(*cache_iterator).second->RemoveReference();
@@ -102,15 +98,11 @@ StyleSheet* StyleSheet::CombineStyleSheet(const StyleSheet* other_sheet) const
 	}
 
 	// Copy over the decorators, and replace any matching decorator names from other_sheet
-	// @todo / @leak: Add and remove references as appropriate, not sufficient as is!
 	new_sheet->decorator_map = decorator_map;
 	for (auto& other_decorator: other_sheet->decorator_map)
 	{
 		new_sheet->decorator_map[other_decorator.first] = other_decorator.second;
 	}
-	for (auto& pair : new_sheet->decorator_map)
-		pair.second.decorator->AddReference();
-
 
 	new_sheet->spritesheet_list = other_sheet->spritesheet_list;
 	new_sheet->spritesheet_list.Merge(spritesheet_list);
@@ -140,7 +132,7 @@ Keyframes * StyleSheet::GetKeyframes(const String & name)
 	return nullptr;
 }
 
-Decorator* StyleSheet::GetDecorator(const String& name) const
+std::shared_ptr<Decorator> StyleSheet::GetDecorator(const String& name) const
 {
 	auto it = decorator_map.find(name);
 	if (it == decorator_map.end())
@@ -153,7 +145,7 @@ const Sprite* StyleSheet::GetSprite(const String& name) const
 	return spritesheet_list.GetSprite(name);
 }
 
-Decorator* StyleSheet::GetOrInstanceDecorator(const String& decorator_value, const String& source_file, int source_line_number)
+std::shared_ptr<Decorator> StyleSheet::GetOrInstanceDecorator(const String& decorator_value, const String& source_file, int source_line_number)
 {
 	// Try to find a decorator declared with @decorator or otherwise previously instanced shorthand decorator.
 	auto it_find = decorator_map.find(decorator_value);
@@ -191,7 +183,7 @@ Decorator* StyleSheet::GetOrInstanceDecorator(const String& decorator_value, con
 
 	specification->SetPropertyDefaults(properties);
 
-	Decorator* decorator = Factory::InstanceDecorator(type, properties, *this);
+	std::shared_ptr<Decorator> decorator = Factory::InstanceDecorator(type, properties, *this);
 	if (!decorator)
 		return nullptr;
 
@@ -200,7 +192,6 @@ Decorator* StyleSheet::GetOrInstanceDecorator(const String& decorator_value, con
 	
 	if (!result.second)
 	{
-		decorator->RemoveReference();
 		return nullptr;
 	}
 
@@ -308,7 +299,7 @@ ElementDefinition* StyleSheet::GetElementDefinition(const Element* element) cons
 	// Create the new definition and add it to our cache. One reference count is added, bringing the total to two; one
 	// for the element that requested it, and one for the cache.
 	ElementDefinition* new_definition = new ElementDefinition();
-	new_definition->Initialise(applicable_nodes, volatile_pseudo_classes, structurally_volatile);
+	new_definition->Initialise(applicable_nodes, volatile_pseudo_classes, structurally_volatile, *this);
 
 	// Add to the address cache.
 //	address_cache[element_address] = new_definition;

+ 2 - 2
Source/Core/StyleSheetParser.cpp

@@ -304,14 +304,14 @@ bool StyleSheetParser::ParseDecoratorBlock(const String& at_name, DecoratorSpeci
 	// Set non-defined properties to their defaults
 	property_specification->SetPropertyDefaults(properties);
 
-	Decorator* decorator = Factory::InstanceDecorator(decorator_type, properties, style_sheet);
+	std::shared_ptr<Decorator> decorator = Factory::InstanceDecorator(decorator_type, properties, style_sheet);
 	if (!decorator)
 	{
 		Log::Message(Log::LT_WARNING, "Could not instance decorator of type '%s' declared at %s:%d.", decorator_type.c_str(), stream_file_name.c_str(), line_number);
 		return false;
 	}
 
-	decorator_map.emplace(name, DecoratorSpecification{ std::move(decorator_type), std::move(properties), decorator });
+	decorator_map.emplace(name, DecoratorSpecification{ std::move(decorator_type), std::move(properties), std::move(decorator) });
 
 	return true;
 }