Introduce a limit on the number of property ids. This allows PropertyIdSet to become much simpler, requiring no dynamic allocation.

Include/RmlUi/Core/ID.h

@@ -30,11 +30,13 @@
 #ifndef RMLUICOREID_H
 #define RMLUICOREID_H
 
+#include <stdint.h>
+
 namespace Rml {
 namespace Core {
 
 
-enum class ShorthandId : uint16_t
+enum class ShorthandId : uint8_t
 {
 	Invalid,
 
@@ -59,11 +61,14 @@ enum class ShorthandId : uint16_t
 	TransformOrigin,
 
 	NumDefinedIds,
-	FirstCustomId = NumDefinedIds
+	FirstCustomId = NumDefinedIds,
+
+	// The maximum number of IDs. This limits the number of possible custom IDs to MaxNumIds - FirstCustomId.
+	MaxNumIds = UINT8_MAX
 };
 
 
-enum class PropertyId : uint16_t
+enum class PropertyId : uint8_t
 {
 	Invalid,
 
@@ -146,9 +151,11 @@ enum class PropertyId : uint16_t
 	FillImage,
 
 	NumDefinedIds,
-	FirstCustomId = NumDefinedIds
-};
+	FirstCustomId = NumDefinedIds,
 
+	// The maximum number of IDs. This limits the number of possible custom IDs to MaxNumIds - FirstCustomId.
+	MaxNumIds = 128
+};
 
 
 enum class EventId : uint16_t 
@@ -199,7 +206,10 @@ enum class EventId : uint16_t
 	NumDefinedIds,
 
 	// Custom IDs start here
-	FirstCustomId = NumDefinedIds
+	FirstCustomId = NumDefinedIds,
+
+	// The maximum number of IDs. This limits the number of possible custom IDs to MaxNumIds - FirstCustomId.
+	MaxNumIds = UINT16_MAX
 };
 
 }

Include/RmlUi/Core/PropertyIdSet.h

@@ -41,18 +41,15 @@ class PropertyIdSetIterator;
 /*
 	PropertyIdSet is a 'set'-like container for PropertyIds. 
 	
-	It is quite cheap to construct and use, requiring no dynamic allocation for the library-defined IDs as they are based around a bitset.
-	Custom IDs on the other hand need to use a more trafitional set, and are thus more expensive to insert. 
+	Implemented as a wrapper around bitset. It is cheap to construct and use, requiring no dynamic allocation.
 
 	Supports union and intersection operations between two sets, as well as iteration through the IDs that are inserted.
 */
 
