cpp/openttd-patchpack/source Changeset - r28363:a9abee2680b7

Changeset - r28363:a9abee2680b7

Parent rev.

Child rev.

[Not reviewed]

master

0 5 0

Peter Nelson - 11 months ago 2023-12-30 18:24:26
peter1138@openttd.org

Fix #11655: Crash due to NWidgetMatrix modifying widget->index. (#11657)

NWidgetMatrix modifies its child widget's index to indicate which element
is to be drawn, which now causes issues with code that does not know about
stuffing extra data into the index.

Instead, let NWidgetMatrix store the currently processing element, and
retrieve this information from the matrix widget while child widgets are
being drawn.

This means only widgets that are children of NWidgetMatrix need to know
anything about their extra data.

5 files changed with 40 insertions and 35 deletions:

src/object_gui.cpp

src/rail_gui.cpp

src/road_gui.cpp

src/widget.cpp

src/widget_type.h

0 comments (0 inline, 0 general)

src/object_gui.cpp

➞

Show inline comments

@@ @@ -306,7 +306,7 @@ public: @@
 	void DrawWidget(const Rect &r, WidgetID widget) const override
+	{
-		switch (GB(widget, 0, 16)) {
 		switch (widget) {
 			case WID_BO_CLASS_LIST: {
 				Rect mr = r.Shrink(WidgetDimensions::scaled.matrix);
 				uint pos = 0;
@@ @@ -333,7 +333,8 @@ public: @@
 				 * look nice in all layouts, we use the 4x4 layout (smallest previews) as starting point. For the bigger
 				 * previews in the layouts with less views we add space homogeneously on all sides, so the 4x4 preview-rectangle
 				 * is centered in the 2x1, 1x2 resp. 1x1 buttons. */
 				uint matrix_height = this->GetWidget<NWidgetMatrix>(WID_BO_OBJECT_MATRIX)->current_y;
 				const NWidgetMatrix *matrix = this->GetWidget<NWidgetMatrix>(WID_BO_OBJECT_MATRIX);
 				uint matrix_height = matrix->current_y;
 				DrawPixelInfo tmp_dpi;
 				/* Set up a clipping area for the preview. */
@@ @@ -345,7 +346,7 @@ public: @@
 						const DrawTileSprites *dts = &_objects[spec->grf_prop.local_id];
 						DrawOrigTileSeqInGUI(ir.Width() / 2 - 1, (ir.Height() + matrix_height / 2) / 2 - this->object_margin - ScaleSpriteTrad(TILE_PIXELS), dts, PAL_NONE);
 					} else {
-						DrawNewObjectTileInGUI(ir.Width() / 2 - 1, (ir.Height() + matrix_height / 2) / 2 - this->object_margin - ScaleSpriteTrad(TILE_PIXELS), spec, GB(widget, 16, 16));
+						DrawNewObjectTileInGUI(ir.Width() / 2 - 1, (ir.Height() + matrix_height / 2) / 2 - this->object_margin - ScaleSpriteTrad(TILE_PIXELS), spec, matrix->GetCurrentElement());
+					}
+				}
 				break;
@@ @@ -353,7 +354,7 @@ public: @@
 			case WID_BO_SELECT_IMAGE: {
 				ObjectClass *objclass = ObjectClass::Get(_selected_object_class);
-				int obj_index = objclass->GetIndexFromUI(GB(widget, 16, 16));
+				int obj_index = objclass->GetIndexFromUI(this->GetWidget<NWidgetMatrix>(WID_BO_SELECT_MATRIX)->GetCurrentElement());
 				if (obj_index < 0) break;
 				const ObjectSpec *spec = objclass->GetSpec(obj_index);
 				if (spec == nullptr) break;
@@ @@ -496,7 +497,7 @@ public: @@
 	void OnClick([[maybe_unused]] Point pt, WidgetID widget, [[maybe_unused]] int click_count) override
+	{
-		switch (GB(widget, 0, 16)) {
 		switch (widget) {
 			case WID_BO_CLASS_LIST: {
 				auto it = this->vscroll->GetScrolledItemFromWidget(this->object_classes, pt.y, this, widget);
 				if (it == this->object_classes.end()) break;
@@ @@ -508,14 +509,14 @@ public: @@
 			case WID_BO_SELECT_IMAGE: {
 				ObjectClass *objclass = ObjectClass::Get(_selected_object_class);
-				int num_clicked = objclass->GetIndexFromUI(GB(widget, 16, 16));
+				int num_clicked = objclass->GetIndexFromUI(this->GetWidget<NWidgetMatrix>(WID_BO_SELECT_MATRIX)->GetCurrentElement());
 				if (num_clicked >= 0 && objclass->GetSpec(num_clicked)->IsAvailable()) this->SelectOtherObject(num_clicked);
 				break;
+			}
 			case WID_BO_OBJECT_SPRITE:
 				if (_selected_object_index != -1) {
-					_selected_object_view = GB(widget, 16, 16);
+					_selected_object_view = this->GetWidget<NWidgetMatrix>(WID_BO_OBJECT_MATRIX)->GetCurrentElement();
 					this->SelectOtherObject(_selected_object_index); // Re-select the object for a different view.
+				}
 				break;

src/rail_gui.cpp

➞

Show inline comments

@@ @@ -1240,7 +1240,7 @@ public: @@
+	{
 		DrawPixelInfo tmp_dpi;
-		switch (GB(widget, 0, 16)) {
 		switch (widget) {
 			case WID_BRAS_PLATFORM_DIR_X: {
 				/* Set up a clipping area for the '/' station preview */
 				Rect ir = r.Shrink(WidgetDimensions::scaled.bevel);
@@ @@ -1285,7 +1285,7 @@ public: @@
+			}
 			case WID_BRAS_IMAGE: {
-				uint16_t type = GB(widget, 16, 16);
+				uint16_t type = this->GetWidget<NWidgetMatrix>(WID_BRAS_MATRIX)->GetCurrentElement();
 				assert(type < _railstation.station_count);
 				/* Check station availability callback */
 				const StationSpec *statspec = StationClass::Get(_railstation.station_class)->GetSpec(type);
@@ @@ -1325,7 +1325,7 @@ public: @@
 	void OnClick([[maybe_unused]] Point pt, WidgetID widget, [[maybe_unused]] int click_count) override
+	{
-		switch (GB(widget, 0, 16)) {
 		switch (widget) {
 			case WID_BRAS_PLATFORM_DIR_X:
 			case WID_BRAS_PLATFORM_DIR_Y:
 				this->RaiseWidget(_railstation.orientation + WID_BRAS_PLATFORM_DIR_X);
@@ @@ -1470,7 +1470,7 @@ public: @@
+			}
 			case WID_BRAS_IMAGE: {
-				uint16_t y = GB(widget, 16, 16);
+				uint16_t y = this->GetWidget<NWidgetMatrix>(WID_BRAS_MATRIX)->GetCurrentElement();
 				if (y >= _railstation.station_count) return;
 				/* Check station availability callback */
@@ @@ -2095,9 +2095,9 @@ struct BuildRailWaypointWindow : PickerW @@
 	void DrawWidget(const Rect &r, WidgetID widget) const override
+	{
-		switch (GB(widget, 0, 16)) {
 		switch (widget) {
 			case WID_BRW_WAYPOINT: {
-				uint16_t type = this->list.at(GB(widget, 16, 16));
+				uint16_t type = this->list.at(this->GetWidget<NWidgetMatrix>(WID_BRW_WAYPOINT_MATRIX)->GetCurrentElement());
 				const StationSpec *statspec = this->waypoints->GetSpec(type);
 				DrawPixelInfo tmp_dpi;
@@ @@ -2118,9 +2118,9 @@ struct BuildRailWaypointWindow : PickerW @@
 	void OnClick([[maybe_unused]] Point pt, WidgetID widget, [[maybe_unused]] int click_count) override
+	{
-		switch (GB(widget, 0, 16)) {
 		switch (widget) {
 			case WID_BRW_WAYPOINT: {
-				uint16_t sel = GB(widget, 16, 16);
+				uint16_t sel = this->GetWidget<NWidgetMatrix>(WID_BRW_WAYPOINT_MATRIX)->GetCurrentElement();
 				assert(sel < this->list.size());
 				uint16_t type = this->list.at(sel);

src/road_gui.cpp

➞

Show inline comments

@@ @@ -1409,7 +1409,7 @@ public: @@
 	void DrawWidget(const Rect &r, WidgetID widget) const override
+	{
-		switch (GB(widget, 0, 16)) {
 		switch (widget) {
 			case WID_BROS_STATION_NE:
 			case WID_BROS_STATION_SE:
 			case WID_BROS_STATION_SW:
@@ @@ -1451,7 +1451,7 @@ public: @@
+			}
 			case WID_BROS_IMAGE: {
-				uint16_t type = GB(widget, 16, 16);
+				uint16_t type = this->GetWidget<NWidgetMatrix>(WID_BROS_MATRIX)->GetCurrentElement();
 				assert(type < _roadstop_gui_settings.roadstop_count);
 				const RoadStopSpec *spec = RoadStopClass::Get(_roadstop_gui_settings.roadstop_class)->GetSpec(type);
@@ @@ -1497,7 +1497,7 @@ public: @@
 	void OnClick([[maybe_unused]] Point pt, WidgetID widget, [[maybe_unused]] int click_count) override
+	{
-		switch (GB(widget, 0, 16)) {
 		switch (widget) {
 			case WID_BROS_STATION_NE:
 			case WID_BROS_STATION_SE:
 			case WID_BROS_STATION_SW:
@@ @@ -1549,7 +1549,7 @@ public: @@
+			}
 			case WID_BROS_IMAGE: {
-				uint16_t y = GB(widget, 16, 16);
+				uint16_t y = this->GetWidget<NWidgetMatrix>(WID_BROS_MATRIX)->GetCurrentElement();
 				if (y >= _roadstop_gui_settings.roadstop_count) return;
 				const RoadStopSpec *spec = RoadStopClass::Get(_roadstop_gui_settings.roadstop_class)->GetSpec(y);

src/widget.cpp

➞

Show inline comments

@@ @@ -1965,8 +1965,8 @@ void NWidgetMatrix::SetColour(Colours co @@
+}
 /**
  * Sets the clicked widget in the matrix.
  * @param clicked The clicked widget.
  * Sets the clicked element in the matrix.
  * @param clicked The clicked element.
  */
 void NWidgetMatrix::SetClicked(int clicked)
+{
@@ @@ -2014,15 +2014,20 @@ void NWidgetMatrix::SetScrollbar(Scrollb @@
 	this->sb = sb;
+}
 /**
  * Get current element.
  * @returns index of current element.
  */
 int NWidgetMatrix::GetCurrentElement() const
+{
 	return this->current_element;
+}
 void NWidgetMatrix::SetupSmallestSize(Window *w)
+{
 	assert(this->head != nullptr);
 	assert(this->head->next == nullptr);
 	/* Reset the widget number. */
 	NWidgetCore *nw = dynamic_cast<NWidgetCore *>(this->head);
 	assert(nw != nullptr);
 	SB(nw->index, 16, 16, 0);
 	this->head->SetupSmallestSize(w);
 	Dimension padding = { (uint)this->pip_pre + this->pip_post, (uint)this->pip_pre + this->pip_post};
@@ @@ -2087,8 +2092,8 @@ NWidgetCore *NWidgetMatrix::GetWidgetFro @@
 	int widget_row = (y - base_offs_y - (int)this->pip_pre - this->pos_y) / this->widget_h;
 	int sub_wid = (widget_row + start_y) * this->widgets_x + start_x + widget_col;
 	if (sub_wid >= this->count) return nullptr;
 	this->current_element = (widget_row + start_y) * this->widgets_x + start_x + widget_col;
 	if (this->current_element >= this->count) return nullptr;
 	NWidgetCore *child = dynamic_cast<NWidgetCore *>(this->head);
 	assert(child != nullptr);
@@ @@ -2097,8 +2102,6 @@ NWidgetCore *NWidgetMatrix::GetWidgetFro @@
 			this->pos_y + this->pip_pre + widget_row * this->widget_h + base_offs_y,
 			child->smallest_x, child->smallest_y, rtl);
 	SB(child->index, 16, 16, sub_wid);
 	return child->GetWidgetFromPos(x, y);
+}
@@ @@ -2137,12 +2140,11 @@ NWidgetCore *NWidgetMatrix::GetWidgetFro @@
 				if (offs_x >= (int)this->current_x) continue;
 				/* Do we have this many widgets? */
 				int sub_wid = y * this->widgets_x + x;
 				if (sub_wid >= this->count) break;
 				this->current_element = y * this->widgets_x + x;
 				if (this->current_element >= this->count) break;
 				child->AssignSizePosition(ST_RESIZE, offs_x, offs_y, child->smallest_x, child->smallest_y, rtl);
 				child->SetLowered(this->clicked == sub_wid);
 				SB(child->index, 16, 16, sub_wid);
 				child->SetLowered(this->clicked == this->current_element);
 				child->Draw(w);
+			}
+		}

src/widget_type.h

➞

Show inline comments

@@ @@ -564,6 +564,7 @@ public: @@
 	void SetClicked(int clicked);
 	void SetCount(int count);
 	void SetScrollbar(Scrollbar *sb);
 	int GetCurrentElement() const;
 	void SetupSmallestSize(Window *w) override;
 	void AssignSizePosition(SizingType sizing, int x, int y, uint given_width, uint given_height, bool rtl) override;
@@ @@ -574,8 +575,9 @@ public: @@
 protected:
 	WidgetID index; ///< If non-negative, index in the #Window::widget_lookup.
 	Colours colour; ///< Colour of this widget.
 	int clicked;    ///< The currently clicked widget.
 	int count;      ///< Amount of valid widgets.
 	int clicked;    ///< The currently clicked element.
 	int count;      ///< Amount of valid elements.
 	int current_element; ///< The element currently being processed.
 	Scrollbar *sb;  ///< The scrollbar we're associated with.
 private:
 	int widget_w;   ///< The width of the child widget including inter spacing.

0 comments (0 inline, 0 general)