summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorfrosch <frosch@openttd.org>2021-05-09 17:12:34 +0200
committerfrosch <github@elsenhans.name>2021-05-12 23:22:41 +0200
commitf6d5c0136e5ac130fd887daf1b07c13a160c7fc3 (patch)
tree783aa6614c892db50681bd25f1afacfb8c7d37b7
parentf96f11395162a8d57f5fdcef18c7a64f7acf76f8 (diff)
downloadopenttd-f6d5c0136e5ac130fd887daf1b07c13a160c7fc3.tar.xz
Codechange: make Window destruction not rely on undefined behavior.
-rw-r--r--src/window.cpp136
-rw-r--r--src/window_gui.h67
2 files changed, 63 insertions, 140 deletions
diff --git a/src/window.cpp b/src/window.cpp
index 54671f90b..af4721445 100644
--- a/src/window.cpp
+++ b/src/window.cpp
@@ -53,10 +53,8 @@ static Point _drag_delta; ///< delta between mouse cursor and upper left corner
static Window *_mouseover_last_w = nullptr; ///< Window of the last OnMouseOver event.
static Window *_last_scroll_window = nullptr; ///< Window of the last scroll event.
-/** List of windows opened at the screen sorted from the front. */
-Window *_z_front_window = nullptr;
-/** List of windows opened at the screen sorted from the back. */
-Window *_z_back_window = nullptr;
+/** List of windows opened at the screen sorted from the front to back. */
+WindowList _z_windows;
/** If false, highlight is white, otherwise the by the widget defined colour. */
bool _window_highlight_colour = false;
@@ -1109,16 +1107,7 @@ Window::~Window()
free(this->nested_array); // Contents is released through deletion of #nested_root.
delete this->nested_root;
- /*
- * Make fairly sure that this is written, and not "optimized" away.
- * The delete operator is overwritten to not delete it; the deletion
- * happens at a later moment in time after the window has been
- * removed from the list of windows to prevent issues with items
- * being removed during the iteration as not one but more windows
- * may be removed by a single call to ~Window by means of the
- * DeleteChildWindows function.
- */
- const_cast<volatile WindowClass &>(this->window_class) = WC_INVALID;
+ *this->z_position = nullptr;
}
/**
@@ -1231,7 +1220,7 @@ void ChangeWindowOwner(Owner old_owner, Owner new_owner)
}
}
-static void BringWindowToFront(Window *w);
+static void BringWindowToFront(Window *w, bool dirty = true);
/**
* Find a window and make it the relative top-window on the screen.
@@ -1352,89 +1341,22 @@ static uint GetWindowZPriority(WindowClass wc)
}
/**
- * Adds a window to the z-ordering, according to its z-priority.
- * @param w Window to add
- */
-static void AddWindowToZOrdering(Window *w)
-{
- assert(w->z_front == nullptr && w->z_back == nullptr);
-
- if (_z_front_window == nullptr) {
- /* It's the only window. */
- _z_front_window = _z_back_window = w;
- w->z_front = w->z_back = nullptr;
- } else {
- /* Search down the z-ordering for its location. */
- Window *v = _z_front_window;
- uint last_z_priority = UINT_MAX;
- (void)last_z_priority; // Unused without asserts
- while (v != nullptr && (v->window_class == WC_INVALID || GetWindowZPriority(v->window_class) > GetWindowZPriority(w->window_class))) {
- if (v->window_class != WC_INVALID) {
- /* Sanity check z-ordering, while we're at it. */
- assert(last_z_priority >= GetWindowZPriority(v->window_class));
- last_z_priority = GetWindowZPriority(v->window_class);
- }
-
- v = v->z_back;
- }
-
- if (v == nullptr) {
- /* It's the new back window. */
- w->z_front = _z_back_window;
- w->z_back = nullptr;
- _z_back_window->z_back = w;
- _z_back_window = w;
- } else if (v == _z_front_window) {
- /* It's the new front window. */
- w->z_front = nullptr;
- w->z_back = _z_front_window;
- _z_front_window->z_front = w;
- _z_front_window = w;
- } else {
- /* It's somewhere else in the z-ordering. */
- w->z_front = v->z_front;
- w->z_back = v;
- v->z_front->z_back = w;
- v->z_front = w;
- }
- }
-}
-
-
-/**
- * Removes a window from the z-ordering.
- * @param w Window to remove
- */
-static void RemoveWindowFromZOrdering(Window *w)
-{
- if (w->z_front == nullptr) {
- assert(_z_front_window == w);
- _z_front_window = w->z_back;
- } else {
- w->z_front->z_back = w->z_back;
- }
-
- if (w->z_back == nullptr) {
- assert(_z_back_window == w);
- _z_back_window = w->z_front;
- } else {
- w->z_back->z_front = w->z_front;
- }
-
- w->z_front = w->z_back = nullptr;
-}
-
-/**
* On clicking on a window, make it the frontmost window of all windows with an equal
* or lower z-priority. The window is marked dirty for a repaint
* @param w window that is put into the relative foreground
+ * @param dirty whether to mark the window dirty
*/
-static void BringWindowToFront(Window *w)
+static void BringWindowToFront(Window *w, bool dirty)
{
- RemoveWindowFromZOrdering(w);
- AddWindowToZOrdering(w);
+ auto priority = GetWindowZPriority(w->window_class);
+ WindowList::iterator dest = _z_windows.begin();
+ while (dest != _z_windows.end() && (*dest == nullptr || GetWindowZPriority((*dest)->window_class) <= priority)) ++dest;
- w->SetDirty();
+ if (dest != w->z_position) {
+ _z_windows.splice(dest, _z_windows, w->z_position);
+ }
+
+ if (dirty) w->SetDirty();
}
/**
@@ -1476,7 +1398,7 @@ void Window::InitializeData(WindowNumber window_number)
if (!EditBoxInGlobalFocus() || this->nested_root->GetWidgetOfType(WWT_EDITBOX) != nullptr) SetFocusedWindow(this);
/* Insert the window into the correct location in the z-ordering. */
- AddWindowToZOrdering(this);
+ BringWindowToFront(this, false);
}
/**
@@ -1848,6 +1770,7 @@ void Window::InitNested(WindowNumber window_number)
*/
Window::Window(WindowDesc *desc) : window_desc(desc), mouse_capture_widget(-1)
{
+ this->z_position = _z_windows.insert(_z_windows.end(), this);
}
/**
@@ -1875,8 +1798,6 @@ void InitWindowSystem()
{
IConsoleClose();
- _z_back_window = nullptr;
- _z_front_window = nullptr;
_focused_window = nullptr;
_mouseover_last_w = nullptr;
_last_scroll_window = nullptr;
@@ -1898,14 +1819,7 @@ void UnInitWindowSystem()
for (Window *w : Window::Iterate()) delete w;
- for (Window *w = _z_front_window; w != nullptr; /* nothing */) {
- Window *to_del = w;
- w = w->z_back;
- free(to_del);
- }
-
- _z_front_window = nullptr;
- _z_back_window = nullptr;
+ _z_windows.clear();
}
/**
@@ -3068,15 +2982,13 @@ void InputLoop()
CheckSoftLimit();
- /* Do the actual free of the deleted windows. */
- for (Window *v = _z_front_window; v != nullptr; /* nothing */) {
- Window *w = v;
- v = v->z_back;
-
- if (w->window_class != WC_INVALID) continue;
-
- RemoveWindowFromZOrdering(w);
- free(w);
+ /* Remove dead entries from the window list */
+ for (auto it = _z_windows.begin(); it != _z_windows.end(); ) {
+ if (*it == nullptr) {
+ it = _z_windows.erase(it);
+ } else {
+ ++it;
+ }
}
if (_input_events_this_tick != 0) {
diff --git a/src/window_gui.h b/src/window_gui.h
index 2b7c69367..1195c6c49 100644
--- a/src/window_gui.h
+++ b/src/window_gui.h
@@ -10,6 +10,8 @@
#ifndef WINDOW_GUI_H
#define WINDOW_GUI_H
+#include <list>
+
#include "vehicle_type.h"
#include "viewport_type.h"
#include "company_type.h"
@@ -143,8 +145,8 @@ void DrawFrameRect(int left, int top, int right, int bottom, Colours colour, Fra
void DrawCaption(const Rect &r, Colours colour, Owner owner, TextColour text_colour, StringID str, StringAlignment align);
/* window.cpp */
-extern Window *_z_front_window;
-extern Window *_z_back_window;
+using WindowList = std::list<Window *>;
+extern WindowList _z_windows;
extern Window *_focused_window;
@@ -293,19 +295,7 @@ public:
* to destruct them all at the same time too, which is kinda hard.
* @param size the amount of space not to allocate
*/
- inline void *operator new[](size_t size)
- {
- NOT_REACHED();
- }
-
- /**
- * Helper allocation function to disallow something.
- * Don't free the window directly; it corrupts the linked list when iterating
- * @param ptr the pointer not to free
- */
- inline void operator delete(void *ptr)
- {
- }
+ inline void *operator new[](size_t size) = delete;
WindowDesc *window_desc; ///< Window description
WindowFlags flags; ///< Window flags
@@ -336,8 +326,7 @@ public:
int mouse_capture_widget; ///< Widgetindex of current mouse capture widget (e.g. dragged scrollbar). -1 if no widget has mouse capture.
Window *parent; ///< Parent window.
- Window *z_front; ///< The window in front of us in z-order.
- Window *z_back; ///< The window behind us in z-order.
+ WindowList::iterator z_position;
template <class NWID>
inline const NWID *GetWidget(uint widnum) const;
@@ -813,9 +802,9 @@ public:
/**
* Iterator to iterate all valid Windows
- * @tparam Tfront Wether we iterate from front
+ * @tparam TtoBack whether we iterate towards the back.
*/
- template <bool Tfront>
+ template <bool TtoBack>
struct WindowIterator {
typedef Window *value_type;
typedef value_type *pointer;
@@ -823,22 +812,35 @@ public:
typedef size_t difference_type;
typedef std::forward_iterator_tag iterator_category;
- explicit WindowIterator(const Window *start) : w(const_cast<Window *>(start))
+ explicit WindowIterator(WindowList::iterator start) : it(start)
{
this->Validate();
}
+ explicit WindowIterator(const Window *w) : it(w->z_position) {}
- bool operator==(const WindowIterator &other) const { return this->w == other.w; }
+ bool operator==(const WindowIterator &other) const { return this->it == other.it; }
bool operator!=(const WindowIterator &other) const { return !(*this == other); }
- Window * operator*() const { return this->w; }
+ Window * operator*() const { return *this->it; }
WindowIterator & operator++() { this->Next(); this->Validate(); return *this; }
- bool IsEnd() const { return this->w == nullptr; }
+ bool IsEnd() const { return this->it == _z_windows.end(); }
private:
- Window *w;
- void Validate() { while (this->w != nullptr && this->w->window_class == WC_INVALID) this->Next(); }
- void Next() { if (this->w != nullptr) this->w = Tfront ? this->w->z_back : this->w->z_front; }
+ WindowList::iterator it;
+ void Validate()
+ {
+ while (!this->IsEnd() && *this->it == nullptr) this->Next();
+ }
+ void Next()
+ {
+ if constexpr (!TtoBack) {
+ ++this->it;
+ } else if (this->it == _z_windows.begin()) {
+ this->it = _z_windows.end();
+ } else {
+ --this->it;
+ }
+ }
};
using IteratorToFront = WindowIterator<false>; //!< Iterate in Z order towards front.
using IteratorToBack = WindowIterator<true>; //!< Iterate in Z order towards back.
@@ -850,8 +852,17 @@ public:
template <bool Tfront>
struct AllWindows {
AllWindows() {}
- WindowIterator<Tfront> begin() { return WindowIterator<Tfront>(Tfront ? _z_front_window : _z_back_window); }
- WindowIterator<Tfront> end() { return WindowIterator<Tfront>(nullptr); }
+ WindowIterator<Tfront> begin()
+ {
+ if constexpr (Tfront) {
+ auto back = _z_windows.end();
+ if (back != _z_windows.begin()) --back;
+ return WindowIterator<Tfront>(back);
+ } else {
+ return WindowIterator<Tfront>(_z_windows.begin());
+ }
+ }
+ WindowIterator<Tfront> end() { return WindowIterator<Tfront>(_z_windows.end()); }
};
using Iterate = AllWindows<false>; //!< Iterate all windows in whatever order is easiest.
using IterateFromBack = AllWindows<false>; //!< Iterate all windows in Z order from back to front.