diff --git a/core/templates/hash_map.h b/core/templates/hash_map.h index 8d94cd8f2ee..e8b921c39b2 100644 --- a/core/templates/hash_map.h +++ b/core/templates/hash_map.h @@ -33,6 +33,7 @@ #include "core/os/memory.h" #include "core/templates/hashfuncs.h" #include "core/templates/pair.h" +#include "core/templates/sort_list.h" #include @@ -60,8 +61,6 @@ struct HashMapElement { data(p_key, p_value) {} }; -bool _hashmap_variant_less_than(const Variant &p_left, const Variant &p_right); - template , @@ -265,44 +264,18 @@ public: } void sort() { - if (elements == nullptr || num_elements < 2) { - return; // An empty or single element HashMap is already sorted. - } - // Use insertion sort because we want this operation to be fast for the - // common case where the input is already sorted or nearly sorted. - HashMapElement *inserting = head_element->next; - while (inserting != nullptr) { - HashMapElement *after = nullptr; - for (HashMapElement *current = inserting->prev; current != nullptr; current = current->prev) { - if (_hashmap_variant_less_than(inserting->data.key, current->data.key)) { - after = current; - } else { - break; - } - } - HashMapElement *next = inserting->next; - if (after != nullptr) { - // Modify the elements around `inserting` to remove it from its current position. - inserting->prev->next = next; - if (next == nullptr) { - tail_element = inserting->prev; - } else { - next->prev = inserting->prev; - } - // Modify `before` and `after` to insert `inserting` between them. - HashMapElement *before = after->prev; - if (before == nullptr) { - head_element = inserting; - } else { - before->next = inserting; - } - after->prev = inserting; - // Point `inserting` to its new surroundings. - inserting->prev = before; - inserting->next = after; - } - inserting = next; + sort_custom>(); + } + + template + void sort_custom() { + if (size() < 2) { + return; } + + using E = HashMapElement; + SortList, &E::data, &E::prev, &E::next, C> sorter; + sorter.sort(head_element, tail_element); } TValue &get(const TKey &p_key) { diff --git a/core/templates/list.h b/core/templates/list.h index 9f3ebb34fa2..79a67c7e52e 100644 --- a/core/templates/list.h +++ b/core/templates/list.h @@ -32,7 +32,7 @@ #include "core/error/error_macros.h" #include "core/os/memory.h" -#include "core/templates/sort_array.h" +#include "core/templates/sort_list.h" #include @@ -656,104 +656,18 @@ public: where->prev_ptr = value; } - /** - * simple insertion sort - */ - void sort() { sort_custom>(); } template - void sort_custom_inplace() { + void sort_custom() { if (size() < 2) { return; } - Element *from = front(); - Element *current = from; - Element *to = from; - - while (current) { - Element *next = current->next_ptr; - - if (from != current) { - current->prev_ptr = nullptr; - current->next_ptr = from; - - Element *find = from; - C less; - while (find && less(find->value, current->value)) { - current->prev_ptr = find; - current->next_ptr = find->next_ptr; - find = find->next_ptr; - } - - if (current->prev_ptr) { - current->prev_ptr->next_ptr = current; - } else { - from = current; - } - - if (current->next_ptr) { - current->next_ptr->prev_ptr = current; - } else { - to = current; - } - } else { - current->prev_ptr = nullptr; - current->next_ptr = nullptr; - } - - current = next; - } - _data->first = from; - _data->last = to; - } - - template - struct AuxiliaryComparator { - C compare; - _FORCE_INLINE_ bool operator()(const Element *a, const Element *b) const { - return compare(a->value, b->value); - } - }; - - template - void sort_custom() { - //this version uses auxiliary memory for speed. - //if you don't want to use auxiliary memory, use the in_place version - - int s = size(); - if (s < 2) { - return; - } - - Element **aux_buffer = memnew_arr(Element *, s); - - int idx = 0; - for (Element *E = front(); E; E = E->next_ptr) { - aux_buffer[idx] = E; - idx++; - } - - SortArray> sort; - sort.sort(aux_buffer, s); - - _data->first = aux_buffer[0]; - aux_buffer[0]->prev_ptr = nullptr; - aux_buffer[0]->next_ptr = aux_buffer[1]; - - _data->last = aux_buffer[s - 1]; - aux_buffer[s - 1]->prev_ptr = aux_buffer[s - 2]; - aux_buffer[s - 1]->next_ptr = nullptr; - - for (int i = 1; i < s - 1; i++) { - aux_buffer[i]->prev_ptr = aux_buffer[i - 1]; - aux_buffer[i]->next_ptr = aux_buffer[i + 1]; - } - - memdelete_arr(aux_buffer); + SortList sorter; + sorter.sort(_data->first, _data->last); } const void *id() const { diff --git a/core/templates/self_list.h b/core/templates/self_list.h index f2c8da7d66f..0b98bfe2deb 100644 --- a/core/templates/self_list.h +++ b/core/templates/self_list.h @@ -31,6 +31,7 @@ #pragma once #include "core/error/error_macros.h" +#include "core/templates/sort_list.h" #include "core/typedefs.h" template @@ -114,45 +115,13 @@ public: return; } - SelfList *from = _first; - SelfList *current = from; - SelfList *to = from; - - while (current) { - SelfList *next = current->_next; - - if (from != current) { - current->_prev = nullptr; - current->_next = from; - - SelfList *find = from; - C less; - while (find && less(*find->_self, *current->_self)) { - current->_prev = find; - current->_next = find->_next; - find = find->_next; - } - - if (current->_prev) { - current->_prev->_next = current; - } else { - from = current; - } - - if (current->_next) { - current->_next->_prev = current; - } else { - to = current; - } - } else { - current->_prev = nullptr; - current->_next = nullptr; - } - - current = next; - } - _first = from; - _last = to; + struct PtrComparator { + C compare; + _FORCE_INLINE_ bool operator()(const T *p_a, const T *p_b) const { return compare(*p_a, *p_b); } + }; + using Element = SelfList; + SortList sorter; + sorter.sort(_first, _last); } _FORCE_INLINE_ SelfList *first() { return _first; } diff --git a/core/templates/sort_list.h b/core/templates/sort_list.h new file mode 100644 index 00000000000..3f8bbe5289d --- /dev/null +++ b/core/templates/sort_list.h @@ -0,0 +1,148 @@ +/**************************************************************************/ +/* sort_list.h */ +/**************************************************************************/ +/* This file is part of: */ +/* GODOT ENGINE */ +/* https://godotengine.org */ +/**************************************************************************/ +/* Copyright (c) 2014-present Godot Engine contributors (see AUTHORS.md). */ +/* Copyright (c) 2007-2014 Juan Linietsky, Ariel Manzur. */ +/* */ +/* Permission is hereby granted, free of charge, to any person obtaining */ +/* a copy of this software and associated documentation files (the */ +/* "Software"), to deal in the Software without restriction, including */ +/* without limitation the rights to use, copy, modify, merge, publish, */ +/* distribute, sublicense, and/or sell copies of the Software, and to */ +/* permit persons to whom the Software is furnished to do so, subject to */ +/* the following conditions: */ +/* */ +/* The above copyright notice and this permission notice shall be */ +/* included in all copies or substantial portions of the Software. */ +/* */ +/* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, */ +/* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF */ +/* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. */ +/* IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY */ +/* CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, */ +/* TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE */ +/* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ +/**************************************************************************/ + +#pragma once + +#include "core/typedefs.h" + +template > +class SortList { +public: + Comparator compare; + + void sort(Element *&r_head, Element *&r_tail) { + Element *sorted_until; + if (_is_sorted(r_head, r_tail, sorted_until)) { + return; + } + + // In case we're sorting only part of a larger list. + Element *head_prev = r_head->*prev; + r_head->*prev = nullptr; + Element *tail_next = r_tail->*next; + r_tail->*next = nullptr; + + // Sort unsorted section and merge. + Element *head2 = sorted_until->*next; + _split(sorted_until, head2); + _merge_sort(head2, r_tail); + _merge(r_head, sorted_until, head2, r_tail, r_head, r_tail); + + // Reconnect to larger list if needed. + if (head_prev) { + _connect(head_prev, r_head); + } + if (tail_next) { + _connect(r_tail, tail_next); + } + } + +private: + bool _is_sorted(Element *p_head, Element *p_tail, Element *&r_sorted_until) { + r_sorted_until = p_head; + while (r_sorted_until != p_tail) { + if (compare(r_sorted_until->*next->*value, r_sorted_until->*value)) { + return false; + } + + r_sorted_until = r_sorted_until->*next; + } + + return true; + } + + void _merge_sort(Element *&r_head, Element *&r_tail) { + if (r_head == r_tail) { + return; + } + + Element *tail1 = _get_mid(r_head); + Element *head2 = tail1->*next; + _split(tail1, head2); + + _merge_sort(r_head, tail1); + _merge_sort(head2, r_tail); + _merge(r_head, tail1, head2, r_tail, r_head, r_tail); + } + + void _merge( + Element *p_head1, Element *p_tail1, + Element *p_head2, Element *p_tail2, + Element *&r_head, Element *&r_tail) { + if (compare(p_head2->*value, p_head1->*value)) { + r_head = p_head2; + p_head2 = p_head2->*next; + } else { + r_head = p_head1; + p_head1 = p_head1->*next; + } + + Element *curr = r_head; + while (p_head1 && p_head2) { + if (compare(p_head2->*value, p_head1->*value)) { + _connect(curr, p_head2); + p_head2 = p_head2->*next; + } else { + _connect(curr, p_head1); + p_head1 = p_head1->*next; + } + curr = curr->*next; + } + + if (p_head1) { + _connect(curr, p_head1); + r_tail = p_tail1; + } else { + _connect(curr, p_head2); + r_tail = p_tail2; + } + } + + Element *_get_mid(Element *p_head) { + Element *end = p_head; + Element *mid = p_head; + while (end->*next && end->*next->*next) { + end = end->*next->*next; + mid = mid->*next; + } + + return mid; + } + + _FORCE_INLINE_ void _connect(Element *p_a, Element *p_b) { + p_a->*next = p_b; + p_b->*prev = p_a; + } + + _FORCE_INLINE_ void _split(Element *p_a, Element *p_b) { + p_a->*next = nullptr; + p_b->*prev = nullptr; + } +}; diff --git a/core/variant/dictionary.cpp b/core/variant/dictionary.cpp index ed6693daa38..ebd6c516fef 100644 --- a/core/variant/dictionary.cpp +++ b/core/variant/dictionary.cpp @@ -304,9 +304,21 @@ void Dictionary::clear() { _p->variant_map.clear(); } +struct _DictionaryVariantSort { + _FORCE_INLINE_ bool operator()(const KeyValue &p_l, const KeyValue &p_r) const { + bool valid = false; + Variant res; + Variant::evaluate(Variant::OP_LESS, p_l.key, p_r.key, res, valid); + if (!valid) { + res = false; + } + return res; + } +}; + void Dictionary::sort() { ERR_FAIL_COND_MSG(_p->read_only, "Dictionary is in read-only state."); - _p->variant_map.sort(); + _p->variant_map.sort_custom<_DictionaryVariantSort>(); } void Dictionary::merge(const Dictionary &p_dictionary, bool p_overwrite) { diff --git a/modules/mono/editor/bindings_generator.cpp b/modules/mono/editor/bindings_generator.cpp index f61caeebefe..0d3d00b09f3 100644 --- a/modules/mono/editor/bindings_generator.cpp +++ b/modules/mono/editor/bindings_generator.cpp @@ -4009,7 +4009,7 @@ bool BindingsGenerator::_populate_object_type_interfaces() { List> method_list_with_hashes; ClassDB::get_method_list_with_compatibility(type_cname, &method_list_with_hashes, true); - method_list_with_hashes.sort_custom_inplace(); + method_list_with_hashes.sort_custom(); List compat_methods; for (const Pair &E : method_list_with_hashes) { diff --git a/tests/core/templates/test_hash_map.h b/tests/core/templates/test_hash_map.h index 2b5d381cebd..fa022b303e2 100644 --- a/tests/core/templates/test_hash_map.h +++ b/tests/core/templates/test_hash_map.h @@ -145,4 +145,32 @@ TEST_CASE("[HashMap] Const iteration") { ++idx; } } + +TEST_CASE("[HashMap] Sort") { + HashMap hashmap; + int shuffled_ints[]{ 6, 1, 9, 8, 3, 0, 4, 5, 7, 2 }; + + for (int i : shuffled_ints) { + hashmap[i] = i; + } + hashmap.sort(); + + int i = 0; + for (const KeyValue &kv : hashmap) { + CHECK_EQ(kv.key, i); + i++; + } + + struct ReverseSort { + bool operator()(const KeyValue &p_a, const KeyValue &p_b) { + return p_a.key > p_b.key; + } + }; + hashmap.sort_custom(); + + for (const KeyValue &kv : hashmap) { + i--; + CHECK_EQ(kv.key, i); + } +} } // namespace TestHashMap diff --git a/tests/core/templates/test_list.h b/tests/core/templates/test_list.h index 6f306c480c9..a96b3c60aec 100644 --- a/tests/core/templates/test_list.h +++ b/tests/core/templates/test_list.h @@ -310,19 +310,64 @@ TEST_CASE("[List] Move before") { CHECK(list.front()->next()->get() == n[3]->get()); } +template +static void compare_lists(const List &p_result, const List &p_expected) { + CHECK_EQ(p_result.size(), p_expected.size()); + const typename List::Element *result_it = p_result.front(); + const typename List::Element *expected_it = p_expected.front(); + for (int i = 0; i < p_result.size(); i++) { + CHECK(result_it); + CHECK(expected_it); + CHECK_EQ(result_it->get(), expected_it->get()); + result_it = result_it->next(); + expected_it = expected_it->next(); + } + CHECK(!result_it); + CHECK(!expected_it); + + result_it = p_result.back(); + expected_it = p_expected.back(); + for (int i = 0; i < p_result.size(); i++) { + CHECK(result_it); + CHECK(expected_it); + CHECK_EQ(result_it->get(), expected_it->get()); + result_it = result_it->prev(); + expected_it = expected_it->prev(); + } + CHECK(!result_it); + CHECK(!expected_it); +} + TEST_CASE("[List] Sort") { - List list; - list.push_back("D"); - list.push_back("B"); - list.push_back("A"); - list.push_back("C"); + List result{ "D", "B", "A", "C" }; + result.sort(); + List expected{ "A", "B", "C", "D" }; + compare_lists(result, expected); - list.sort(); + List empty_result{}; + empty_result.sort(); + List empty_expected{}; + compare_lists(empty_result, empty_expected); - CHECK(list.front()->get() == "A"); - CHECK(list.front()->next()->get() == "B"); - CHECK(list.back()->prev()->get() == "C"); - CHECK(list.back()->get() == "D"); + List one_result{ 1 }; + one_result.sort(); + List one_expected{ 1 }; + compare_lists(one_result, one_expected); + + List reversed_result{ 2.0, 1.5, 1.0 }; + reversed_result.sort(); + List reversed_expected{ 1.0, 1.5, 2.0 }; + compare_lists(reversed_result, reversed_expected); + + List already_sorted_result{ 1, 2, 3, 4, 5 }; + already_sorted_result.sort(); + List already_sorted_expected{ 1, 2, 3, 4, 5 }; + compare_lists(already_sorted_result, already_sorted_expected); + + List with_duplicates_result{ 1, 2, 3, 1, 2, 3 }; + with_duplicates_result.sort(); + List with_duplicates_expected{ 1, 1, 2, 2, 3, 3 }; + compare_lists(with_duplicates_result, with_duplicates_expected); } TEST_CASE("[List] Swap adjacent front and back") { diff --git a/core/templates/hash_map.cpp b/tests/core/templates/test_self_list.h similarity index 70% rename from core/templates/hash_map.cpp rename to tests/core/templates/test_self_list.h index 93664dd2e10..1abfc026b5e 100644 --- a/core/templates/hash_map.cpp +++ b/tests/core/templates/test_self_list.h @@ -1,5 +1,5 @@ /**************************************************************************/ -/* hash_map.cpp */ +/* test_self_list.h */ /**************************************************************************/ /* This file is part of: */ /* GODOT ENGINE */ @@ -28,16 +28,45 @@ /* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ /**************************************************************************/ -#include "hash_map.h" +#pragma once -#include "core/variant/variant.h" +#include "core/templates/self_list.h" -bool _hashmap_variant_less_than(const Variant &p_left, const Variant &p_right) { - bool valid = false; - Variant res; - Variant::evaluate(Variant::OP_LESS, p_left, p_right, res, valid); - if (!valid) { - res = false; +#include "tests/test_macros.h" + +namespace TestSelfList { + +TEST_CASE("[SelfList] Sort") { + const int SIZE = 5; + int numbers[SIZE]{ 3, 2, 5, 1, 4 }; + SelfList elements[SIZE]{ + SelfList(&numbers[0]), + SelfList(&numbers[1]), + SelfList(&numbers[2]), + SelfList(&numbers[3]), + SelfList(&numbers[4]), + }; + + SelfList::List list; + for (int i = 0; i < SIZE; i++) { + list.add_last(&elements[i]); + } + + SelfList *it = list.first(); + for (int i = 0; i < SIZE; i++) { + CHECK_EQ(numbers[i], *it->self()); + it = it->next(); + } + + list.sort(); + it = list.first(); + for (int i = 1; i <= SIZE; i++) { + CHECK_EQ(i, *it->self()); + it = it->next(); + } + + for (SelfList &element : elements) { + element.remove_from_list(); } - return res; } +} // namespace TestSelfList diff --git a/tests/test_main.cpp b/tests/test_main.cpp index 7383f6408dd..7663ae04909 100644 --- a/tests/test_main.cpp +++ b/tests/test_main.cpp @@ -106,6 +106,7 @@ #include "tests/core/templates/test_oa_hash_map.h" #include "tests/core/templates/test_paged_array.h" #include "tests/core/templates/test_rid.h" +#include "tests/core/templates/test_self_list.h" #include "tests/core/templates/test_span.h" #include "tests/core/templates/test_vector.h" #include "tests/core/templates/test_vset.h"