From 4ae797b802197a73a5dad226c5748d2102665782 Mon Sep 17 00:00:00 2001 From: lawnjelly Date: Fri, 20 Nov 2020 10:19:24 +0000 Subject: [PATCH] Batching - more error checking options This adds some more error checking options for development, and simplifies the control flow checks which now become debug asserts. --- .../gles3/rasterizer_canvas_base_gles3.cpp | 8 ++ drivers/gles_common/batch_diagnose.inc | 43 +++++++++ drivers/gles_common/rasterizer_array.h | 6 +- drivers/gles_common/rasterizer_asserts.h | 58 ++++++++++++ .../gles_common/rasterizer_canvas_batcher.h | 90 +++++++++---------- .../gles_common/rasterizer_storage_common.h | 5 +- 6 files changed, 161 insertions(+), 49 deletions(-) create mode 100644 drivers/gles_common/rasterizer_asserts.h diff --git a/drivers/gles3/rasterizer_canvas_base_gles3.cpp b/drivers/gles3/rasterizer_canvas_base_gles3.cpp index 7f80a25869..af5b51d743 100644 --- a/drivers/gles3/rasterizer_canvas_base_gles3.cpp +++ b/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); diff --git a/drivers/gles_common/batch_diagnose.inc b/drivers/gles_common/batch_diagnose.inc index 46b0a72825..54ecf66744 100644 --- a/drivers/gles_common/batch_diagnose.inc +++ b/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 = ""; diff --git a/drivers/gles_common/rasterizer_array.h b/drivers/gles_common/rasterizer_array.h index 13360ba23b..503146fb5d 100644 --- a/drivers/gles_common/rasterizer_array.h +++ b/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 diff --git a/drivers/gles_common/rasterizer_asserts.h b/drivers/gles_common/rasterizer_asserts.h new file mode 100644 index 0000000000..d2aae6becc --- /dev/null +++ b/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 diff --git a/drivers/gles_common/rasterizer_canvas_batcher.h b/drivers/gles_common/rasterizer_canvas_batcher.h index 4f38abd459..811b9ecf27 100644 --- a/drivers/gles_common/rasterizer_canvas_batcher.h +++ b/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 diff --git a/drivers/gles_common/rasterizer_storage_common.h b/drivers/gles_common/rasterizer_storage_common.h index aa81f2c260..97afaf2cae 100644 --- a/drivers/gles_common/rasterizer_storage_common.h +++ b/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