Merge pull request #43720 from lawnjelly/ewok_asserts

Batching - more error checking options

drivers/gles3/rasterizer_canvas_base_gles3.cpp

@@ -32,6 +32,7 @@
 
 #include "core/os/os.h"
 #include "core/project_settings.h"
+#include "drivers/gles_common/rasterizer_asserts.h"
 #include "rasterizer_scene_gles3.h"
 #include "servers/visual/visual_server_raster.h"
 
@@ -533,6 +534,13 @@ void RasterizerCanvasBaseGLES3::_draw_generic_indices(GLuint p_primitive, const
 	ERR_FAIL_COND(buffer_ofs > data.polygon_buffer_size);
 #endif
 
+#ifdef RASTERIZER_EXTRA_CHECKS
+	// very slow, do not enable in normal use
+	for (int n = 0; n < p_index_count; n++) {
+		RAST_DEV_DEBUG_ASSERT(p_indices[n] < p_vertex_count);
+	}
+#endif
+
 	//bind the indices buffer.
 	glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, data.polygon_index_buffer);
 	storage->buffer_orphan_and_upload(data.polygon_index_buffer_size, 0, sizeof(int) * p_index_count, p_indices, GL_ELEMENT_ARRAY_BUFFER, _buffer_upload_usage_flag);

drivers/gles_common/batch_diagnose.inc

