Unit tests for uiTable API (and fix bugs found using it :)

5 files changed, 311 insertions, 36 deletions

diff --git a/source/blender/editors/include/UI_table.h b/source/blender/editors/include/UI_table.h
index ca1220198d8..9524616aa32 100644
--- a/source/blender/editors/include/UI_table.h
+++ b/source/blender/editors/include/UI_table.h
@@ -41,6 +41,11 @@ enum uiTableColumnAlignemt {
 	TABLE_COLUMN_ALIGN_RIGHT,
 };
 
+enum uiTableUnit {
+	TABLE_UNIT_PX,
+	TABLE_UNIT_PERCENT,
+};
+
 
 uiTable *UI_table_horizontal_flow_create(void) ATTR_WARN_UNUSED_RESULT;
 uiTable *UI_table_vertical_flow_create(void) ATTR_WARN_UNUSED_RESULT;
@@ -55,14 +60,13 @@ uiTableColumn *UI_table_column_add(uiTable *table, const char *idname, const cha
                                    uiTableCellDrawFunc cell_draw) ATTR_NONNULL(1, 2);
 void UI_table_column_remove(uiTable *table, uiTableColumn *column) ATTR_NONNULL();
 uiTableColumn *UI_table_column_lookup(uiTable *table, const char *idname) ATTR_NONNULL() ATTR_WARN_UNUSED_RESULT;
-void UI_table_column_width_set(uiTableColumn *col, const uiTableSize width, const int min_width_px) ATTR_NONNULL();
+void UI_table_column_width_set(uiTableColumn *column, const unsigned int width, enum uiTableUnit unit,
+                               const int min_width_px) ATTR_NONNULL();
 void UI_table_column_alignment_set(uiTableColumn *column, enum uiTableColumnAlignemt alignment) ATTR_NONNULL();
 /* *** Rows *** */
 uiTableRow *UI_table_row_add(uiTable *table, void *rowdata) ATTR_NONNULL(1);
 void UI_table_row_height_set(uiTable *table, uiTableRow *row, unsigned int height) ATTR_NONNULL();
 
-uiTableSize UI_table_size_px(const int value);
-uiTableSize UI_table_size_percentage(const int value);
 unsigned int UI_table_get_rowcount(const uiTable *table);
 
 #endif /* __UI_TABLE_H__ */
diff --git a/source/blender/editors/interface/table.c b/source/blender/editors/interface/table.c
index d2a977677d4..94420613eae 100644
--- a/source/blender/editors/interface/table.c
+++ b/source/blender/editors/interface/table.c
@@ -69,10 +69,7 @@ typedef struct TableHorizontalFlow {
  * for size properties (only column widths right now).
  */
 typedef struct uiTableSize {
-	enum {
-		TABLE_UNIT_PX,
-		TABLE_UNIT_PERCENT,
-	} unit;
+	enum uiTableUnit unit;
 	int value;
 } uiTableSize;
 
@@ -154,15 +151,24 @@ static void table_row_height_set(uiTable *table, uiTableRow *row, unsigned int h
 	}
 }
 