-
 class RMLUICORE_API PropertyIdSet {
 private:
-	static constexpr size_t N = (size_t)PropertyId::NumDefinedIds;
+	static constexpr size_t N = size_t(PropertyId::MaxNumIds);
 	std::bitset<N> defined_ids;
-	SmallOrderedSet<PropertyId> custom_ids;
 
 public:
 	PropertyIdSet() {
@@ -60,92 +57,59 @@ public:
 	}
 
 	void Insert(PropertyId id) {
-		if ((size_t)id < N)
-			defined_ids.set((size_t)id);
-		else
-			custom_ids.insert(id);
+		RMLUI_ASSERT(size_t(id) < N);
+		defined_ids.set((size_t)id);
 	}
 
 	void Clear() {
 		defined_ids.reset();
-		custom_ids.clear();
 	}
 	void Erase(PropertyId id) {
-		if ((size_t)id < N)
-			defined_ids.reset((size_t)id);
-		else
-			custom_ids.erase(id);
+		RMLUI_ASSERT(size_t(id) < N);
+		defined_ids.reset((size_t)id);
 	}
 
 	bool Empty() const {
-		return defined_ids.none() && custom_ids.empty();
+		return defined_ids.none();
 	}
 	bool Contains(PropertyId id) const {
-		if ((size_t)id < N)
-			return defined_ids.test((size_t)id);
-		else
-			return custom_ids.count(id) == 1;
+		return defined_ids.test((size_t)id);
 	}
 
 	size_t Size() const {
-		return defined_ids.count() + custom_ids.size();
+		return defined_ids.count();
 	}
 
 	// Union with another set
-	PropertyIdSet& operator|=(const PropertyIdSet& other)
-	{
+	PropertyIdSet& operator|=(const PropertyIdSet& other) {
 		defined_ids |= other.defined_ids;
-		custom_ids.insert(other.custom_ids.begin(), other.custom_ids.end());
 		return *this;
 	}
-
-	PropertyIdSet operator|(const PropertyIdSet& other) const
-	{
+	PropertyIdSet operator|(const PropertyIdSet& other) const {
 		PropertyIdSet result = *this;
 		result |= other;
 		return result;
 	}
 
 	// Intersection with another set
-	PropertyIdSet& operator&=(const PropertyIdSet& other)
-	{
+	PropertyIdSet& operator&=(const PropertyIdSet& other) {
 		defined_ids &= other.defined_ids;
-		if (custom_ids.size() > 0 && other.custom_ids.size() > 0)
-		{
-			for (auto it = custom_ids.begin(); it != custom_ids.end();)
-				if (other.custom_ids.count(*it) == 0)
-					it = custom_ids.erase(it);
-				else
-					++it;
-		}
-		else
-		{
-			custom_ids.clear();
-		}
 		return *this;
 	}
-	
-	PropertyIdSet operator&(const PropertyIdSet& other) const
-	{
+	PropertyIdSet operator&(const PropertyIdSet& other) const {
 		PropertyIdSet result;
 		result.defined_ids = (defined_ids & other.defined_ids);
-		if (custom_ids.size() > 0 && other.custom_ids.size() > 0)
-		{
-			for (PropertyId id : custom_ids)
-				if (other.custom_ids.count(id) == 1)
-					result.custom_ids.insert(id);
-		}
 		return result;
 	}
 
 	// Iterator support. Iterates through all the PropertyIds that are set (contained).
-	// @note: Modifying the container invalidates the iterators. Only const_iterators are provided.
+	// @note: Only const_iterators are provided.
 	inline PropertyIdSetIterator begin() const;
 	inline PropertyIdSetIterator end() const;
 
-	// Erases the property id represented by a valid iterator. Invalidates any previous iterators.
-	// @return A new valid iterator pointing to the next element or end().
-	inline PropertyIdSetIterator Erase(const PropertyIdSetIterator& it);
+	// Erases the property id represented by a valid iterator. Iterator must be in the range [begin, end).
+	// @return A new iterator pointing to the next element or end().
+	inline PropertyIdSetIterator Erase(PropertyIdSetIterator it);
 };
 
 
@@ -153,79 +117,61 @@ public:
 class RMLUICORE_API PropertyIdSetIterator
 {
 public:
-	using CustomIdsIt = SmallOrderedSet<PropertyId>::const_iterator;
-
-	PropertyIdSetIterator() : container(nullptr), defined_ids_index(0), custom_ids_iterator() {}
-	PropertyIdSetIterator(const PropertyIdSet* container, size_t defined_ids_index, CustomIdsIt custom_ids_iterator)
-		: container(container), defined_ids_index(defined_ids_index), custom_ids_iterator(custom_ids_iterator) 
+	PropertyIdSetIterator() = default;
+	PropertyIdSetIterator(const PropertyIdSet* container, size_t id_index) : container(container), id_index(id_index)
 	{
 		ProceedToNextValid();
 	}
 	
 	PropertyIdSetIterator& operator++() {
-		if (defined_ids_index < N)
-			++defined_ids_index;
-		else
-			++custom_ids_iterator;
+		++id_index;
 		ProceedToNextValid();
 		return *this;
 	}
 
 	bool operator==(const PropertyIdSetIterator& other) const {
-		return container == other.container && defined_ids_index == other.defined_ids_index && custom_ids_iterator == other.custom_ids_iterator;
+		return container == other.container && id_index == other.id_index;
 	}
 	bool operator!=(const PropertyIdSetIterator& other) const { 
 		return !(*this == other); 
 	}
-
 	PropertyId operator*() const { 
-		if (defined_ids_index < N)
-			return static_cast<PropertyId>(defined_ids_index);
-		else
-			return *custom_ids_iterator;
+		return static_cast<PropertyId>(id_index);
 	}
 
 private:
 
 	inline void ProceedToNextValid()
 	{
-		for (; defined_ids_index < N; ++defined_ids_index)
+		for (; id_index < size_t(PropertyId::MaxNumIds); ++id_index)
 		{
-			if (container->Contains( static_cast<PropertyId>(defined_ids_index) ))
+			if (container->Contains( static_cast<PropertyId>(id_index) ))
 				return;
 		}
 	}
 
-	static constexpr size_t N = (size_t)PropertyId::NumDefinedIds;
-	const PropertyIdSet* container;
-	size_t defined_ids_index;
-	CustomIdsIt custom_ids_iterator;
+	const PropertyIdSet* container = nullptr;
+	size_t id_index = 0;
 
-	friend PropertyIdSetIterator PropertyIdSet::Erase(const PropertyIdSetIterator&);
+	friend PropertyIdSetIterator PropertyIdSet::Erase(PropertyIdSetIterator);
 };
 
 
 
 PropertyIdSetIterator PropertyIdSet::begin() const {
-	return PropertyIdSetIterator(this, 1, custom_ids.begin());
+	if (Empty())
+		return end();
+	return PropertyIdSetIterator(this, 1);
 }
 
 PropertyIdSetIterator PropertyIdSet::end() const {
-	return PropertyIdSetIterator(this, N, custom_ids.end());
+	return PropertyIdSetIterator(this, N);
 }
 
-PropertyIdSetIterator PropertyIdSet::Erase(const PropertyIdSetIterator& it_in) {
-	RMLUI_ASSERT(it_in.container == this);
-	PropertyIdSetIterator it = it_in;
-	if (it.defined_ids_index < N)
-	{
-		defined_ids.reset(it.defined_ids_index);
-		++it;
-	}
-	else
-	{
-		it.custom_ids_iterator = custom_ids.erase(it.custom_ids_iterator);
-	}
+PropertyIdSetIterator PropertyIdSet::Erase(PropertyIdSetIterator it) {
+	RMLUI_ASSERT(it.container == this && it.id_index < N);
+	defined_ids.reset(it.id_index);
+	++it;
 	return it;
 }
 

Include/RmlUi/Core/PropertySpecification.h

@@ -30,7 +30,9 @@
 #define RMLUICOREPROPERTYSPECIFICATION_H
 
 #include "Header.h"
+#include "Types.h"
 #include "PropertyIdSet.h"
+#include "ID.h"
 
 namespace Rml {
 namespace Core {
@@ -104,9 +106,9 @@ public:
 	const ShorthandDefinition* GetShorthand(ShorthandId id) const;
 	const ShorthandDefinition* GetShorthand(const String& shorthand_name) const;
 
-	/// Parse declaration by name, whether its a property or shorthand.
+	/// Parse declaration by name, whether it's a property or shorthand.
 	bool ParsePropertyDeclaration(PropertyDictionary& dictionary, const String& property_name, const String& property_value) const;
-	/// Parse property declaration by ID
+	/// Parse property declaration by ID.
 	bool ParsePropertyDeclaration(PropertyDictionary& dictionary, PropertyId property_id, const String& property_value) const;
 	/// Parses a shorthand declaration, setting any parsed and validated properties on the given dictionary.
 	/// @return True if all properties were parsed successfully, false otherwise.
@@ -129,9 +131,9 @@ private:
 	UniquePtr<PropertyIdNameMap> property_map;
 	UniquePtr<ShorthandIdNameMap> shorthand_map;
 
-	PropertyIdSet property_names;
-	PropertyIdSet inherited_property_names;
-	PropertyIdSet properties_forcing_layout;
+	PropertyIdSet property_ids;
+	PropertyIdSet property_ids_inherited;
+	PropertyIdSet property_ids_forcing_layout;
 
 	bool ParsePropertyValues(StringList& values_list, const String& values, bool split_values) const;
 

Include/RmlUi/Core/Types.h

@@ -103,7 +103,7 @@ struct Transition;
 struct TransitionList;
 struct Rectangle;
 enum class EventId : uint16_t;
-enum class PropertyId : uint16_t;
+enum class PropertyId : uint8_t;
 
 // Types for external interfaces.
 using FileHandle = uintptr_t;

Source/Core/ElementDefinition.h

@@ -56,7 +56,6 @@ public:
 	const Property* GetProperty(PropertyId id) const;
 
 	/// Returns the list of property ids this element definition defines.
-	/// @param[out] property_names The list to store the defined property ids in.
 	const PropertyIdSet& GetPropertyIds() const;
 
 	const PropertyDictionary& GetProperties() const { return properties; }

Source/Core/EventSpecification.cpp

@@ -122,8 +122,16 @@ static EventSpecification& GetOrInsert(const String& event_type, bool interrupti
 	if (it != type_lookup.end())
 		return GetMutable(it->second);
 
+	const size_t new_id_num = specifications.size();
+	if (new_id_num >= size_t(EventId::MaxNumIds))
+	{
+		Log::Message(Log::LT_ERROR, "Error while registering event type '%s': Maximum number of allowed events exceeded.", event_type.c_str());
+		RMLUI_ERROR;
+		return specifications.front();
+	}
+
 	// No specification found for this name, insert a new entry with default values
-	EventId new_id = static_cast<EventId>(specifications.size());
+	EventId new_id = static_cast<EventId>(new_id_num);
 	specifications.push_back(EventSpecification{ new_id, event_type, interruptible, bubbles, default_action_phase });
 	type_lookup.emplace(event_type, new_id);
 	return specifications.back();

Source/Core/IdNameMap.h

@@ -30,6 +30,7 @@
 #define RMLUICOREIDNAMEMAP_H
 
 #include "../../Include/RmlUi/Core/Header.h"
+#include "../../Include/RmlUi/Core/Types.h"
 #include <algorithm>
 
 namespace Rml {
@@ -49,7 +50,8 @@ protected:
 	}
 
 public:
-	void AddPair(ID id, const String& name) {
+	void AddPair(ID id, const String& name)
+	{
 		// Should only be used for defined IDs
 		if ((size_t)id >= name_map.size())
 			name_map.resize(1 + (size_t)id);
@@ -59,7 +61,8 @@ public:
 		(void)inserted;
 	}
 
-	void AssertAllInserted(ID number_of_defined_ids) const {
+	void AssertAllInserted(ID number_of_defined_ids) const
+	{
 		std::ptrdiff_t cnt = std::count_if(name_map.begin(), name_map.end(), [](const String& name) { return !name.empty(); });
 		RMLUI_ASSERT(cnt == (std::ptrdiff_t)number_of_defined_ids && reverse_map.size() == (size_t)number_of_defined_ids);
 		(void)cnt;

Source/Core/PropertySpecification.cpp

@@ -27,6 +27,7 @@
  */
 
 #include "../../Include/RmlUi/Core/PropertySpecification.h"
+#include "../../Include/RmlUi/Core/Debug.h"
 #include "../../Include/RmlUi/Core/Log.h"
 #include "../../Include/RmlUi/Core/PropertyDefinition.h"
 #include "../../Include/RmlUi/Core/PropertyDictionary.h"
@@ -60,14 +61,20 @@ PropertyDefinition& PropertySpecification::RegisterProperty(const String& proper
 		property_map->AddPair(id, property_name);
 
 	size_t index = (size_t)id;
-	RMLUI_ASSERT(index < (size_t)INT16_MAX);
+
+	if (index >= size_t(PropertyId::MaxNumIds))
+	{
+		Log::Message(Log::LT_ERROR, "Fatal error while registering property '%s': Maximum number of allowed properties exceeded. Continuing execution may lead to crash.", property_name.c_str());
+		RMLUI_ERROR;
+		return *properties[0];
+	}
 
 	if (index < properties.size())
 	{
 		// We don't want to owerwrite an existing entry.
 		if (properties[index])
 		{
-			Log::Message(Log::LT_ERROR, "While registering property '%s': The property is already registered, ignoring.", property_name.c_str());
+			Log::Message(Log::LT_ERROR, "While registering property '%s': The property is already registered.", property_name.c_str());
 			return *properties[index];
 		}
 	}
@@ -79,11 +86,11 @@ PropertyDefinition& PropertySpecification::RegisterProperty(const String& proper
 
 	// Create and insert the new property
 	properties[index] = std::make_unique<PropertyDefinition>(id, default_value, inherited, forces_layout);
-	property_names.Insert(id);
+	property_ids.Insert(id);
 	if (inherited)
-		inherited_property_names.Insert(id);
+		property_ids_inherited.Insert(id);
 	if (forces_layout)
-		properties_forcing_layout.Insert(id);
+		property_ids_forcing_layout.Insert(id);
 
 	return *properties[index];
 }
@@ -105,18 +112,18 @@ const PropertyDefinition* PropertySpecification::GetProperty(const String& prope
 // Fetches a list of the names of all registered property definitions.
 const PropertyIdSet& PropertySpecification::GetRegisteredProperties(void) const
 {
-	return property_names;
+	return property_ids;
 }
 
 // Fetches a list of the names of all registered property definitions.
 const PropertyIdSet& PropertySpecification::GetRegisteredInheritedProperties(void) const
 {
-	return inherited_property_names;
+	return property_ids_inherited;
 }
 
 const PropertyIdSet& PropertySpecification::GetRegisteredPropertiesForcingLayout() const
 {
-	return properties_forcing_layout;
+	return property_ids_forcing_layout;
 }
 
 // Registers a shorthand property definition.
@@ -177,7 +184,12 @@ ShorthandId PropertySpecification::RegisterShorthand(const String& shorthand_nam
 	property_shorthand->type = type;
 
 	const size_t index = (size_t)id;
-	RMLUI_ASSERT(index < (size_t)INT16_MAX);
+
+	if (index >= size_t(ShorthandId::MaxNumIds))
+	{
+		Log::Message(Log::LT_ERROR, "Error while registering shorthand '%s': Maximum number of allowed shorthands exceeded.", shorthand_name.c_str());
+		return ShorthandId::Invalid;
+	}
 
 	if (index < shorthands.size())
 	{

Source/Core/StyleSheetSpecification.cpp

@@ -62,7 +62,7 @@ struct DefaultStyleSheetParsers {
 
 StyleSheetSpecification::StyleSheetSpecification() : 
 	// Reserve space for all defined ids and some more for custom properties
-	properties(2 * (size_t)PropertyId::NumDefinedIds, 2 * (size_t)ShorthandId::NumDefinedIds)
+	properties((size_t)PropertyId::MaxNumIds, 2 * (size_t)ShorthandId::NumDefinedIds)
 {
 	RMLUI_ASSERT(instance == nullptr);
 	instance = this;