@@ -1,3 +1,46 @@
+void _debug_write_garbage() {
+	// extremely slow, writes garbage over arrays to detect using
+	// uninitialized in graphical output. Do not enable in normal use!!
+#ifdef RASTERIZER_EXTRA_CHECKS
+	int num_verts = MIN(bdata.vertices.max_size(), 32);
+	for (int n = 0; n < num_verts; n++) {
+		bdata.vertices[n].pos.set(Math::random(-200.0f, 200.0f), Math::random(-200.0f, 200.0f));
+		bdata.vertices[n].uv.set(Math::random(0.0f, 1.0f), Math::random(0.0f, 1.0f));
+	}
+
+	int num_colors = MIN(bdata.vertex_colors.max_size(), 32);
+	for (int n = 0; n < num_colors; n++) {
+		bdata.vertex_colors[n].set(Math::randf(), Math::randf(), Math::randf(), 1.0f);
+	}
+
+	int num_modulates = MIN(bdata.vertex_modulates.max_size(), 32);
+	for (int n = 0; n < num_modulates; n++) {
+		bdata.vertex_modulates[n].set(Math::randf(), Math::randf(), Math::randf(), 1.0f);
+	}
+
+	int num_light_angles = MIN(bdata.light_angles.max_size(), 32);
+	for (int n = 0; n < num_light_angles; n++) {
+		bdata.light_angles[n] = Math::random(-3.0f, +3.0f);
+	}
+
+	int num_transforms = MIN(bdata.vertex_transforms.max_size(), 32);
+	for (int n = 0; n < num_transforms; n++) {
+		bdata.vertex_transforms[n].translate.set(Math::random(-200.0f, 200.0f), Math::random(-200.0f, 200.0f));
+		bdata.vertex_transforms[n].basis[0].set(Math::random(-2.0f, 2.0f), Math::random(-2.0f, 2.0f));
+		bdata.vertex_transforms[n].basis[1].set(Math::random(-2.0f, 2.0f), Math::random(-2.0f, 2.0f));
+	}
+
+	int num_unit_verts = MIN(bdata.unit_vertices.max_size(), 32);
+	for (int n = 0; n < num_unit_verts; n++) {
+		uint8_t *data = bdata.unit_vertices.get_unit(n);
+		for (int b = 0; b > bdata.unit_vertices.get_unit_size_bytes(); b++) {
+			data[b] = Math::random(0, 255);
+		}
+	}
+
+#endif
+}
+
 String get_command_type_string(const RasterizerCanvas::Item::Command &p_command) const {
 	String sz = "";
 

drivers/gles_common/rasterizer_array.h

@@ -28,7 +28,8 @@
 /* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.                */
 /*************************************************************************/
 
-#pragma once
+#ifndef RASTERIZER_ARRAY_H
+#define RASTERIZER_ARRAY_H
 
 /**
  * Fast single-threaded growable array for POD types.
@@ -275,6 +276,7 @@ public:
 
 	int size() const { return _size; }
 	int max_size() const { return _max_size; }
+	int get_unit_size_bytes() const { return _unit_size_bytes; }
 
 	void free() {
 		if (_list) {
@@ -326,3 +328,5 @@ private:
 	int _unit_size_bytes;
 	int _max_unit_size_bytes;
 };
+
+#endif // RASTERIZER_ARRAY_H

drivers/gles_common/rasterizer_asserts.h

@@ -0,0 +1,58 @@
+/*************************************************************************/
+/*  rasterizer_asserts.h                                                 */
+/*************************************************************************/
+/*                       This file is part of:                           */
+/*                           GODOT ENGINE                                */
+/*                      https://godotengine.org                          */
+/*************************************************************************/
+/* Copyright (c) 2007-2020 Juan Linietsky, Ariel Manzur.                 */
+/* Copyright (c) 2014-2020 Godot Engine contributors (cf. AUTHORS.md).   */
+/*                                                                       */
+/* Permission is hereby granted, free of charge, to any person obtaining */
+/* a copy of this software and associated documentation files (the       */
+/* "Software"), to deal in the Software without restriction, including   */
+/* without limitation the rights to use, copy, modify, merge, publish,   */
+/* distribute, sublicense, and/or sell copies of the Software, and to    */
+/* permit persons to whom the Software is furnished to do so, subject to */
+/* the following conditions:                                             */
+/*                                                                       */
+/* The above copyright notice and this permission notice shall be        */
+/* included in all copies or substantial portions of the Software.       */
+/*                                                                       */
+/* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,       */
+/* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF    */
+/* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.*/
+/* IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY  */
+/* CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,  */
+/* TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE     */
+/* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.                */
+/*************************************************************************/
+
+#ifndef RASTERIZER_ASSERTS_H
+#define RASTERIZER_ASSERTS_H
+
+// For flow control checking, we want an easy way to apply asserts that occur in debug development builds only.
+// This is enforced by outputting a warning which will fail CI checks if the define is set in a PR.
+#if defined(TOOLS_ENABLED) && defined(DEBUG_ENABLED)
+// only uncomment this define for error checking in development, not in the main repository
+// as these checks will slow things down in debug builds.
+//#define RASTERIZER_EXTRA_CHECKS
+#endif
+
+#ifdef RASTERIZER_EXTRA_CHECKS
+#ifndef _MSC_VER
+#warning do not define RASTERIZER_EXTRA_CHECKS in main repository builds
+#endif
+#define RAST_DEV_DEBUG_ASSERT(a) CRASH_COND(!(a))
+#else
+#define RAST_DEV_DEBUG_ASSERT(a)
+#endif
+
+// Also very useful, an assert check that only occurs in debug tools builds
+#if defined(TOOLS_ENABLED) && defined(DEBUG_ENABLED)
+#define RAST_DEBUG_ASSERT(a) CRASH_COND(!(a))
+#else
+#define RAST_DEBUG_ASSERT(a)
+#endif
+
+#endif // RASTERIZER_ASSERTS_H

drivers/gles_common/rasterizer_canvas_batcher.h

@@ -28,11 +28,13 @@
 /* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.                */
 /*************************************************************************/
 
-#pragma once
+#ifndef RASTERIZER_CANVAS_BATCHER_H
+#define RASTERIZER_CANVAS_BATCHER_H
 
 #include "core/os/os.h"
 #include "core/project_settings.h"
 #include "rasterizer_array.h"
+#include "rasterizer_asserts.h"
 #include "rasterizer_storage_common.h"
 #include "servers/visual/rasterizer.h"
 
@@ -100,6 +102,12 @@ public:
 			b = p_c.b;
 			a = p_c.a;
 		}
