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

#70267 Retopology Overlay #104599

Merged
Clément Foucault merged 30 commits from bonj/blender:retopology-overlay into main 2023-03-03 00:35:56 +01:00
Conversation 39 Commits 30 Files Changed 25 +32 -7
8 changed files with 32 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 6f3ed2f125 - Show all commits

2
source/blender/draw/CMakeLists.txt
View File

@ -638,6 +638,7 @@ set(GLSL_SRC
engines/overlay/shaders/overlay_edit_mesh_analysis_frag.glsl
engines/overlay/shaders/overlay_edit_mesh_analysis_vert.glsl
engines/overlay/shaders/overlay_edit_mesh_common_lib.glsl
engines/overlay/shaders/overlay_edit_mesh_depth_vert.glsl
engines/overlay/shaders/overlay_edit_mesh_frag.glsl
engines/overlay/shaders/overlay_edit_mesh_geom.glsl
engines/overlay/shaders/overlay_edit_mesh_normal_vert.glsl
@ -708,7 +709,6 @@ set(GLSL_SRC
engines/overlay/shaders/overlay_sculpt_mask_frag.glsl
engines/overlay/shaders/overlay_sculpt_mask_vert.glsl
engines/overlay/shaders/overlay_uniform_color_frag.glsl
engines/overlay/shaders/overlay_uniform_color_vert.glsl
engines/overlay/shaders/overlay_varying_color.glsl
engines/overlay/shaders/overlay_viewer_attribute_curve_vert.glsl
engines/overlay/shaders/overlay_viewer_attribute_curves_vert.glsl

2
source/blender/draw/engines/overlay/overlay_edit_mesh.cc
View File

@ -112,7 +112,7 @@ void OVERLAY_edit_mesh_cache_init(OVERLAY_Data *vedata)
((show_retopology) ? DRWState(0) : DRW_STATE_CULL_BACK);
DRW_PASS_CREATE(psl->edit_mesh_depth_ps[i], state | pd->clipping_state);
sh = OVERLAY_shader_depth_only();
sh = OVERLAY_shader_edit_mesh_depth();
grp = pd->edit_mesh_depth_grp[i] = DRW_shgroup_create(sh, psl->edit_mesh_depth_ps[i]);
DRW_shgroup_uniform_float_copy(grp, "retopologyBias", retopology_bias);
}

1
source/blender/draw/engines/overlay/overlay_private.hh
View File

@ -730,6 +730,7 @@ GPUShader *OVERLAY_shader_edit_gpencil_wire(void);
GPUShader *OVERLAY_shader_edit_lattice_point(void);
GPUShader *OVERLAY_shader_edit_lattice_wire(void);
GPUShader *OVERLAY_shader_edit_mesh_analysis(void);
GPUShader *OVERLAY_shader_edit_mesh_depth(void);
GPUShader *OVERLAY_shader_edit_mesh_edge(bool use_flat_interp);
GPUShader *OVERLAY_shader_edit_mesh_face(void);
GPUShader *OVERLAY_shader_edit_mesh_facedot(void);

12
source/blender/draw/engines/overlay/overlay_shader.cc
View File

@ -153,6 +153,18 @@ GPUShader *OVERLAY_shader_depth_only(void)
return sh_data->depth_only;
}
GPUShader *OVERLAY_shader_edit_mesh_depth(void)
{
const DRWContextState *draw_ctx = DRW_context_state_get();
OVERLAY_Shaders *sh_data = &e_data.sh_data[draw_ctx->sh_cfg];
if (!sh_data->depth_only) {
sh_data->depth_only = GPU_shader_create_from_info_name(
(draw_ctx->sh_cfg == GPU_SHADER_CFG_CLIPPED) ? "overlay_edit_mesh_depth_clipped" :
"overlay_edit_mesh_depth");
}
return sh_data->depth_only;
}
GPUShader *OVERLAY_shader_edit_mesh_vert(void)
{
const DRWContextState *draw_ctx = DRW_context_state_get();

15
source/blender/draw/engines/overlay/shaders/infos/overlay_edit_mode_info.hh
View File

@ -38,6 +38,18 @@ GPU_SHADER_CREATE_INFO(overlay_edit_mesh_common_no_geom)
.additional_info("draw_modelmat", "draw_globals");
#endif
GPU_SHADER_CREATE_INFO(overlay_edit_mesh_depth)
.do_static_compilation(true)
.vertex_in(0, Type::VEC3, "pos")
.push_constant(Type::FLOAT, "retopologyBias")
.vertex_source("overlay_edit_mesh_depth_vert.glsl")
.fragment_source("overlay_depth_only_frag.glsl")
.additional_info("draw_mesh");
GPU_SHADER_CREATE_INFO(overlay_edit_mesh_depth_clipped)
.do_static_compilation(true)
.additional_info("overlay_edit_mesh_depth", "drw_clipped");
GPU_SHADER_INTERFACE_INFO(overlay_edit_mesh_vert_iface, "")
.smooth(Type::VEC4, "finalColor")
.smooth(Type::FLOAT, "vertexCrease");
@ -599,7 +611,6 @@ GPU_SHADER_CREATE_INFO(overlay_edit_gpencil_guide_point_clipped)
GPU_SHADER_CREATE_INFO(overlay_depth_only)
.do_static_compilation(true)
.vertex_in(0, Type::VEC3, "pos")
.push_constant(Type::FLOAT, "retopologyBias")
.vertex_source("overlay_depth_only_vert.glsl")
.fragment_source("overlay_depth_only_frag.glsl")
.additional_info("draw_mesh");
@ -619,7 +630,7 @@ GPU_SHADER_CREATE_INFO(overlay_uniform_color)
.vertex_in(0, Type::VEC3, "pos")
.push_constant(Type::VEC4, "ucolor")
.fragment_out(0, Type::VEC4, "fragColor")
.vertex_source("overlay_uniform_color_vert.glsl")
.vertex_source("overlay_depth_only_vert.glsl")
.fragment_source("overlay_uniform_color_frag.glsl")
.additional_info("draw_mesh");

3
source/blender/draw/engines/overlay/shaders/overlay_depth_only_vert.glsl
View File

@ -9,8 +9,5 @@ void main()
vec3 world_pos = point_object_to_world(pos);
gl_Position = point_world_to_ndc(world_pos);
/* Offset Z position for retopology overlay. */
gl_Position.z -= retopologyBias / abs(gl_Position.w);
view_clipping_distances(world_pos);
bonj marked this conversation as resolved
Clément Foucault commented 2023-02-14 02:14:36 +01:00
Review
Copy Link

You edit a file that is used in other part of the engine. Maybe it is better to duplicate the shader as overlay_edit_mesh_depth_vert.glsl.

You edit a file that is used in other part of the engine. Maybe it is better to duplicate the shader as `overlay_edit_mesh_depth_vert.glsl`.
Jorijn de Graaf commented 2023-02-14 14:00:14 +01:00
Review
Copy Link

Previously it was like this: overlay_uniform_color and overlay_depth_only use overlay_depth_only_vert.glsl
Now it is like this: overlay_uniform_color uses overlay_uniform_color_vert.glsl, and overlay_depth_only uses overlay_depth_only_vert.glsl
What you're proposing is like this: overlay_uniform_color uses overlay_depth_only_vert.glsl, and overlay_depth_only uses overlay_edit_mesh_depth_vert.glsl
These glsl files are only used by these shaders, not anywhere else.
I think it makes a lot more sense to name the files after the shaders that use them.

Previously it was like this: overlay_uniform_color and overlay_depth_only use overlay_depth_only_vert.glsl Now it is like this: overlay_uniform_color uses overlay_uniform_color_vert.glsl, and overlay_depth_only uses overlay_depth_only_vert.glsl What you're proposing is like this: overlay_uniform_color uses overlay_depth_only_vert.glsl, and overlay_depth_only uses overlay_edit_mesh_depth_vert.glsl These glsl files are only used by these shaders, not anywhere else. I think it makes a lot more sense to name the files after the shaders that use them.
Clément Foucault commented 2023-02-14 19:21:28 +01:00
Review
Copy Link

Yes, but I'm suggesting to rename the overlay_depth_only to overlay_edit_mesh_depth_only too.

Changing the uniform color shader is a cleanup think and should be committed separately.

Yes, but I'm suggesting to rename the `overlay_depth_only` to `overlay_edit_mesh_depth_only` too. Changing the uniform color shader is a cleanup think and should be committed separately.
Jorijn de Graaf commented 2023-02-14 19:59:59 +01:00
Review
Copy Link

Oh you mean renaming the shader itself, as well as its vertex and fragment shaders, to overlay_edit_mesh_depth_only(_vert/frag.glsl)?
And then reverting overlay_uniform_color to how it was, letting someone else deal with it another time?
I suppose I can do that yeah.
There was one change I made that I still think we shouldn't revert, which is that the clipping version of overlay_uniform_color inherited from the depth only shader, which I'm fairly certain must've been a mistake, because all other clipping variants of shaders do inherit how they should.

Oh you mean renaming the shader itself, as well as its vertex and fragment shaders, to overlay_edit_mesh_depth_only(\_vert/frag.glsl)? And then reverting overlay_uniform_color to how it was, letting someone else deal with it another time? I suppose I can do that yeah. There was one change I made that I still think we shouldn't revert, which is that the clipping version of overlay_uniform_color inherited from the depth only shader, which I'm fairly certain must've been a mistake, because all other clipping variants of shaders do inherit how they should.
Clément Foucault commented 2023-02-14 20:45:01 +01:00
Review
Copy Link

This should also be in another commit. You can create another PR for these two thing (only one PR for both).

This should also be in another commit. You can create another PR for these two thing (only one PR for both).
Jorijn de Graaf commented 2023-02-14 23:00:05 +01:00
Review
Copy Link

After thinking about it for a long time, I understand now.
The depth_only shader is used in several places I was unaware of, so I will revert it to its original state, and do the same for uniform_color.
I will then add a new shader specifically for the depth pass in mesh edit mode, which will get its own vertex shader (it can reuse the depth_only fragment shader though).

After thinking about it for a long time, I understand now. The depth_only shader is used in several places I was unaware of, so I will revert it to its original state, and do the same for uniform_color. I will then add a new shader specifically for the depth pass in mesh edit mode, which will get its own vertex shader (it can reuse the depth_only fragment shader though).
Jorijn de Graaf commented 2023-02-14 23:25:35 +01:00
Review
Copy Link

Alright I think this should be resolved now, so I'll mark it as such.

Alright I think this should be resolved now, so I'll mark it as such.
}
bonj marked this conversation as resolved
Clément Foucault commented 2023-02-14 02:11:05 +01:00
Review
Copy Link

