Fix Segfault and Incorrect theme drawing in Tabs

Author: lodetrick

scene/gui/tab_bar.cpp

@@ -507,13 +507,8 @@ void TabBar::_notification(int p_what) {
 
 			int limit_minus_buttons = size.width - theme_cache.increment_icon->get_width() - theme_cache.decrement_icon->get_width();
 
-			int ofs = tabs[offset].ofs_cache;
-			int tab_separation_offset = 0;
-
 			// Draw unselected tabs in the back.
 			for (int i = offset; i <= max_drawn_tab; i++) {
-				tab_separation_offset = (i - offset) * theme_cache.tab_separation;
-
 				if (tabs[i].hidden) {
 					continue;
 				}
@@ -533,23 +528,15 @@ void TabBar::_notification(int p_what) {
 						col = theme_cache.font_unselected_color;
 					}
 
-					_draw_tab(sb, col, i, rtl ? (size.width - ofs - tab_separation_offset - tabs[i].size_cache) : (ofs + tab_separation_offset), false);
+					_draw_tab(sb, col, i, rtl ? (size.width - tabs[i].ofs_cache - tabs[i].size_cache) : tabs[i].ofs_cache, false);
 				}
-
-				ofs += tabs[i].size_cache;
 			}
 
 			// Draw selected tab in the front, but only if it's visible.
 			if (current >= offset && current <= max_drawn_tab && !tabs[current].hidden) {
-				tab_separation_offset = (current - offset) * theme_cache.tab_separation;
-				if (tab_alignment == ALIGNMENT_LEFT && (current - offset) > 1) {
-					tab_separation_offset = theme_cache.tab_separation;
-				}
-
 				Ref<StyleBox> sb = tabs[current].disabled ? theme_cache.tab_disabled_style : theme_cache.tab_selected_style;
-				float x = rtl ? (size.width - tabs[current].ofs_cache - tab_separation_offset - tabs[current].size_cache) : (tabs[current].ofs_cache + tab_separation_offset);
 
-				_draw_tab(sb, theme_cache.font_selected_color, current, x, has_focus());
+				_draw_tab(sb, theme_cache.font_selected_color, current, rtl ? (size.width - tabs[current].ofs_cache - tabs[current].size_cache) : tabs[current].ofs_cache, has_focus());
 			}
 
 			if (buttons_visible) {
@@ -1187,9 +1174,6 @@ void TabBar::_update_cache(bool p_update_hover) {
 		}
 
 		w += tabs[i].size_cache;
-		if ((i - offset) > 0) {
-			w += theme_cache.tab_separation;
-		}
 
 		// Check if all tabs would fit inside the area.
 		if (clip_tabs && i > offset && (w > limit || (offset > 0 && w > limit_minus_buttons))) {
@@ -1210,6 +1194,9 @@ void TabBar::_update_cache(bool p_update_hover) {
 
 				max_drawn_tab--;
 			}
+		} else {
+			// Only add the tab separation if this isn't the last tab drawn.
+			w += theme_cache.tab_separation;
 		}
 
 		tabs.write[i].accessibility_item_dirty = true;
@@ -1236,6 +1223,7 @@ void TabBar::_update_cache(bool p_update_hover) {
 
 		if (!tabs[i].hidden) {
 			w += tabs[i].size_cache;
+			w += theme_cache.tab_separation;
 		}
 	}
 
@@ -1827,14 +1815,11 @@ void TabBar::ensure_tab_visible(int p_idx) {
 
 Rect2 TabBar::get_tab_rect(int p_tab) const {
 	ERR_FAIL_INDEX_V(p_tab, tabs.size(), Rect2());
-	int tab_separation_offset = (p_tab - offset) * theme_cache.tab_separation;
-	if (tab_alignment == ALIGNMENT_LEFT && (p_tab - offset) > 1) {
-		tab_separation_offset = theme_cache.tab_separation;
-	}
+
 	if (is_layout_rtl()) {
-		return Rect2(get_size().width - tabs[p_tab].ofs_cache - tab_separation_offset - tabs[p_tab].size_cache, 0, tabs[p_tab].size_cache, get_size().height);
+		return Rect2(get_size().width - tabs[p_tab].ofs_cache - tabs[p_tab].size_cache, 0, tabs[p_tab].size_cache, get_size().height);
 	} else {
-		return Rect2(tabs[p_tab].ofs_cache + tab_separation_offset, 0, tabs[p_tab].size_cache, get_size().height);
+		return Rect2(tabs[p_tab].ofs_cache, 0, tabs[p_tab].size_cache, get_size().height);
 	}
 }
 

scene/gui/tab_container.cpp

@@ -347,27 +347,29 @@ void TabContainer::_repaint() {
 }
 
 void TabContainer::_update_margins() {
-	int menu_width = theme_cache.menu_icon->get_width();
-
 	// Directly check for validity, to avoid errors when quitting.
 	bool has_popup = popup_obj_id.is_valid();
+	int menu_width = 0;
+	if (has_popup) {
+		menu_width = theme_cache.menu_icon->get_width();
+	}
 
 	if (get_tab_count() == 0) {
 		tab_bar->set_offset(SIDE_LEFT, 0);
-		tab_bar->set_offset(SIDE_RIGHT, has_popup ? -menu_width : 0);
+		tab_bar->set_offset(SIDE_RIGHT, -menu_width);
 
 		return;
 	}
 
 	switch (get_tab_alignment()) {
 		case TabBar::ALIGNMENT_LEFT: {
 			tab_bar->set_offset(SIDE_LEFT, theme_cache.side_margin);
-			tab_bar->set_offset(SIDE_RIGHT, has_popup ? -menu_width : 0);
+			tab_bar->set_offset(SIDE_RIGHT, -menu_width);
 		} break;
 
 		case TabBar::ALIGNMENT_CENTER: {
 			tab_bar->set_offset(SIDE_LEFT, 0);
-			tab_bar->set_offset(SIDE_RIGHT, has_popup ? -menu_width : 0);
+			tab_bar->set_offset(SIDE_RIGHT, -menu_width);
 		} break;
 
 		case TabBar::ALIGNMENT_RIGHT: {
@@ -384,7 +386,7 @@ void TabContainer::_update_margins() {
 
 			// Calculate if all the tabs would still fit if the margin was present.
 			if (get_clip_tabs() && (tab_bar->get_offset_buttons_visible() || (get_tab_count() > 1 && (total_tabs_width + theme_cache.side_margin) > get_size().width))) {
-				tab_bar->set_offset(SIDE_RIGHT, has_popup ? -menu_width : 0);
+				tab_bar->set_offset(SIDE_RIGHT, -menu_width);
 			} else {
 				tab_bar->set_offset(SIDE_RIGHT, -theme_cache.side_margin);
 			}