+		void set(float rr, float gg, float bb, float aa) {
+			r = rr;
+			g = gg;
+			b = bb;
+			a = aa;
+		}
 		bool operator==(const BatchColor &p_c) const {
 			return (r == p_c.r) && (g == p_c.g) && (b == p_c.b) && (a == p_c.a);
 		}
@@ -623,9 +631,7 @@ public:
 
 			// this should always succeed after growing
 			batch = bdata.batches.request();
-#if defined(TOOLS_ENABLED) && defined(DEBUG_ENABLED)
-			CRASH_COND(!batch);
-#endif
+			RAST_DEBUG_ASSERT(batch);
 		}
 
 		if (p_blank)
@@ -1488,9 +1494,7 @@ bool C_PREAMBLE::_prefill_polygon(RasterizerCanvas::Item::CommandPolygon *p_poly
 	}
 
 	BatchColor *vertex_colors = bdata.vertex_colors.request(num_inds);
-#if defined(TOOLS_ENABLED) && defined(DEBUG_ENABLED)
-	CRASH_COND(!vertex_colors);
-#endif
+	RAST_DEBUG_ASSERT(vertex_colors);
 
 	// are we using large FVF?
 	////////////////////////////////////
@@ -1500,9 +1504,7 @@ bool C_PREAMBLE::_prefill_polygon(RasterizerCanvas::Item::CommandPolygon *p_poly
 	BatchColor *vertex_modulates = nullptr;
 	if (use_modulate) {
 		vertex_modulates = bdata.vertex_modulates.request(num_inds);
-#if defined(TOOLS_ENABLED) && defined(DEBUG_ENABLED)
-		CRASH_COND(!vertex_modulates);
-#endif
+		RAST_DEBUG_ASSERT(vertex_modulates);
 		// precalc the vertex modulate (will be shared by all verts)
 		// we store the modulate as an attribute in the fvf rather than a uniform
 		vertex_modulates[0].set(r_fill_state.final_modulate);
@@ -1511,9 +1513,7 @@ bool C_PREAMBLE::_prefill_polygon(RasterizerCanvas::Item::CommandPolygon *p_poly
 	BatchTransform *pBT = nullptr;
 	if (use_large_verts) {
 		pBT = bdata.vertex_transforms.request(num_inds);
-#if defined(TOOLS_ENABLED) && defined(DEBUG_ENABLED)
-		CRASH_COND(!pBT);
-#endif
+		RAST_DEBUG_ASSERT(pBT);
 		// precalc the batch transform (will be shared by all verts)
 		// we store the transform as an attribute in the fvf rather than a uniform
 		const Transform2D &tr = r_fill_state.transform_combined;
@@ -1604,9 +1604,7 @@ bool C_PREAMBLE::_prefill_polygon(RasterizerCanvas::Item::CommandPolygon *p_poly
 		for (int n = 0; n < num_inds; n++) {
 			int ind = p_poly->indices[n];
 
-#if defined(TOOLS_ENABLED) && defined(DEBUG_ENABLED)
-			CRASH_COND(ind >= p_poly->points.size());
-#endif
+			RAST_DEV_DEBUG_ASSERT(ind < p_poly->points.size());
 
 			// this could be moved outside the loop
 			if (r_fill_state.transform_mode != TM_NONE) {
@@ -1710,9 +1708,7 @@ PREAMBLE(bool)::_software_skin_poly(RasterizerCanvas::Item::CommandPolygon *p_po
 
 				total_weight += weight;
 
-#if defined(TOOLS_ENABLED) && defined(DEBUG_ENABLED)
-				CRASH_COND(bone_id >= bone_count);
-#endif
+				RAST_DEBUG_ASSERT(bone_id < bone_count);
 				const Transform2D &bone_tr = bone_transforms[bone_id];
 
 				Vector2 pos = bone_tr.xform(src_pos_back_transformed);
@@ -1749,9 +1745,7 @@ PREAMBLE(bool)::_software_skin_poly(RasterizerCanvas::Item::CommandPolygon *p_po
 	for (int n = 0; n < num_inds; n++) {
 		int ind = p_poly->indices[n];
 
-#if defined(TOOLS_ENABLED) && defined(DEBUG_ENABLED)
-		CRASH_COND(ind >= num_verts);
-#endif
+		RAST_DEV_DEBUG_ASSERT(ind < num_verts);
 		const Point2 &pos = pTemps[ind];
 		bvs[n].pos.set(pos.x, pos.y);
 
@@ -2002,9 +1996,7 @@ bool C_PREAMBLE::_prefill_rect(RasterizerCanvas::Item::CommandRect *rect, FillSt
 	if (use_modulate) {
 		// store the final modulate separately from the rect modulate
 		BatchColor *pBC = bdata.vertex_modulates.request(4);
-#if defined(TOOLS_ENABLED) && defined(DEBUG_ENABLED)
-		CRASH_COND(pBC == nullptr);
-#endif
+		RAST_DEBUG_ASSERT(pBC);
 		pBC[0].set(r_fill_state.final_modulate);
 		pBC[1] = pBC[0];
 		pBC[2] = pBC[0];
@@ -2014,9 +2006,7 @@ bool C_PREAMBLE::_prefill_rect(RasterizerCanvas::Item::CommandRect *rect, FillSt
 	if (use_large_verts) {
 		// store the transform separately
 		BatchTransform *pBT = bdata.vertex_transforms.request(4);
-#if defined(TOOLS_ENABLED) && defined(DEBUG_ENABLED)
-		CRASH_COND(pBT == nullptr);
-#endif
+		RAST_DEBUG_ASSERT(pBT);
 
 		const Transform2D &tr = r_fill_state.transform_combined;
 
@@ -2035,9 +2025,7 @@ bool C_PREAMBLE::_prefill_rect(RasterizerCanvas::Item::CommandRect *rect, FillSt
 		// or sync them up during translation. We are syncing in translation.
 		// N.B. There may be batches that don't require light_angles between batches that do.
 		float *angles = bdata.light_angles.request(4);
-#if defined(TOOLS_ENABLED) && defined(DEBUG_ENABLED)
-		CRASH_COND(angles == nullptr);
-#endif
+		RAST_DEBUG_ASSERT(angles);
 
 		float angle = 0.0f;
 		const float TWO_PI = Math_PI * 2;
@@ -2375,6 +2363,11 @@ PREAMBLE(void)::flush_render_batches(RasterizerCanvas::Item *p_first_item, Raste
 
 	// if we overrode the fvf for lines, set it back to the joined item fvf
 	bdata.fvf = backup_fvf;
+
+	// overwrite source buffers with garbage if error checking
+#ifdef RASTERIZER_EXTRA_CHECKS
+	_debug_write_garbage();
+#endif
 }
 
 PREAMBLE(void)::render_joined_item_commands(const BItemJoined &p_bij, RasterizerCanvas::Item *p_current_clip, bool &r_reclip, typename T_STORAGE::Material *p_material, bool p_lit) {
@@ -2550,7 +2543,7 @@ PREAMBLE(void)::join_sorted_items() {
 			// but it is stupidly complex to calculate later, which would probably be slower.
 			r->final_modulate = _render_item_state.final_modulate;
 		} else {
-			CRASH_COND(_render_item_state.joined_item == 0);
+			RAST_DEBUG_ASSERT(_render_item_state.joined_item != 0);
 			_render_item_state.joined_item->num_item_refs += 1;
 			_render_item_state.joined_item->bounding_rect = _render_item_state.joined_item->bounding_rect.merge(ci->global_rect_cache);
 
@@ -2748,7 +2741,7 @@ PREAMBLE(void)::_translate_batches_to_vertex_colored_FVF() {
 	bdata.unit_vertices.prepare(sizeof(BatchVertexColored));
 
 	const BatchColor *source_vertex_colors = &bdata.vertex_colors[0];
-	CRASH_COND(bdata.vertex_colors.size() != bdata.vertices.size());
+	RAST_DEBUG_ASSERT(bdata.vertex_colors.size() == bdata.vertices.size());
 
 	int num_verts = bdata.vertices.size();
 
@@ -2789,10 +2782,8 @@ void C_PREAMBLE::_translate_batches_to_larger_FVF(uint32_t p_sequence_batch_type
 	// the sizes should be equal, and allocations should never fail. Hence the use of debug
 	// asserts to check program flow, these should not occur at runtime unless the allocation
 	// code has been altered.
-#if defined(TOOLS_ENABLED) && defined(DEBUG_ENABLED)
-	CRASH_COND(bdata.unit_vertices.max_size() != bdata.vertices.max_size());
-	CRASH_COND(bdata.batches_temp.max_size() != bdata.batches.max_size());
-#endif
+	RAST_DEBUG_ASSERT(bdata.unit_vertices.max_size() == bdata.vertices.max_size());
+	RAST_DEBUG_ASSERT(bdata.batches_temp.max_size() == bdata.batches.max_size());
 
 	Color curr_col(-1.0f, -1.0f, -1.0f, -1.0f);
 
@@ -2828,16 +2819,16 @@ void C_PREAMBLE::_translate_batches_to_larger_FVF(uint32_t p_sequence_batch_type
 						int end_vert = first_vert + (4 * source_batch.num_commands);
 
 						for (int v = first_vert; v < end_vert; v++) {
+							RAST_DEV_DEBUG_ASSERT(bdata.vertices.size());
 							const BatchVertex &bv = bdata.vertices[v];
 							BATCH_VERTEX_TYPE *cv = (BATCH_VERTEX_TYPE *)bdata.unit_vertices.request();
-#if defined(TOOLS_ENABLED) && defined(DEBUG_ENABLED)
-							CRASH_COND(!cv);
-#endif
+							RAST_DEBUG_ASSERT(cv);
 							cv->pos = bv.pos;
 							cv->uv = bv.uv;
 							cv->col = source_batch.color;
 
 							if (INCLUDE_LIGHT_ANGLES) {
+								RAST_DEV_DEBUG_ASSERT(bdata.light_angles.size());
 								// this is required to allow compilation with non light angle vertex.
 								// it should be compiled out.
 								BatchVertexLightAngled *lv = (BatchVertexLightAngled *)cv;
@@ -2848,11 +2839,13 @@ void C_PREAMBLE::_translate_batches_to_larger_FVF(uint32_t p_sequence_batch_type
 							} // if including light angles
 
 							if (INCLUDE_MODULATE) {
+								RAST_DEV_DEBUG_ASSERT(bdata.vertex_modulates.size());
 								BatchVertexModulated *mv = (BatchVertexModulated *)cv;
 								mv->modulate = *source_vertex_modulates++;
 							} // including modulate
 
 							if (INCLUDE_LARGE) {
+								RAST_DEV_DEBUG_ASSERT(bdata.vertex_transforms.size());
 								BatchVertexLarge *lv = (BatchVertexLarge *)cv;
 								lv->transform = *source_vertex_transforms++;
 							} // if including large
@@ -2874,9 +2867,7 @@ void C_PREAMBLE::_translate_batches_to_larger_FVF(uint32_t p_sequence_batch_type
 
 		if (needs_new_batch) {
 			dest_batch = bdata.batches_temp.request();
-#if defined(TOOLS_ENABLED) && defined(DEBUG_ENABLED)
-			CRASH_COND(!dest_batch);
-#endif
+			RAST_DEBUG_ASSERT(dest_batch);
 
 			*dest_batch = source_batch;
 
@@ -2888,11 +2879,10 @@ void C_PREAMBLE::_translate_batches_to_larger_FVF(uint32_t p_sequence_batch_type
 				int end_vert = first_vert + (4 * source_batch.num_commands);
 
 				for (int v = first_vert; v < end_vert; v++) {
+					RAST_DEV_DEBUG_ASSERT(bdata.vertices.size());
 					const BatchVertex &bv = bdata.vertices[v];
 					BATCH_VERTEX_TYPE *cv = (BATCH_VERTEX_TYPE *)bdata.unit_vertices.request();
-#if defined(TOOLS_ENABLED) && defined(DEBUG_ENABLED)
-					CRASH_COND(!cv);
-#endif
+					RAST_DEBUG_ASSERT(cv);
 					cv->pos = bv.pos;
 					cv->uv = bv.uv;
 
@@ -2900,10 +2890,12 @@ void C_PREAMBLE::_translate_batches_to_larger_FVF(uint32_t p_sequence_batch_type
 					if (!include_poly_color) {
 						cv->col = source_batch.color;
 					} else {
+						RAST_DEV_DEBUG_ASSERT(bdata.vertex_colors.size());
 						cv->col = *source_vertex_colors++;
 					}
 
 					if (INCLUDE_LIGHT_ANGLES) {
+						RAST_DEV_DEBUG_ASSERT(bdata.light_angles.size());
 						// this is required to allow compilation with non light angle vertex.
 						// it should be compiled out.
 						BatchVertexLightAngled *lv = (BatchVertexLightAngled *)cv;
@@ -2914,11 +2906,13 @@ void C_PREAMBLE::_translate_batches_to_larger_FVF(uint32_t p_sequence_batch_type
 					} // if using light angles
 
 					if (INCLUDE_MODULATE) {
+						RAST_DEV_DEBUG_ASSERT(bdata.vertex_modulates.size());
 						BatchVertexModulated *mv = (BatchVertexModulated *)cv;
 						mv->modulate = *source_vertex_modulates++;
 					} // including modulate
 
 					if (INCLUDE_LARGE) {
+						RAST_DEV_DEBUG_ASSERT(bdata.vertex_transforms.size());
 						BatchVertexLarge *lv = (BatchVertexLarge *)cv;
 						lv->transform = *source_vertex_transforms++;
 					} // if including large
@@ -2980,7 +2974,7 @@ PREAMBLE(bool)::_detect_item_batch_break(RenderItemState &r_ris, RasterizerCanva
 		for (int command_num = 0; command_num < command_count; command_num++) {
 
 			RasterizerCanvas::Item::Command *command = commands[command_num];
-			CRASH_COND(!command);
+			RAST_DEBUG_ASSERT(command);
 
 			switch (command->type) {
 
@@ -3051,3 +3045,5 @@ PREAMBLE(bool)::_detect_item_batch_break(RenderItemState &r_ris, RasterizerCanva
 #undef PREAMBLE
 #undef T_PREAMBLE
 #undef C_PREAMBLE
+
+#endif // RASTERIZER_CANVAS_BATCHER_H

drivers/gles_common/rasterizer_storage_common.h

@@ -28,7 +28,8 @@
 /* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.                */
 /*************************************************************************/
 
-#pragma once
+#ifndef RASTERIZER_STORAGE_COMMON_H
+#define RASTERIZER_STORAGE_COMMON_H
 
 class RasterizerStorageCommon {
 public:
@@ -72,3 +73,5 @@ public:
 		BTF_POLY = 1 << BT_POLY,
 	};
 };
+
+#endif // RASTERIZER_STORAGE_COMMON_H