Move the max out of the shader. That stands for all shaders.

Move the max out of the shader. That stands for all shaders.
Jorijn de Graaf commented 2023-02-14 02:53:57 +01:00
Review
Copy Link

This is necessary because the value is -1.0 when the checkbox is disabled.
That is how the shader knows whether to use the regular face color or the retopology face color.
Alternatively I could add an extra shader constant, but I was told to get rid of that, hence the current solution.
I guess a third solution would be to make the minimum value something like 0.001, then check for 0.0 to determine face color; I would not prefer this approach though.

This is necessary because the value is -1.0 when the checkbox is disabled. That is how the shader knows whether to use the regular face color or the retopology face color. Alternatively I could add an extra shader constant, but I was told to get rid of that, hence the current solution. I guess a third solution would be to make the minimum value something like 0.001, then check for 0.0 to determine face color; I would not prefer this approach though.
Clément Foucault commented 2023-02-14 13:05:19 +01:00
Review
Copy Link

minimum value something like 0.001

You can use FLT_MIN for that. I'm sure the difference with 0.0 will not be noticeable.

> minimum value something like 0.001 You can use `FLT_MIN` for that. I'm sure the difference with 0.0 will not be noticeable.
Jorijn de Graaf commented 2023-02-14 13:43:03 +01:00
Review
Copy Link

