From d096ce16440c958956c9866d457dfd83c76f057e Mon Sep 17 00:00:00 2001 From: lawnjelly Date: Mon, 11 May 2020 15:46:31 +0100 Subject: [PATCH 1/2] GLES2 Batching - Fix custom shaders reading VERTEX In situations where custom canvas shaders read VERTEX values, they could read incorrect global positions rather than local positions where batching had baked item transforms. This PR prevents joining items that read VERTEX built in in shaders, and thus prevents this situation arising (as unjoined items will not bake transforms). --- drivers/gles2/rasterizer_canvas_gles2.cpp | 5 +++-- drivers/gles2/rasterizer_storage_gles2.cpp | 11 +++++++++-- drivers/gles2/rasterizer_storage_gles2.h | 15 ++++++++++----- 3 files changed, 22 insertions(+), 9 deletions(-) diff --git a/drivers/gles2/rasterizer_canvas_gles2.cpp b/drivers/gles2/rasterizer_canvas_gles2.cpp index 5baafaff037..eacb0b5579a 100644 --- a/drivers/gles2/rasterizer_canvas_gles2.cpp +++ b/drivers/gles2/rasterizer_canvas_gles2.cpp @@ -2267,7 +2267,8 @@ bool RasterizerCanvasGLES2::try_join_item(Item *p_ci, RenderItemState &r_ris, bo // does the shader contain BUILTINs which should break the batching? if (r_ris.shader_cache && !unshaded) { - if (r_ris.shader_cache->canvas_item.prevent_color_baking) { + const unsigned int test_flags = RasterizerStorageGLES2::Shader::CanvasItem::PREVENT_COLOR_BAKING | RasterizerStorageGLES2::Shader::CanvasItem::PREVENT_VERTEX_BAKING; + if (r_ris.shader_cache->canvas_item.batch_flags & test_flags) { // we will do this same test on the shader during the rendering pass in order to set a bool not to bake vertex colors // instead of saving this info as it is cheap to calculate join = false; @@ -3000,7 +3001,7 @@ void RasterizerCanvasGLES2::render_joined_item(const BItemJoined &p_bij, RenderI // does the shader contain BUILTINs which break the batching and should prevent color baking? bdata.prevent_color_baking = false; if (r_ris.shader_cache && !unshaded) { - if (r_ris.shader_cache->canvas_item.prevent_color_baking) { + if (r_ris.shader_cache->canvas_item.batch_flags & RasterizerStorageGLES2::Shader::CanvasItem::PREVENT_COLOR_BAKING) { bdata.prevent_color_baking = true; } } diff --git a/drivers/gles2/rasterizer_storage_gles2.cpp b/drivers/gles2/rasterizer_storage_gles2.cpp index 1ca7cc30281..6a2a2b7113b 100644 --- a/drivers/gles2/rasterizer_storage_gles2.cpp +++ b/drivers/gles2/rasterizer_storage_gles2.cpp @@ -1426,7 +1426,8 @@ void RasterizerStorageGLES2::_update_shader(Shader *p_shader) const { p_shader->canvas_item.uses_time = false; p_shader->canvas_item.uses_modulate = false; p_shader->canvas_item.reads_color = false; - p_shader->canvas_item.prevent_color_baking = false; + p_shader->canvas_item.reads_vertex = false; + p_shader->canvas_item.batch_flags = 0; shaders.actions_canvas.render_mode_values["blend_add"] = Pair(&p_shader->canvas_item.blend_mode, Shader::CanvasItem::BLEND_MODE_ADD); shaders.actions_canvas.render_mode_values["blend_mix"] = Pair(&p_shader->canvas_item.blend_mode, Shader::CanvasItem::BLEND_MODE_MIX); @@ -1444,6 +1445,7 @@ void RasterizerStorageGLES2::_update_shader(Shader *p_shader) const { shaders.actions_canvas.usage_flag_pointers["MODULATE"] = &p_shader->canvas_item.uses_modulate; shaders.actions_canvas.read_flag_pointers["COLOR"] = &p_shader->canvas_item.reads_color; + shaders.actions_canvas.read_flag_pointers["VERTEX"] = &p_shader->canvas_item.reads_vertex; actions = &shaders.actions_canvas; actions->uniforms = &p_shader->uniforms; @@ -1533,7 +1535,12 @@ void RasterizerStorageGLES2::_update_shader(Shader *p_shader) const { // some logic for batching if (p_shader->mode == VS::SHADER_CANVAS_ITEM) { - p_shader->canvas_item.prevent_color_baking = p_shader->canvas_item.uses_modulate | p_shader->canvas_item.reads_color; + if (p_shader->canvas_item.uses_modulate | p_shader->canvas_item.reads_color) { + p_shader->canvas_item.batch_flags |= Shader::CanvasItem::PREVENT_COLOR_BAKING; + } + if (p_shader->canvas_item.reads_vertex) { + p_shader->canvas_item.batch_flags |= Shader::CanvasItem::PREVENT_VERTEX_BAKING; + } } p_shader->shader->set_custom_shader(p_shader->custom_code_id); diff --git a/drivers/gles2/rasterizer_storage_gles2.h b/drivers/gles2/rasterizer_storage_gles2.h index 32e1d9bbdaa..f85461593d6 100644 --- a/drivers/gles2/rasterizer_storage_gles2.h +++ b/drivers/gles2/rasterizer_storage_gles2.h @@ -447,16 +447,21 @@ public: int light_mode; + enum BatchFlags { + PREVENT_COLOR_BAKING = 1 << 0, + PREVENT_VERTEX_BAKING = 1 << 1, + }; + // these flags are specifically for batching + // some of the logic is thus in rasterizer_storage.cpp + // we could alternatively set bitflags for each 'uses' and test on the fly + unsigned int batch_flags; + bool uses_screen_texture; bool uses_screen_uv; bool uses_time; bool uses_modulate; bool reads_color; - - // this flag is specifically for batching - // some of the logic is thus in rasterizer_storage.cpp - // we could alternatively set some bitflags here and test on the fly - bool prevent_color_baking; + bool reads_vertex; } canvas_item; From 57c70d8e9c16f3020b6704abef44e5dfee55e0f9 Mon Sep 17 00:00:00 2001 From: lawnjelly Date: Mon, 11 May 2020 17:18:57 +0100 Subject: [PATCH 2/2] GLES2 Batching - Prevent VERTEX baking within items in custom shaders In addition to prevent item joins when VERTEX reads are present in a custom shader, it is also necessary to prevent baking extra matrices (extra transforms) WITHIN items, because these can also report incorrect results. --- drivers/gles2/rasterizer_canvas_gles2.cpp | 18 +++++++++++------- drivers/gles2/rasterizer_canvas_gles2.h | 9 ++++++--- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/drivers/gles2/rasterizer_canvas_gles2.cpp b/drivers/gles2/rasterizer_canvas_gles2.cpp index eacb0b5579a..37857cf98ce 100644 --- a/drivers/gles2/rasterizer_canvas_gles2.cpp +++ b/drivers/gles2/rasterizer_canvas_gles2.cpp @@ -60,7 +60,7 @@ RasterizerCanvasGLES2::BatchData::BatchData() { settings_colored_vertex_format_threshold = 0.0f; settings_batch_buffer_num_verts = 0; scissor_threshold_area = 0.0f; - prevent_color_baking = false; + joined_item_batch_flags = 0; diagnose_frame = false; next_diagnose_tick = 10000; diagnose_frame_number = 9999999999; // some high number @@ -1629,6 +1629,12 @@ void RasterizerCanvasGLES2::render_joined_item_commands(const BItemJoined &p_bij fill_state.use_hardware_transform = p_bij.use_hardware_transform(); fill_state.extra_matrix_sent = false; + // in the special case of custom shaders that read from VERTEX (i.e. vertex position) + // we want to disable software transform of extra matrix + if (bdata.joined_item_batch_flags & RasterizerStorageGLES2::Shader::CanvasItem::PREVENT_VERTEX_BAKING) { + fill_state.extra_matrix_sent = true; + } + for (unsigned int i = 0; i < p_bij.num_item_refs; i++) { const BItemRef &ref = bdata.item_refs[p_bij.first_item_ref + i]; item = ref.item; @@ -1688,7 +1694,7 @@ void RasterizerCanvasGLES2::flush_render_batches(Item *p_first_item, Item *p_cur // only check whether to convert if there are quads (prevent divide by zero) // and we haven't decided to prevent color baking (due to e.g. MODULATE // being used in a shader) - if (bdata.total_quads && !bdata.prevent_color_baking) { + if (bdata.total_quads && !(bdata.joined_item_batch_flags & RasterizerStorageGLES2::Shader::CanvasItem::PREVENT_COLOR_BAKING)) { // minus 1 to prevent single primitives (ratio 1.0) always being converted to colored.. // in that case it is slightly cheaper to just have the color as part of the batch float ratio = (float)(bdata.total_color_changes - 1) / (float)bdata.total_quads; @@ -2998,12 +3004,10 @@ void RasterizerCanvasGLES2::render_joined_item(const BItemJoined &p_bij, RenderI bool unshaded = r_ris.shader_cache && (r_ris.shader_cache->canvas_item.light_mode == RasterizerStorageGLES2::Shader::CanvasItem::LIGHT_MODE_UNSHADED || (blend_mode != RasterizerStorageGLES2::Shader::CanvasItem::BLEND_MODE_MIX && blend_mode != RasterizerStorageGLES2::Shader::CanvasItem::BLEND_MODE_PMALPHA)); bool reclip = false; - // does the shader contain BUILTINs which break the batching and should prevent color baking? - bdata.prevent_color_baking = false; + // does the shader contain BUILTINs which break the batching and should prevent color baking or vertex baking? + bdata.joined_item_batch_flags = 0; if (r_ris.shader_cache && !unshaded) { - if (r_ris.shader_cache->canvas_item.batch_flags & RasterizerStorageGLES2::Shader::CanvasItem::PREVENT_COLOR_BAKING) { - bdata.prevent_color_baking = true; - } + bdata.joined_item_batch_flags = r_ris.shader_cache->canvas_item.batch_flags; } if (r_ris.last_blend_mode != blend_mode) { diff --git a/drivers/gles2/rasterizer_canvas_gles2.h b/drivers/gles2/rasterizer_canvas_gles2.h index 3494b564f6c..3bfd1e9af4e 100644 --- a/drivers/gles2/rasterizer_canvas_gles2.h +++ b/drivers/gles2/rasterizer_canvas_gles2.h @@ -200,9 +200,12 @@ class RasterizerCanvasGLES2 : public RasterizerCanvasBaseGLES2 { // to alternate batching method and add color to the vertex format. int total_color_changes; - // if the shader is using MODULATE, we prevent baking so the final_modulate can - // be read in the shader - bool prevent_color_baking; + // if the shader is using MODULATE, we prevent baking color so the final_modulate can + // be read in the shader. + // if the shader is reading VERTEX, we prevent baking vertex positions with extra matrices etc + // to prevent the read position being incorrect. + // These flags are defined in RasterizerStorageGLES2::Shader::CanvasItem::BatchFlags + unsigned int joined_item_batch_flags; // measured in pixels, recalculated each frame float scissor_threshold_area;