From 94ed206bfc5b44cc84e91599f9881cbb98011ace Mon Sep 17 00:00:00 2001 From: Author Lawnjelly Date: Mon, 11 May 2020 21:19:34 +0200 Subject: [PATCH] GLES2 Batching - Prevent baking colors with COLOR writes Writing to COLOR in a custom shader can result in incorrect results if colors are baked (vertex color and modulate). This PR prevents baking with COLOR output, except under the special circumstances that final modulate is (1, 1, 1, 1), in which case the result will be correct. This should still allow color baking in many scenarios with custom shaders. --- drivers/gles2/rasterizer_canvas_gles2.cpp | 48 ++++++++++++++-------- drivers/gles2/rasterizer_canvas_gles2.h | 7 +++- drivers/gles2/rasterizer_storage_gles2.cpp | 6 +-- drivers/gles2/rasterizer_storage_gles2.h | 8 ++-- 4 files changed, 42 insertions(+), 27 deletions(-) diff --git a/drivers/gles2/rasterizer_canvas_gles2.cpp b/drivers/gles2/rasterizer_canvas_gles2.cpp index 37857cf98ce..c0708c564b1 100644 --- a/drivers/gles2/rasterizer_canvas_gles2.cpp +++ b/drivers/gles2/rasterizer_canvas_gles2.cpp @@ -1908,6 +1908,7 @@ void RasterizerCanvasGLES2::join_sorted_items() { _render_item_state.joined_item->num_item_refs = 1; _render_item_state.joined_item->bounding_rect = ci->global_rect_cache; _render_item_state.joined_item->z_index = z; + _render_item_state.joined_item->flags = bdata.joined_item_batch_flags; // add the reference BItemRef *r = bdata.item_refs.request_with_grow(); @@ -2184,7 +2185,8 @@ bool RasterizerCanvasGLES2::try_join_item(Item *p_ci, RenderItemState &r_ris, bo } // if there are any state changes we change join to false - // we also set r_batch_break to true if we don't want this item joined + // we also set r_batch_break to true if we don't want this item joined to the next + // (e.g. an item that must not be joined at all) r_batch_break = false; bool join = true; @@ -2271,17 +2273,6 @@ bool RasterizerCanvasGLES2::try_join_item(Item *p_ci, RenderItemState &r_ris, bo 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 should break the batching? - if (r_ris.shader_cache && !unshaded) { - 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; - r_batch_break = true; - } - } - // we are precalculating the final_modulate ahead of time because we need this for baking of final modulate into vertex colors // (only in software transform mode) // This maybe inefficient storing it... @@ -2292,6 +2283,33 @@ bool RasterizerCanvasGLES2::try_join_item(Item *p_ci, RenderItemState &r_ris, bo r_ris.last_blend_mode = blend_mode; } + // does the shader contain BUILTINs which should break the batching? + bdata.joined_item_batch_flags = 0; + if (r_ris.shader_cache && !unshaded) { + + unsigned int and_flags = r_ris.shader_cache->canvas_item.batch_flags & (RasterizerStorageGLES2::Shader::CanvasItem::PREVENT_COLOR_BAKING | RasterizerStorageGLES2::Shader::CanvasItem::PREVENT_VERTEX_BAKING); + if (and_flags) { + + bool break_batching = true; + + if (and_flags == RasterizerStorageGLES2::Shader::CanvasItem::PREVENT_COLOR_BAKING) { + // in some circumstances, if the modulate is identity, we still allow baking because reading modulate / color + // will still be okay to do in the shader with no ill effects + if (r_ris.final_modulate == Color(1, 1, 1, 1)) { + break_batching = false; + } + } + + if (break_batching) { + join = false; + r_batch_break = true; + + // save the flags so that they don't need to be recalculated in the 2nd pass + bdata.joined_item_batch_flags |= r_ris.shader_cache->canvas_item.batch_flags; + } + } + } + if ((blend_mode == RasterizerStorageGLES2::Shader::CanvasItem::BLEND_MODE_MIX || blend_mode == RasterizerStorageGLES2::Shader::CanvasItem::BLEND_MODE_PMALPHA) && r_ris.item_group_light && !unshaded) { // we cannot join lit items easily. @@ -3004,12 +3022,6 @@ 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 or vertex baking? - bdata.joined_item_batch_flags = 0; - if (r_ris.shader_cache && !unshaded) { - bdata.joined_item_batch_flags = r_ris.shader_cache->canvas_item.batch_flags; - } - if (r_ris.last_blend_mode != blend_mode) { switch (blend_mode) { diff --git a/drivers/gles2/rasterizer_canvas_gles2.h b/drivers/gles2/rasterizer_canvas_gles2.h index 3bfd1e9af4e..c7ed774663d 100644 --- a/drivers/gles2/rasterizer_canvas_gles2.h +++ b/drivers/gles2/rasterizer_canvas_gles2.h @@ -135,7 +135,10 @@ class RasterizerCanvasGLES2 : public RasterizerCanvasBaseGLES2 { // note the z_index may only be correct for the first of the joined item references // this has implications for light culling with z ranged lights. - int z_index; + int16_t z_index; + + // these are defined in RasterizerStorageGLES2::Shader::CanvasItem::BatchFlags + uint16_t flags; // we are always splitting items with lots of commands, // and items with unhandled primitives (default) @@ -205,7 +208,7 @@ class RasterizerCanvasGLES2 : public RasterizerCanvasBaseGLES2 { // 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; + uint32_t joined_item_batch_flags; // measured in pixels, recalculated each frame float scissor_threshold_area; diff --git a/drivers/gles2/rasterizer_storage_gles2.cpp b/drivers/gles2/rasterizer_storage_gles2.cpp index 6a2a2b7113b..f2a298d51e3 100644 --- a/drivers/gles2/rasterizer_storage_gles2.cpp +++ b/drivers/gles2/rasterizer_storage_gles2.cpp @@ -1425,7 +1425,7 @@ void RasterizerStorageGLES2::_update_shader(Shader *p_shader) const { p_shader->canvas_item.uses_screen_uv = false; 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.uses_color = false; p_shader->canvas_item.reads_vertex = false; p_shader->canvas_item.batch_flags = 0; @@ -1443,8 +1443,8 @@ void RasterizerStorageGLES2::_update_shader(Shader *p_shader) const { shaders.actions_canvas.usage_flag_pointers["SCREEN_TEXTURE"] = &p_shader->canvas_item.uses_screen_texture; shaders.actions_canvas.usage_flag_pointers["TIME"] = &p_shader->canvas_item.uses_time; shaders.actions_canvas.usage_flag_pointers["MODULATE"] = &p_shader->canvas_item.uses_modulate; + shaders.actions_canvas.usage_flag_pointers["COLOR"] = &p_shader->canvas_item.uses_color; - 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; @@ -1535,7 +1535,7 @@ void RasterizerStorageGLES2::_update_shader(Shader *p_shader) const { // some logic for batching if (p_shader->mode == VS::SHADER_CANVAS_ITEM) { - if (p_shader->canvas_item.uses_modulate | p_shader->canvas_item.reads_color) { + if (p_shader->canvas_item.uses_modulate | p_shader->canvas_item.uses_color) { p_shader->canvas_item.batch_flags |= Shader::CanvasItem::PREVENT_COLOR_BAKING; } if (p_shader->canvas_item.reads_vertex) { diff --git a/drivers/gles2/rasterizer_storage_gles2.h b/drivers/gles2/rasterizer_storage_gles2.h index f85461593d6..f27e979928c 100644 --- a/drivers/gles2/rasterizer_storage_gles2.h +++ b/drivers/gles2/rasterizer_storage_gles2.h @@ -447,20 +447,20 @@ public: int light_mode; + // 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 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; + bool uses_color; bool reads_vertex; } canvas_item;