That's a very large negative value.
I could use FLT_EPSILON though, perhaps not as minimum for the property, but that I add it before sending the value to the shader, to make sure it's never 0 when the effect is enabled.
Edit: Did it, I think it's a good solution.

That's a very large negative value. I could use FLT_EPSILON though, perhaps not as minimum for the property, but that I add it before sending the value to the shader, to make sure it's never 0 when the effect is enabled. Edit: Did it, I think it's a good solution.

3
source/blender/draw/engines/overlay/shaders/overlay_uniform_color_vert.glsl → source/blender/draw/engines/overlay/shaders/overlay_edit_mesh_depth_vert.glsl
View File

@ -9,5 +9,8 @@ void main()
vec3 world_pos = point_object_to_world(pos);
gl_Position = point_world_to_ndc(world_pos);
/* Offset Z position for retopology overlay. */
gl_Position.z -= retopologyBias / abs(gl_Position.w);
view_clipping_distances(world_pos);
}

1
source/blender/draw/tests/shaders_test.cc
View File

@ -219,6 +219,7 @@ static void test_overlay_glsl_shaders()
EXPECT_NE(OVERLAY_shader_edit_lattice_point(), nullptr);
EXPECT_NE(OVERLAY_shader_edit_lattice_wire(), nullptr);
EXPECT_NE(OVERLAY_shader_edit_mesh_analysis(), nullptr);
EXPECT_NE(OVERLAY_shader_edit_mesh_depth(), nullptr);
EXPECT_NE(OVERLAY_shader_edit_mesh_edge(false), nullptr);
EXPECT_NE(OVERLAY_shader_edit_mesh_edge(true), nullptr);
EXPECT_NE(OVERLAY_shader_edit_mesh_face(), nullptr);