blender
/
blender
136
407
Fork 589
Code Issues 6.3k Pull Requests 767 Projects 19 Wiki Activity

Fix #115460: EEVEE-Next: Missing bind when drawing curves #121064

Merged
Jeroen Bakker merged 8 commits from Jeroen-Bakker/blender:eevee/fix/missing-curve-texture-bind into main 2024-05-17 11:16:05 +02:00
Conversation 6 Commits 8 Files Changed 2 +132 -13
2 changed files with 132 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

105
source/blender/draw/intern/draw_cache_impl_curves.cc
View File

@ -681,26 +681,111 @@ static bool ensure_attributes(const Curves &curves,
CurvesEvalFinalCache &final_cache = cache.eval_cache.final;
if (gpu_material) {
/* The following code should be kept in sync with `mesh_cd_calc_used_gpu_layers`. */
Jeroen-Bakker marked this conversation as resolved Outdated
Clément Foucault commented 2024-05-16 11:49:02 +02:00
Outdated
Review
Copy Link

Next code

Are you refering to following code or future code? This is ambiguous.

I guess you mean The following code.

> Next code Are you refering to following code or future code? This is ambiguous. I guess you mean `The following code`.
DRW_Attributes attrs_needed;
drw_attributes_clear(&attrs_needed);
ListBase gpu_attrs = GPU_material_attributes(gpu_material);
LISTBASE_FOREACH (const GPUMaterialAttribute *, gpu_attr, &gpu_attrs) {
const char *name = gpu_attr->name;
eCustomDataType type = static_cast<eCustomDataType>(gpu_attr->type);
int layer = -1;
std::optional<bke::AttrDomain> domain;
int layer_index;
eCustomDataType type;
bke::AttrDomain domain;
if (drw_custom_data_match_attribute(cd_curve, name, &layer_index, &type)) {
domain = bke::AttrDomain::Curve;
}
else if (drw_custom_data_match_attribute(cd_point, name, &layer_index, &type)) {
domain = bke::AttrDomain::Point;
if (gpu_attr->type == CD_AUTO_FROM_NAME) {
Jeroen-Bakker marked this conversation as resolved Outdated
Clément Foucault commented 2024-05-10 16:19:57 +02:00
Outdated
Review
Copy Link

This function should either be a copy of mesh_cd_calc_used_gpu_layers or be unified.

This function should either be a copy of `mesh_cd_calc_used_gpu_layers` or be unified.
Jeroen Bakker commented 2024-05-16 08:27:35 +02:00
Outdated
Review
Copy Link

Return types are different (bool vs used), data values are different (DRW_MeshCDMask vs DRW_Attributes) CD_MTFACE vs CD_PROP_FLOAT2) data structure storage is different, logic is different. So would most likely be a copy and removing parts that are not needed. Unifying would change the impact in areas that are not well documented.

Would rather see this working, before restructuring the code to be more unified.
Already copying the same logic is a challenge.

Return types are different (bool vs used), data values are different (DRW_MeshCDMask vs DRW_Attributes) CD_MTFACE vs CD_PROP_FLOAT2) data structure storage is different, logic is different. So would most likely be a copy and removing parts that are not needed. Unifying would change the impact in areas that are not well documented. Would rather see this working, before restructuring the code to be more unified. Already copying the same logic is a challenge.
/* We need to deduce what exact layer is used.
*
* We do it based on the specified name.
*/
if (name[0] != '\0') {
layer = CustomData_get_named_layer(cd_curve, CD_PROP_FLOAT2, name);
type = CD_MTFACE;
domain = bke::AttrDomain::Curve;
if (layer == -1) {
/* Try to match a generic attribute, we use the first attribute domain with a
* matching name. */
if (drw_custom_data_match_attribute(cd_point, name, &layer, &type)) {
domain = bke::AttrDomain::Point;
}
else if (drw_custom_data_match_attribute(cd_curve, name, &layer, &type)) {
domain = bke::AttrDomain::Curve;
}
else {
domain.reset();
layer = -1;
}
}
if (layer == -1) {
continue;
}
}
else {
/* Fall back to the UV layer, which matches old behavior. */
type = CD_MTFACE;
}
}
else {
continue;
if (drw_custom_data_match_attribute(cd_curve, name, &layer, &type)) {
domain = bke::AttrDomain::Curve;
}
else if (drw_custom_data_match_attribute(cd_point, name, &layer, &type)) {
domain = bke::AttrDomain::Point;
}
}
drw_attributes_add_request(&attrs_needed, name, type, layer_index, domain);
switch (type) {
case CD_MTFACE: {
if (layer == -1) {
layer = (name[0] != '\0') ?
CustomData_get_named_layer(cd_curve, CD_PROP_FLOAT2, name) :
CustomData_get_render_layer(cd_curve, CD_PROP_FLOAT2);
if (layer != -1) {
domain = bke::AttrDomain::Curve;
}
}
if (layer == -1) {
layer = (name[0] != '\0') ?
CustomData_get_named_layer(cd_point, CD_PROP_FLOAT2, name) :
CustomData_get_render_layer(cd_point, CD_PROP_FLOAT2);
if (layer != -1) {
domain = bke::AttrDomain::Point;
}
}
if (layer != -1 && name[0] == '\0' && domain.has_value()) {
name = CustomData_get_layer_name(
domain == bke::AttrDomain::Curve ? cd_curve : cd_point, CD_PROP_FLOAT2, layer);
}
if (layer != -1 && domain.has_value()) {
drw_attributes_add_request(&attrs_needed, name, CD_PROP_FLOAT2, layer, *domain);
}
break;
}
case CD_TANGENT:
case CD_ORCO:
break;
case CD_PROP_BYTE_COLOR:
case CD_PROP_COLOR:
case CD_PROP_QUATERNION:
case CD_PROP_FLOAT3:
case CD_PROP_BOOL:
case CD_PROP_INT8:
case CD_PROP_INT32:
case CD_PROP_INT32_2D:
case CD_PROP_FLOAT:
case CD_PROP_FLOAT2: {
if (layer != -1 && domain.has_value()) {
drw_attributes_add_request(&attrs_needed, name, type, layer, *domain);
}
break;
}
default:
break;
}
}
if (!drw_attributes_overlap(&final_cache.attr_used, &attrs_needed)) {

40
source/blender/draw/intern/draw_curves.cc
View File

@ -15,6 +15,7 @@
#include "BKE_attribute.hh"
#include "BKE_curves.hh"
#include "BKE_customdata.hh"
#include "GPU_batch.hh"
#include "GPU_capabilities.hh"
@ -235,6 +236,7 @@ DRWShadingGroup *DRW_shgroup_curves_create_sub(Object *object,
DRW_shgroup_buffer_texture(shgrp, "au", g_dummy_vbo);
DRW_shgroup_buffer_texture(shgrp, "c", g_dummy_vbo);
DRW_shgroup_buffer_texture(shgrp, "ac", g_dummy_vbo);
DRW_shgroup_buffer_texture(shgrp, "a", g_dummy_vbo);
/* TODO: Generalize radius implementation for curves data type. */
float hair_rad_shape = 0.0f;
@ -265,10 +267,20 @@ DRWShadingGroup *DRW_shgroup_curves_create_sub(Object *object,
DRW_shgroup_buffer_texture(shgrp, "hairLen", curves_cache->proc_length_buf);
}
int curve_data_render_uv = 0;
int point_data_render_uv = 0;
if (CustomData_has_layer(&curves_id.geometry.curve_data, CD_PROP_FLOAT2)) {
curve_data_render_uv = CustomData_get_render_layer(&curves_id.geometry.curve_data,
Jeroen-Bakker marked this conversation as resolved Outdated
Clément Foucault commented 2024-05-16 11:51:50 +02:00
Outdated
Review
Copy Link

Look up how particle code does it, you should also support active color and such:


  if (psmd != nullptr && psmd->mesh_final != nullptr) {
    if (CustomData_has_layer(&psmd->mesh_final->corner_data, CD_PROP_FLOAT2)) {
      cache->num_uv_layers = CustomData_number_of_layers(&psmd->mesh_final->corner_data,
                                                         CD_PROP_FLOAT2);
      active_uv = CustomData_get_active_layer(&psmd->mesh_final->corner_data, CD_PROP_FLOAT2);
      render_uv = CustomData_get_render_layer(&psmd->mesh_final->corner_data, CD_PROP_FLOAT2);
    }
    if (CustomData_has_layer(&psmd->mesh_final->corner_data, CD_PROP_BYTE_COLOR)) {
      cache->num_col_layers = CustomData_number_of_layers(&psmd->mesh_final->corner_data,
                                                          CD_PROP_BYTE_COLOR);
      if (psmd->mesh_final->active_color_attribute != nullptr) {
        active_col = CustomData_get_named_layer(&psmd->mesh_final->corner_data,
                                                CD_PROP_BYTE_COLOR,
                                                psmd->mesh_final->active_color_attribute);
      }
      if (psmd->mesh_final->default_color_attribute != nullptr) {
        render_col = CustomData_get_named_layer(&psmd->mesh_final->corner_data,
                                                CD_PROP_BYTE_COLOR,
                                                psmd->mesh_final->default_color_attribute);
      }
    }
  }
Look up how particle code does it, you should also support active color and such: ```Cpp if (psmd != nullptr && psmd->mesh_final != nullptr) { if (CustomData_has_layer(&psmd->mesh_final->corner_data, CD_PROP_FLOAT2)) { cache->num_uv_layers = CustomData_number_of_layers(&psmd->mesh_final->corner_data, CD_PROP_FLOAT2); active_uv = CustomData_get_active_layer(&psmd->mesh_final->corner_data, CD_PROP_FLOAT2); render_uv = CustomData_get_render_layer(&psmd->mesh_final->corner_data, CD_PROP_FLOAT2); } if (CustomData_has_layer(&psmd->mesh_final->corner_data, CD_PROP_BYTE_COLOR)) { cache->num_col_layers = CustomData_number_of_layers(&psmd->mesh_final->corner_data, CD_PROP_BYTE_COLOR); if (psmd->mesh_final->active_color_attribute != nullptr) { active_col = CustomData_get_named_layer(&psmd->mesh_final->corner_data, CD_PROP_BYTE_COLOR, psmd->mesh_final->active_color_attribute); } if (psmd->mesh_final->default_color_attribute != nullptr) { render_col = CustomData_get_named_layer(&psmd->mesh_final->corner_data, CD_PROP_BYTE_COLOR, psmd->mesh_final->default_color_attribute); } } }
Hans Goudey commented 2024-05-16 14:44:05 +02:00
Outdated
Review
Copy Link

There is no active/default color attribute for curves, curves don't have color attributes currently.

There is no active/default color attribute for curves, curves don't have color attributes currently.
👍 2
Clément Foucault commented 2024-05-16 14:49:05 +02:00
Outdated
Review
Copy Link

Ok then i'm fine with the current code. But at least move CustomData_get_render_layer out of the loop.

Ok then i'm fine with the current code. But at least move `CustomData_get_render_layer` out of the loop.
CD_PROP_FLOAT2);
Jeroen-Bakker marked this conversation as resolved Outdated
Clément Foucault commented 2024-05-10 16:43:12 +02:00
Outdated
Review
Copy Link

Loop over the name, don't choose one or the other.

Loop over the name, don't choose one or the other.
}
if (CustomData_has_layer(&curves_id.geometry.point_data, CD_PROP_FLOAT2)) {
point_data_render_uv = CustomData_get_render_layer(&curves_id.geometry.point_data,
CD_PROP_FLOAT2);
}
const DRW_Attributes &attrs = curves_cache->final.attr_used;
for (int i = 0; i < attrs.num_requests; i++) {
const DRW_AttributeRequest &request = attrs.requests[i];
char sampler_name[32];
drw_curves_get_attribute_sampler_name(request.attribute_name, sampler_name);
@ -276,14 +288,19 @@ DRWShadingGroup *DRW_shgroup_curves_create_sub(Object *object,
if (!curves_cache->proc_attributes_buf[i]) {
continue;
}
DRW_shgroup_buffer_texture(shgrp, sampler_name, curves_cache->proc_attributes_buf[i]);
if (request.cd_type == CD_PROP_FLOAT2 && request.layer_index == curve_data_render_uv) {
DRW_shgroup_buffer_texture(shgrp, "a", curves_cache->proc_attributes_buf[i]);
}
}
else {
if (!curves_cache->final.attributes_buf[i]) {
continue;
}
DRW_shgroup_buffer_texture(shgrp, sampler_name, curves_cache->final.attributes_buf[i]);
if (request.cd_type == CD_PROP_FLOAT2 && request.layer_index == point_data_render_uv) {
DRW_shgroup_buffer_texture(shgrp, "a", curves_cache->final.attributes_buf[i]);
}
}
/* Some attributes may not be used in the shader anymore and were not garbage collected yet, so
@ -462,6 +479,7 @@ gpu::Batch *curves_sub_pass_setup_implementation(PassT &sub_ps,
sub_ps.bind_texture("au", g_dummy_vbo);
sub_ps.bind_texture("c", g_dummy_vbo);
sub_ps.bind_texture("ac", g_dummy_vbo);
sub_ps.bind_texture("a", g_dummy_vbo);
/* TODO: Generalize radius implementation for curves data type. */
float hair_rad_shape = 0.0f;
@ -492,10 +510,20 @@ gpu::Batch *curves_sub_pass_setup_implementation(PassT &sub_ps,
sub_ps.bind_texture("hairLen", curves_cache->proc_length_buf);
}
int curve_data_render_uv = 0;
int point_data_render_uv = 0;
if (CustomData_has_layer(&curves_id.geometry.curve_data, CD_PROP_FLOAT2)) {
curve_data_render_uv = CustomData_get_render_layer(&curves_id.geometry.curve_data,
CD_PROP_FLOAT2);
}
if (CustomData_has_layer(&curves_id.geometry.point_data, CD_PROP_FLOAT2)) {
point_data_render_uv = CustomData_get_render_layer(&curves_id.geometry.point_data,
CD_PROP_FLOAT2);
}
const DRW_Attributes &attrs = curves_cache->final.attr_used;
for (int i = 0; i < attrs.num_requests; i++) {
const DRW_AttributeRequest &request = attrs.requests[i];
char sampler_name[32];
drw_curves_get_attribute_sampler_name(request.attribute_name, sampler_name);
@ -504,12 +532,18 @@ gpu::Batch *curves_sub_pass_setup_implementation(PassT &sub_ps,
continue;
}
sub_ps.bind_texture(sampler_name, curves_cache->proc_attributes_buf[i]);
if (request.cd_type == CD_PROP_FLOAT2 && request.layer_index == curve_data_render_uv) {
sub_ps.bind_texture("a", curves_cache->proc_attributes_buf[i]);
}
}
else {
if (!curves_cache->final.attributes_buf[i]) {
continue;
}
sub_ps.bind_texture(sampler_name, curves_cache->final.attributes_buf[i]);
if (request.cd_type == CD_PROP_FLOAT2 && request.layer_index == point_data_render_uv) {
sub_ps.bind_texture("a", curves_cache->final.attributes_buf[i]);
}
}
/* Some attributes may not be used in the shader anymore and were not garbage collected yet, so