Skip to content

Commit 41a39de

Browse files
lodetricklodetrick
committed
Fix Segfault and Incorrect theme drawing in Tabs
1 parent 6a6a116 commit 41a39de

File tree

3 files changed

+84
-86
lines changed

3 files changed

+84
-86
lines changed

‎scene/gui/tab_bar.cpp‎

Lines changed: 73 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
#include "scene/gui/texture_rect.h"
3636
#include "scene/main/viewport.h"
3737
#include "scene/theme/theme_db.h"
38+
#include <cstdlib>
3839

3940
Size2 TabBar::get_minimum_size() const {
4041
Size2 ms;
@@ -507,13 +508,8 @@ void TabBar::_notification(int p_what) {
507508

508509
int limit_minus_buttons = size.width - theme_cache.increment_icon->get_width() - theme_cache.decrement_icon->get_width();
509510

510-
int ofs = tabs[offset].ofs_cache;
511-
int tab_separation_offset = 0;
512-
513511
// Draw unselected tabs in the back.
514512
for (int i = offset; i <= max_drawn_tab; i++) {
515-
tab_separation_offset = (i - offset) * theme_cache.tab_separation;
516-
517513
if (tabs[i].hidden) {
518514
continue;
519515
}
@@ -533,23 +529,15 @@ void TabBar::_notification(int p_what) {
533529
col = theme_cache.font_unselected_color;
534530
}
535531

536-
_draw_tab(sb, col, i, rtl ? (size.width - ofs - tab_separation_offset - tabs[i].size_cache) : (ofs + tab_separation_offset), false);
532+
_draw_tab(sb, col, i, rtl ? (size.width - tabs[i].ofs_cache - tabs[i].size_cache) : tabs[i].ofs_cache, false);
537533
}
538-
539-
ofs += tabs[i].size_cache;
540534
}
541535

542536
// Draw selected tab in the front, but only if it's visible.
543537
if (current >= offset && current <= max_drawn_tab && !tabs[current].hidden) {
544-
tab_separation_offset = (current - offset) * theme_cache.tab_separation;
545-
if (tab_alignment == ALIGNMENT_LEFT && (current - offset) > 1) {
546-
tab_separation_offset = theme_cache.tab_separation;
547-
}
548-
549538
Ref<StyleBox> sb = tabs[current].disabled ? theme_cache.tab_disabled_style : theme_cache.tab_selected_style;
550-
float x = rtl ? (size.width - tabs[current].ofs_cache - tab_separation_offset - tabs[current].size_cache) : (tabs[current].ofs_cache + tab_separation_offset);
551539

552-
_draw_tab(sb, theme_cache.font_selected_color, current, x, has_focus());
540+
_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());
553541
}
554542