-static unsigned int table_get_tot_width_unfixed_columns(const uiTable *table)
+static unsigned int table_column_width_clamp(const uiTableColumn *column, const unsigned int maxwidth,
+                                             const unsigned int unclamped_width)
+{
+	unsigned int width = unclamped_width;
+	CLAMP(width, column->min_width, maxwidth);
+	return width;
+}
+
+static unsigned int table_calc_tot_width_unfixed_columns(const uiTable *table)
 {
 	unsigned int nonfixed_width = table->max_width;
 
 	TABLE_COLUMNS_ITER_BEGIN(table, column)
 	{
 		if (column->width.unit == TABLE_UNIT_PX) {
-			BLI_assert(nonfixed_width - column->width.value > 0);
-			nonfixed_width -= column->width.value;
+			unsigned int width = table_column_width_clamp(column, table->max_width, column->width.value);
+			BLI_assert(nonfixed_width >= width);
+			nonfixed_width -= width;
 		}
 	}
 	TABLE_COLUMNS_ITER_END;
@@ -174,22 +180,23 @@ static struct TableColumnDrawInfo table_column_drawinfo_init(const uiTable *tabl
 {
 	struct TableColumnDrawInfo drawinfo = {};
 
-	drawinfo.totwidth_nonfixed = table_get_tot_width_unfixed_columns(table);
+	drawinfo.totwidth_nonfixed = table_calc_tot_width_unfixed_columns(table);
 	BLI_assert(drawinfo.totwidth_nonfixed <= table->max_width);
 
 	return drawinfo;
 }
 
-static unsigned int table_column_calc_width(const uiTableColumn *column, const struct TableColumnDrawInfo *drawinfo)
+static unsigned int table_column_calc_width(const uiTableColumn *column, const struct TableColumnDrawInfo *drawinfo,
+                                            const unsigned int maxwidth)
 {
 	unsigned int width = column->width.value;
 
 	if (column->width.unit == TABLE_UNIT_PERCENT) {
+		CLAMP_MAX(width, 100); /* more than 100% doesn't make sense */
 		width = iroundf(width * 0.01f * drawinfo->totwidth_nonfixed);
 	}
-	width = MAX2(column->min_width, width);
 
-	return width;
+	return table_column_width_clamp(column, maxwidth, width);
 }
 
 /**
@@ -200,7 +207,7 @@ static void table_column_calc_x_coords(const uiTableColumn *column, const float
                                        struct TableColumnDrawInfo *io_drawinfo,
                                        int *r_xmin, int *r_xmax)
 {
-	const unsigned int width = table_column_calc_width(column, io_drawinfo);
+	const unsigned int width = table_column_calc_width(column, io_drawinfo, max_width);
 
 	if (column->alignment == TABLE_COLUMN_ALIGN_LEFT) {
 		*r_xmin = io_drawinfo->totwidth_left;
@@ -217,11 +224,18 @@ static void table_column_calc_x_coords(const uiTableColumn *column, const float
 	}
 }
 
-static void table_row_calc_y_coords(uiTableRow *row, const unsigned int ofs_y, int *r_ymin, int *r_ymax)
+static void table_row_calc_y_coords(uiTable *table, uiTableRow *row, const unsigned int ofs_y, int *r_ymin, int *r_ymax)
 {
+	unsigned int height = row->height;
+
+	if (table->flow_direction == TABLE_FLOW_HORIZONTAL) {
+		TableHorizontalFlow *horizontal_table = (TableHorizontalFlow *)table;
+		CLAMP_MAX(height, horizontal_table->max_height);
+	}
+
 	/* Assuming inverted direction, from top to bottom. */
 	*r_ymax = -ofs_y;
-	*r_ymin = *r_ymax - row->height;
+	*r_ymin = *r_ymax - height;
 }
 
 
@@ -297,7 +311,7 @@ uiTableColumn *UI_table_column_add(uiTable *table, const char *idname, const cha
 	col->drawname = drawname;
 	col->cell_draw = cell_draw;
 	col->alignment = TABLE_COLUMN_ALIGN_LEFT;
-	UI_table_column_width_set(col, UI_table_size_percentage(100), 0);
+	UI_table_column_width_set(col, 100, TABLE_UNIT_PERCENT, 0);
 
 	BLI_addtail(&table->columns, col);
 
@@ -326,11 +340,13 @@ uiTableColumn *UI_table_column_lookup(uiTable *table, const char *idname)
 /**
  * Set the size info for \a col.
  * \param width: The width in either pixels (#UI_table_size_px), or percentage (#UI_table_size_percentage).
+ * \param min_width_px: Minimum width for the column (always in px).
  */
-void UI_table_column_width_set(uiTableColumn *column, const uiTableSize width, const int min_width_px)
+void UI_table_column_width_set(uiTableColumn *column, const unsigned int width, enum uiTableUnit unit,
+                               const int min_width_px)
 {
-	BLI_assert(width.value >= 0);
-	column->width = width;
+	column->width.unit = unit;
+	column->width.value = width;
 	column->min_width = min_width_px;
 }
 
@@ -371,11 +387,10 @@ void UI_table_draw(uiTable *table)
 	} flow_table = {table};
 
 	struct TableColumnDrawInfo column_drawinfo = table_column_drawinfo_init(table);
-	unsigned int xofs = 0, yofs = 0;
 	BLI_mempool_iter iter;
+	unsigned int xofs = 0, yofs = 0;
 
 
-	BLI_mempool_iternew(table->row_pool, &iter);
 	TABLE_COLUMNS_ITER_BEGIN(table, column)
 	{
 		int prev_row_height = -1; /* to check if rows have consistent height */
@@ -384,6 +399,7 @@ void UI_table_draw(uiTable *table)
 
 		table_column_calc_x_coords(column, table->max_width, &column_drawinfo, &drawrect.xmin, &drawrect.xmax);
 
+		BLI_mempool_iternew(table->row_pool, &iter);
 		for (uiTableRow *row = BLI_mempool_iterstep(&iter); row; row = BLI_mempool_iterstep(&iter)) {
 			/* check for consistent row height */
 			if ((prev_row_height >= 0) && (row->height != prev_row_height)) {
@@ -395,7 +411,7 @@ void UI_table_draw(uiTable *table)
 				yofs = 0;
 			}
 
-			table_row_calc_y_coords(row, yofs, &drawrect.ymin, &drawrect.ymax);
+			table_row_calc_y_coords(table, row, yofs, &drawrect.ymin, &drawrect.ymax);
 			column->cell_draw(row->rowdata, drawrect);
 
 			yofs += row->height;
@@ -415,18 +431,6 @@ void UI_table_draw(uiTable *table)
 	TABLE_COLUMNS_ITER_END;
 }
 
-uiTableSize UI_table_size_px(const int value)
-{
-	uiTableSize size = {.unit = TABLE_UNIT_PX, .value = value};
-	return size;
-}
-
-uiTableSize UI_table_size_percentage(const int value)
-{
-	uiTableSize size = {.unit = TABLE_UNIT_PERCENT, .value = value};
-	return size;
-}
-
 unsigned int UI_table_get_rowcount(const uiTable *table)
 {
 	return BLI_mempool_count(table->row_pool);
diff --git a/tests/gtests/CMakeLists.txt b/tests/gtests/CMakeLists.txt
index a3860ce3e67..ae3129fc23e 100644
--- a/tests/gtests/CMakeLists.txt
+++ b/tests/gtests/CMakeLists.txt
@@ -10,6 +10,7 @@ if(WITH_GTESTS)
 	add_subdirectory(testing)
 	add_subdirectory(blenlib)
 	add_subdirectory(guardedalloc)
+	add_subdirectory(interface)
 	add_subdirectory(bmesh)
 endif()
 
diff --git a/tests/gtests/interface/CMakeLists.txt b/tests/gtests/interface/CMakeLists.txt
new file mode 100644
index 00000000000..d279f1675a1
--- /dev/null
+++ b/tests/gtests/interface/CMakeLists.txt
@@ -0,0 +1,44 @@
+# ***** BEGIN GPL LICENSE BLOCK *****
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software Foundation,
+# Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+#
+# ***** END GPL LICENSE BLOCK *****
+
+set(INC
+	.
+	..
+	../../../source/blender/blenlib
+	../../../source/blender/editors/include
+	../../../source/blender/makesdna
+)
+
+include_directories(${INC})
+
+setup_libdirs()
+get_property(BLENDER_SORTED_LIBS GLOBAL PROPERTY BLENDER_SORTED_LIBS_PROP)
+
+# Current BLENDER_SORTED_LIBS works with starting list of symbols in creator, but not
+# for this test. Doubling the list does let all the symbols be resolved, but link time is a bit painful.
+set(BLENDER_SORTED_LIBS ${BLENDER_SORTED_LIBS} ${BLENDER_SORTED_LIBS})
+
+if(WITH_BUILDINFO)
+	set(_buildinfo_src "$<TARGET_OBJECTS:buildinfoobj>")
+else()
+	set(_buildinfo_src "")
+endif()
+BLENDER_SRC_GTEST(UI_table "UI_table_test.cc;${_buildinfo_src}" "${BLENDER_SORTED_LIBS}")
+unset(_buildinfo_src)
+
+setup_liblinks(UI_table_test)
diff --git a/tests/gtests/interface/UI_table_test.cc b/tests/gtests/interface/UI_table_test.cc
new file mode 100644
index 00000000000..0664a7a7fca
--- /dev/null
+++ b/tests/gtests/interface/UI_table_test.cc
@@ -0,0 +1,222 @@
+/* Apache License, Version 2.0 */
+
+#include "testing/testing.h"
+
+extern "C" {
+#include "BLI_mempool.h"
+#include "BLI_rect.h"
+#include "BLI_utildefines.h"
+#include "UI_table.h"
+}
+
+
+/**
+ * Add a bunch of rows, check if their count matches expectation.
+ */
+TEST(ui_table, RowAdd)
+{
+	uiTable *table = UI_table_vertical_flow_create();
+
+	UI_table_column_add(table, "testcol", NULL, NULL);
+	for (int i = 0; i < 100; i++) {
+		UI_table_row_add(table, NULL);
+	}
+
+	EXPECT_EQ(100, UI_table_get_rowcount(table));
+
+	UI_table_free(table);
+}
+
+static struct {
+	int tot_cells;
+	int tot_rows_col1;
+	int tot_rows_col2;
+	int tot_rows_col3;
+} draw_stats = {};
+
+static void table_draw_test_ex()
+{
+	draw_stats.tot_cells++;
+}
+static void table_draw_test_col1(void *UNUSED(rowdata), rcti UNUSED(drawrect))
+{
+	table_draw_test_ex();
+	draw_stats.tot_rows_col1++;
+}
+static void table_draw_test_col2(void *UNUSED(rowdata), rcti UNUSED(drawrect))
+{
+	table_draw_test_ex();
+	draw_stats.tot_rows_col2++;
+}
+static void table_draw_test_col3(void *UNUSED(rowdata), rcti UNUSED(drawrect))
+{
+	table_draw_test_ex();
+	draw_stats.tot_rows_col3++;
+}
+
+/**
+ * Draw a number of columns and rows, gather some statistics and check if they meet expectations.
+ */
+TEST(ui_table, CellsDraw)
+{
+	uiTable *table = UI_table_vertical_flow_create();
+
+	UI_table_column_add(table, "testcol1", NULL, table_draw_test_col1);
+	UI_table_column_add(table, "testcol2", NULL, table_draw_test_col2);
+	UI_table_column_add(table, "testcol3", NULL, table_draw_test_col3);
+	for (int i = 0; i < 10; i++) {
+		UI_table_row_add(table, NULL);
+	}
+
+	/* fills draw_stats */
+	UI_table_draw(table);
+
+	EXPECT_EQ(30, draw_stats.tot_cells);
+	EXPECT_EQ(10, draw_stats.tot_rows_col1);
+	EXPECT_EQ(10, draw_stats.tot_rows_col2);
+	EXPECT_EQ(10, draw_stats.tot_rows_col3);
+
+	UI_table_free(table);
+}
+
+void table_draw_test_alignment_left(void *UNUSED(rowdata), rcti drawrect)
+{
+	EXPECT_EQ(0, drawrect.xmin);
+	EXPECT_EQ(50, drawrect.xmax);
+}
+void table_draw_test_alignment_right(void *UNUSED(rowdata), rcti drawrect)
+{
+	EXPECT_EQ(50, drawrect.xmin);
+	EXPECT_EQ(100, drawrect.xmax);
+}
+
+/**
+ * Check if alignment works as expected with a column-width of 50%, one aligned to left and one to right.
+ */
+TEST(ui_table, ColumnAlignPercentage)
+{
+	uiTable *table;
+	uiTableColumn *col;
+
+	table = UI_table_vertical_flow_create();
+	UI_table_max_width_set(table, 100);
+
+	col = UI_table_column_add(table, "left_align", NULL, table_draw_test_alignment_left);
+	UI_table_column_width_set(col, 50, TABLE_UNIT_PERCENT, 0);
+	col = UI_table_column_add(table, "right_align", NULL, table_draw_test_alignment_right);
+	UI_table_column_width_set(col, 50, TABLE_UNIT_PERCENT, 0);
+	UI_table_column_alignment_set(col, TABLE_COLUMN_ALIGN_RIGHT);
+	for (int i = 0; i < 10; i++) {
+		UI_table_row_add(table, NULL);
+	}
+
+	UI_table_draw(table);
+
+	UI_table_free(table);
+}
+
+void table_draw_test_alignment_left_percent(void *UNUSED(rowdata), rcti drawrect)
+{
+	EXPECT_EQ(10, drawrect.xmin);
+	EXPECT_EQ(50, drawrect.xmax);
+}
+void table_draw_test_alignment_right_percent(void *UNUSED(rowdata), rcti drawrect)
+{
+	EXPECT_EQ(60, drawrect.xmin);
+	EXPECT_EQ(100, drawrect.xmax);
+}
+void table_draw_test_alignment_left_px(void *UNUSED(rowdata), rcti drawrect)
+{
+	EXPECT_EQ(0, drawrect.xmin);
+	EXPECT_EQ(10, drawrect.xmax);
+}
+void table_draw_test_alignment_right_px(void *UNUSED(rowdata), rcti drawrect)
+{
+	EXPECT_EQ(50, drawrect.xmin);
+	EXPECT_EQ(60, drawrect.xmax);
+}
+
+/**
+ * Check if alignment works as expected with mixed left/right alignment and px/percentage sizes.
+ */
+TEST(ui_table, ColumnAlignMixed)
+{
+	uiTable *table;
+	uiTableColumn *col;
+
+	table = UI_table_vertical_flow_create();
+	UI_table_max_width_set(table, 100);
+
+	col = UI_table_column_add(table, "left_align_px", NULL, table_draw_test_alignment_left_px);
+	UI_table_column_width_set(col, 10, TABLE_UNIT_PX, 0);
+	/* intentionally adding a right aligned column first */
+	col = UI_table_column_add(table, "right_align_percent", NULL, table_draw_test_alignment_right_percent);
+	UI_table_column_width_set(col, 50, TABLE_UNIT_PERCENT, 0);
+	UI_table_column_alignment_set(col, TABLE_COLUMN_ALIGN_RIGHT);
+	col = UI_table_column_add(table, "left_align_percent", NULL, table_draw_test_alignment_left_percent);
+	UI_table_column_width_set(col, 50, TABLE_UNIT_PERCENT, 0);
+	col = UI_table_column_add(table, "right_align_px", NULL, table_draw_test_alignment_right_px);
+	UI_table_column_width_set(col, 10, TABLE_UNIT_PX, 0);
+	for (int i = 0; i < 10; i++) {
+		UI_table_row_add(table, NULL);
+	}
+
+	UI_table_draw(table);
+
+	UI_table_free(table);
+}
+
+void table_draw_test_oversize(void *UNUSED(rowdata), rcti drawrect)
+{
+	EXPECT_EQ(100, BLI_rcti_size_x(&drawrect));
+}
+
+/**
+ * Try creating a table with columns of a larger width than table.
+ */
+TEST(ui_table, ColumnOversize)
+{
+	uiTable *table;
+	uiTableColumn *col;
+
+	table = UI_table_vertical_flow_create();
+	UI_table_max_width_set(table, 100);
+
+	col = UI_table_column_add(table, "oversize", NULL, table_draw_test_oversize);
+	UI_table_column_width_set(col, 110, TABLE_UNIT_PX, 0);
+	for (int i = 0; i < 10; i++) {
+		UI_table_row_add(table, NULL);
+	}
+
+	UI_table_draw(table);
+
+	UI_table_free(table);
+}
+
+void table_draw_test_horizontal_flow_oversize(void *UNUSED(rowdata), rcti drawrect)
+{
+	EXPECT_EQ(0, drawrect.ymax);
+	EXPECT_EQ(-10, drawrect.ymin);
+}
+
+/**
+ * Try creating a horizontal-flow table where rows have larger height than table max-height.
+ */
+TEST(ui_table, HorizontalFlowOversize)
+{
+	uiTable *table;
+
+	table = UI_table_horizontal_flow_create();
+	UI_table_horizontal_flow_max_height_set(table, 10);
+	UI_table_max_width_set(table, 100);
+
+	UI_table_column_add(table, "oversize", NULL, table_draw_test_horizontal_flow_oversize);
+	for (int i = 0; i < 10; i++) {
+		uiTableRow *row = UI_table_row_add(table, NULL);
+		UI_table_row_height_set(table, row, 20);
+	}
+
+	UI_table_draw(table);
+
+	UI_table_free(table);
+}