555543
if (buttons_visible) {
@@ -583,43 +571,25 @@ void TabBar::_notification(int p_what) {
583571
}
584572

585573
if (dragging_valid_tab) {
586-
int x;
574+
int x = 0;
587575

588576
int closest_tab = get_closest_tab_idx_to_point(get_local_mouse_position());
589577
if (closest_tab != -1) {
590578
Rect2 tab_rect = get_tab_rect(closest_tab);
579+
x = tab_rect.position.x;
580+
581+
// Only add the tab_separation if closest tab is not on the edge.
582+
bool not_leftmost_tab = -1 != (rtl ? get_next_available(closest_tab) : get_previous_available(closest_tab));
583+
bool not_rightmost_tab = -1 != (rtl ? get_previous_available(closest_tab) : get_next_available(closest_tab));
591584

592585
// Calculate midpoint between tabs.
593-
if (rtl) {
594-
if (get_local_mouse_position().x > tab_rect.position.x + tab_rect.size.width / 2) {
595-
if (closest_tab > 0) { // On right side of closest_tab and not first tab.
596-
Rect2 next_tab_rect = get_tab_rect(closest_tab - 1);
597-
x = (tab_rect.position.x + tab_rect.size.width + next_tab_rect.position.x) / 2;
598-
} else { // First tab, will appear on right edge.
599-
x = tab_rect.position.x + tab_rect.size.width;
600-
}
601-
} else {
602-
if (closest_tab < max_drawn_tab) { // On left side of closest_tab and not last tab.
603-
Rect2 prev_tab_rect = get_tab_rect(closest_tab + 1);
604-
x = (tab_rect.position.x + prev_tab_rect.position.x + prev_tab_rect.size.width) / 2;
605-
} else { // Last tab, will appear on left edge.
606-
x = tab_rect.position.x;
607-
}
608-
}
609-
} else if (get_local_mouse_position().x > tab_rect.position.x + tab_rect.size.width / 2) {
610-
if (closest_tab < max_drawn_tab) { // On right side of closest_tab and not last tab.
611-
Rect2 next_tab_rect = get_tab_rect(closest_tab + 1);
612-
x = (tab_rect.position.x + tab_rect.size.width + next_tab_rect.position.x) / 2;
613-
} else { // Last tab, will appear on right edge.
614-
x = tab_rect.position.x + tab_rect.size.width;
615-
}
616-
} else {
617-
if (closest_tab > 0) { // On left side of closest_tab and not first tab.
618-
Rect2 prev_tab_rect = get_tab_rect(closest_tab - 1);
619-
x = (tab_rect.position.x + prev_tab_rect.position.x + prev_tab_rect.size.width) / 2;
620-
} else { // First tab, will appear on left edge.
621-
x = tab_rect.position.x;
586+
if (get_local_mouse_position().x > tab_rect.get_center().x) {
587+
x += tab_rect.size.x;
588+
if (not_rightmost_tab) {
589+
x += theme_cache.tab_separation / 2;
622590
}
591+
} else if (not_leftmost_tab) {
592+
x -= theme_cache.tab_separation / 2;
623593
}
624594
} else {
625595
if (rtl ^ (get_local_mouse_position().x < get_tab_rect(0).position.x)) {
@@ -839,31 +809,49 @@ int TabBar::get_hovered_tab() const {
839809
return hover;
840810
}
841811

842-
bool TabBar::select_previous_available() {
843-
const int offset_end = (get_current_tab() + 1);
812+
int TabBar::get_previous_available(int p_idx) const {
813+
ERR_FAIL_COND_V(p_idx < -1 || p_idx > get_tab_count(), -1);
814+
const int idx = p_idx == -1 ? get_current_tab() : p_idx;
815+
const int offset_end = idx + 1;
844816
for (int i = 1; i < offset_end; i++) {
845-
int target_tab = get_current_tab() - i;
817+
int target_tab = idx - i;
846818
if (target_tab < 0) {
847819
target_tab += get_tab_count();
848820
}
849821
if (!is_tab_disabled(target_tab) && !is_tab_hidden(target_tab)) {
850-
set_current_tab(target_tab);
851-
return true;
822+
return target_tab;
852823
}
853824
}
854-
return false;
825+
return -1;
855826
}
856827

857-
bool TabBar::select_next_available() {
858-
const int offset_end = (get_tab_count() - get_current_tab());
828+
int TabBar::get_next_available(int p_idx) const {
829+
ERR_FAIL_COND_V(p_idx < -1 || p_idx > get_tab_count(), -1);
830+
const int idx = p_idx == -1 ? get_current_tab() : p_idx;
831+
const int offset_end = get_tab_count() - idx;
859832
for (int i = 1; i < offset_end; i++) {
860-
int target_tab = (get_current_tab() + i) % get_tab_count();
833+
int target_tab = (idx + i) % get_tab_count();
861834
if (!is_tab_disabled(target_tab) && !is_tab_hidden(target_tab)) {
862-
set_current_tab(target_tab);
863-
return true;
835+
return target_tab;
864836
}
865837
}
866-
return false;
838+
return -1;
839+
}
840+
841+
bool TabBar::select_previous_available() {
842+
const int previous_available = get_previous_available();
843+
if (previous_available != -1) {
844+
set_current_tab(previous_available);
845+
}
846+
return previous_available != -1;
847+
}
848+
849+
bool TabBar::select_next_available() {
850+
const int next_available = get_next_available();
851+
if (next_available != -1) {
852+
set_current_tab(next_available);
853+
}
854+
return next_available != -1;
867855
}
868856

869857
void TabBar::set_tab_offset(int p_offset) {
@@ -1187,9 +1175,6 @@ void TabBar::_update_cache(bool p_update_hover) {
11871175
}
11881176

11891177
w += tabs[i].size_cache;
1190-
if ((i - offset) > 0) {
1191-
w += theme_cache.tab_separation;
1192-
}
11931178

11941179
// Check if all tabs would fit inside the area.
11951180
if (clip_tabs && i > offset && (w > limit || (offset > 0 && w > limit_minus_buttons))) {
@@ -1210,6 +1195,9 @@ void TabBar::_update_cache(bool p_update_hover) {
12101195

12111196
max_drawn_tab--;
12121197
}
1198+
} else {
1199+
// Only add the tab separation if this isn't the last tab drawn.
1200+
w += theme_cache.tab_separation;
12131201
}
12141202

12151203
tabs.write[i].accessibility_item_dirty = true;
@@ -1232,10 +1220,11 @@ void TabBar::_update_cache(bool p_update_hover) {
12321220
}
12331221

12341222
for (int i = offset; i <= max_drawn_tab; i++) {
1235-
tabs.write[i].ofs_cache = w;
1236-
12371223
if (!tabs[i].hidden) {
1224+
tabs.write[i].ofs_cache = w;
1225+
12381226
w += tabs[i].size_cache;
1227+
w += theme_cache.tab_separation;
12391228
}
12401229
}
12411230

@@ -1543,28 +1532,35 @@ void TabBar::_move_tab_from(TabBar *p_from_tabbar, int p_from_index, int p_to_in
15431532
}
15441533

15451534
int TabBar::get_tab_idx_at_point(const Point2 &p_point) const {
1535+
if (tabs.is_empty()) {
1536+
return -1;
1537+
}
1538+
15461539
int hover_now = -1;
15471540

1548-
if (!tabs.is_empty()) {
1549-
for (int i = offset; i <= max_drawn_tab; i++) {
1550-
Rect2 rect = get_tab_rect(i);
1551-
if (rect.has_point(p_point)) {
1552-
hover_now = i;
1553-
}
1541+
for (int i = offset; i <= max_drawn_tab; i++) {
1542+
Rect2 rect = get_tab_rect(i);
1543+
if (rect.has_point(p_point)) {
1544+
hover_now = i;
15541545
}
15551546
}
15561547

15571548
return hover_now;
15581549
}
15591550

15601551
int TabBar::get_closest_tab_idx_to_point(const Point2 &p_point) const {
1561-
int closest_tab = get_tab_idx_at_point(p_point); // See if we're hovering over a tab first.
1552+
if (tabs.is_empty()) {
1553+
return -1;
1554+
}
15621555

1563-
if (closest_tab == -1) { // Didn't find a tab, so get the closest one.
1564-
float closest_distance = FLT_MAX;
1565-
for (int i = offset; i <= max_drawn_tab; i++) {
1566-
Vector2 center = get_tab_rect(i).get_center();
1567-
float distance = center.distance_to(p_point);
1556+
int closest_tab = -1;
1557+
float closest_distance = FLT_MAX;
1558+
1559+
// Search along the x-axis since the TabBar is horizontal.
1560+
for (int i = offset; i <= max_drawn_tab; i++) {
1561+
if (!tabs[i].hidden) {
1562+
float center = get_tab_rect(i).get_center().x;
1563+
float distance = Math::abs(center - p_point.x);
15681564
if (distance < closest_distance) {
15691565
closest_distance = distance;
15701566
closest_tab = i;
@@ -1827,14 +1823,11 @@ void TabBar::ensure_tab_visible(int p_idx) {
18271823

18281824
Rect2 TabBar::get_tab_rect(int p_tab) const {
18291825
ERR_FAIL_INDEX_V(p_tab, tabs.size(), Rect2());
1830-
int tab_separation_offset = (p_tab - offset) * theme_cache.tab_separation;
1831-
if (tab_alignment == ALIGNMENT_LEFT && (p_tab - offset) > 1) {
1832-
tab_separation_offset = theme_cache.tab_separation;
1833-
}
1826+
18341827
if (is_layout_rtl()) {
1835-
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);
1828+
return Rect2(get_size().width - tabs[p_tab].ofs_cache - tabs[p_tab].size_cache, 0, tabs[p_tab].size_cache, get_size().height);
18361829
} else {
1837-
return Rect2(tabs[p_tab].ofs_cache + tab_separation_offset, 0, tabs[p_tab].size_cache, get_size().height);
1830+
return Rect2(tabs[p_tab].ofs_cache, 0, tabs[p_tab].size_cache, get_size().height);
18381831
}
18391832
}
18401833

‎scene/gui/tab_bar.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,9 @@ class TabBar : public Control {
261261
int get_previous_tab() const;
262262
int get_hovered_tab() const;
263263

264+
int get_previous_available(int p_idx = -1) const;
265+
int get_next_available(int p_idx = -1) const;
266+
264267
bool select_previous_available();
265268
bool select_next_available();
266269

‎scene/gui/tab_container.cpp‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -347,27 +347,29 @@ void TabContainer::_repaint() {
347347
}
348348

349349
void TabContainer::_update_margins() {
350-
int menu_width = theme_cache.menu_icon->get_width();
351-
352350
// Directly check for validity, to avoid errors when quitting.
353351
bool has_popup = popup_obj_id.is_valid();
352+
int menu_width = 0;
353+
if (has_popup) {
354+
menu_width = theme_cache.menu_icon->get_width();
355+
}
354356

355357
if (get_tab_count() == 0) {
356358
tab_bar->set_offset(SIDE_LEFT, 0);
357-
tab_bar->set_offset(SIDE_RIGHT, has_popup ? -menu_width : 0);
359+
tab_bar->set_offset(SIDE_RIGHT, -menu_width);
358360

359361
return;
360362
}
361363

362364
switch (get_tab_alignment()) {
363365
case TabBar::ALIGNMENT_LEFT: {
364366
tab_bar->set_offset(SIDE_LEFT, theme_cache.side_margin);
365-
tab_bar->set_offset(SIDE_RIGHT, has_popup ? -menu_width : 0);
367+
tab_bar->set_offset(SIDE_RIGHT, -menu_width);
366368
} break;
367369

368370
case TabBar::ALIGNMENT_CENTER: {
369371
tab_bar->set_offset(SIDE_LEFT, 0);
370-
tab_bar->set_offset(SIDE_RIGHT, has_popup ? -menu_width : 0);
372+
tab_bar->set_offset(SIDE_RIGHT, -menu_width);
371373
} break;
372374

373375
case TabBar::ALIGNMENT_RIGHT: {
@@ -384,7 +386,7 @@ void TabContainer::_update_margins() {
384386

385387
// Calculate if all the tabs would still fit if the margin was present.
386388
if (get_clip_tabs() && (tab_bar->get_offset_buttons_visible() || (get_tab_count() > 1 && (total_tabs_width + theme_cache.side_margin) > get_size().width))) {
387-
tab_bar->set_offset(SIDE_RIGHT, has_popup ? -menu_width : 0);
389+
tab_bar->set_offset(SIDE_RIGHT, -menu_width);
388390
} else {
389391
tab_bar->set_offset(SIDE_RIGHT, -theme_cache.side_margin);
390392
}

0 commit comments

Comments
 (0)