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 +33 -17
5 changed files with 33 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit ad07bd6931 - Show all commits

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

@ -64,11 +64,7 @@ void OVERLAY_edit_mesh_cache_init(OVERLAY_Data *vedata)
RegionView3D *rv3d = draw_ctx->rv3d;
bool show_retopology = (v3d->overlay.edit_flag & V3D_OVERLAY_EDIT_RETOPOLOGY) != 0;
/* Add epsilon to ensure the value is never zero when the effect is enabled. */
float retopology_bias = (show_retopology) ?
((rv3d->is_persp) ?
(v3d->overlay.retopology_bias * v3d->clip_start + FLT_EPSILON) :
(v3d->overlay.retopology_bias / v3d->clip_end + FLT_EPSILON)) :
0.0f;
float retopology_bias = (show_retopology) ? (v3d->overlay.retopology_bias + FLT_EPSILON) : 0.0f;
bonj marked this conversation as resolved
Jeroen Bakker commented 2023-02-28 04:00:22 +01:00
Review
Copy Link

Just a small tweak.
Use max_ff(v3d->overlay.retopology_offset, FLT_EPSILON) and remove comment as this is then cleaner in the code.

Just a small tweak. Use `max_ff(v3d->overlay.retopology_offset, FLT_EPSILON)` and remove comment as this is then cleaner in the code.
Jorijn de Graaf commented 2023-02-28 04:22:46 +01:00
Review
Copy Link

Makes sense, I'll do that.

Makes sense, I'll do that.
pd->edit_mesh.do_faces = true;
pd->edit_mesh.do_edges = true;

5
source/blender/draw/engines/overlay/shaders/overlay_edit_mesh_depth_vert.glsl
View File

@ -7,10 +7,11 @@ void main()
GPU_INTEL_VERTEX_SHADER_WORKAROUND
vec3 world_pos = point_object_to_world(pos);
gl_Position = point_world_to_ndc(world_pos);
vec3 view_pos = point_world_to_view(world_pos);
gl_Position = point_view_to_ndc(view_pos);
/* Offset Z position for retopology overlay. */
gl_Position.z -= retopologyBias / abs(gl_Position.w);
gl_Position.z -= get_homogenous_z_offset(view_pos.z, gl_Position.w, retopologyBias);
view_clipping_distances(world_pos);
}

9
source/blender/draw/engines/overlay/shaders/overlay_edit_mesh_vert.glsl
View File

@ -29,7 +29,11 @@ void main()
GPU_INTEL_VERTEX_SHADER_WORKAROUND
vec3 world_pos = point_object_to_world(pos);
gl_Position = point_world_to_ndc(world_pos);
vec3 view_pos = point_world_to_view(world_pos);
gl_Position = point_view_to_ndc(view_pos);
/* Offset Z position for retopology overlay. */
gl_Position.z -= get_homogenous_z_offset(view_pos.z, gl_Position.w, retopologyBias);
uvec4 m_data = data & uvec4(dataMask);
@ -95,8 +99,5 @@ void main()
finalColor.rgb = non_linear_blend_color(colorEditMeshMiddle.rgb, finalColor.rgb, facing);
bonj marked this conversation as resolved
Clément Foucault commented 2023-02-14 02:20:36 +01:00
Review
Copy Link

I am pretty sure the bias is incorrect.

I think it should be somethign like gl_Position.z -= retopologyBias * abs(gl_Position.w);.

Here your bias decrease with the distance (or increases the closer you are) which is quite weird.
I will have to double check that.

I am pretty sure the bias is incorrect. I think it should be somethign like `gl_Position.z -= retopologyBias * abs(gl_Position.w);`. Here your bias decrease with the distance (or increases the closer you are) which is quite weird. I will have to double check that.
Jorijn de Graaf commented 2023-02-14 14:17:00 +01:00
Review
Copy Link

It's counter-intuitive, but dividing seems to be the right approach here.
If we multiply by W, the effective offset changes in an undesirable way when zooming in or out.
You need very different bias values to get the same result at different view distances.
When not using W at all, it's not as bad, but still has issues compared to dividing by W.
I don't really know how to describe it, so you might have to test for yourself.

It's counter-intuitive, but dividing seems to be the right approach here. If we multiply by W, the effective offset changes in an undesirable way when zooming in or out. You need very different bias values to get the same result at different view distances. When not using W at all, it's not as bad, but still has issues compared to dividing by W. I don't really know how to describe it, so you might have to test for yourself.
Jorijn de Graaf commented 2023-02-15 23:38:25 +01:00
Review
Copy Link

Perhaps it's important to note how the bias is calculated.

float retopology_bias = (show_retopology) ?
                            ((rv3d->is_persp) ?
                                 (v3d->overlay.retopology_bias * v3d->clip_start + FLT_EPSILON) :
                                 (v3d->overlay.retopology_bias / v3d->clip_end + FLT_EPSILON)) :
                            0.0f;
Perhaps it's important to note how the bias is calculated. ```cpp float retopology_bias = (show_retopology) ? ((rv3d->is_persp) ? (v3d->overlay.retopology_bias * v3d->clip_start + FLT_EPSILON) : (v3d->overlay.retopology_bias / v3d->clip_end + FLT_EPSILON)) : 0.0f; ```
Jorijn de Graaf commented 2023-02-22 00:27:42 +01:00
Review
Copy Link

I have not been able to make it look correct with multiplying by W instead of dividing.
Since it works correctly as is, I'm going to consider this resolved.

I have not been able to make it look correct with multiplying by W instead of dividing. Since it works correctly as is, I'm going to consider this resolved.
#endif
/* Offset Z position for retopology overlay. */
gl_Position.z -= retopologyBias / abs(gl_Position.w);
view_clipping_distances(world_pos);
}

15
source/blender/draw/engines/overlay/shaders/overlay_edit_mesh_vert_no_geom.glsl
View File

@ -82,8 +82,15 @@ void main()
vec3 world_pos0 = point_object_to_world(in_pos0);
vec3 world_pos1 = point_object_to_world(in_pos1);
vec4 out_pos0 = point_world_to_ndc(world_pos0);
vec4 out_pos1 = point_world_to_ndc(world_pos1);
vec3 view_pos0 = point_world_to_view(world_pos0);
vec3 view_pos1 = point_world_to_view(world_pos1);
vec4 out_pos0 = point_view_to_ndc(view_pos0);
vec4 out_pos1 = point_view_to_ndc(view_pos1);
/* Offset Z position for retopology overlay. */
out_pos0.z -= get_homogenous_z_offset(view_pos0.z, out_pos0.w, retopologyBias);
out_pos1.z -= get_homogenous_z_offset(view_pos1.z, out_pos1.w, retopologyBias);
uvec4 m_data0 = uvec4(in_data0) & uvec4(dataMask);
uvec4 m_data1 = uvec4(in_data1) & uvec4(dataMask);
@ -143,10 +150,6 @@ void main()
colorEditMeshMiddle.rgb, out_finalColor[1].rgb, facing1);
#endif
/* Offset Z position for retopology overlay. */
out_pos0.z -= retopologyBias / abs(out_pos0.w);
out_pos1.z -= retopologyBias / abs(out_pos1.w);
// -------- GEOM SHADER ALTERNATIVE ----------- //
vec2 ss_pos[2];

15
source/blender/draw/intern/shaders/common_view_lib.glsl
View File

@ -257,6 +257,21 @@ vec3 point_world_to_view(vec3 p)
return (ViewMatrix * vec4(p, 1.0)).xyz;
}
/* From "Projection Matrix Tricks" by Eric Lengyel:
* http://www.terathon.com/gdc07_lengyel.pdf (p. 18 Depth Modification)
bonj marked this conversation as resolved
Clément Foucault commented 2023-02-26 18:38:07 +01:00
Review
Copy Link

My bad. The equation is actually comming from page 24.

My bad. The equation is actually comming from page 24.
Jorijn de Graaf commented 2023-02-26 19:35:55 +01:00
Review
Copy Link

I thought you picked 18 because it's the start of the section about depth modification. I think it makes sense to point to the whole range of pages (18-26).

I thought you picked 18 because it's the start of the section about depth modification. I think it makes sense to point to the whole range of pages (18-26).
Clément Foucault commented 2023-02-26 21:55:56 +01:00
Review
Copy Link

Yes, but here it is the reference to a specific formula. I also think this reference should be inside the function. Comment on top of function is only for documentation of the function usage.

Yes, but here it is the reference to a specific formula. I also think this reference should be inside the function. Comment on top of function is only for documentation of the function usage.
Jorijn de Graaf commented 2023-02-26 22:35:19 +01:00
Review
Copy Link

Done.

Done.
*
* View Z is used to adjust for perspective projection.
* Homogenous W is used to convert from NDC to homogenous space. */
float get_homogenous_z_offset(float vs_z, float hs_w, float offset)
bonj marked this conversation as resolved
Clément Foucault commented 2023-02-26 18:45:55 +01:00
Review
Copy Link

Maybe also add a note that offset is in viewspace. So positive values are closer to camera.

Maybe also add a note that offset is in viewspace. So positive values are closer to camera.
Jorijn de Graaf commented 2023-02-27 00:33:49 +01:00
Review
Copy Link

Done, now each parameter has an explanation.

Done, now each parameter has an explanation.
{
if (ProjectionMatrix[3][3] == 0.0) {
return -ProjectionMatrix[3][2] * (offset / (vs_z * (vs_z + offset))) * hs_w;
bonj marked this conversation as resolved
Clément Foucault commented 2023-02-26 18:39:10 +01:00
Review
Copy Link

Remove the negative sign here and everywhere get_homogenous_z_offset is called.
The negative sign in the equation from the slide is inside ProjectionMatrix[3][2].

Remove the negative sign here and everywhere `get_homogenous_z_offset` is called. The negative sign in the equation from the slide is inside `ProjectionMatrix[3][2]`.
Jorijn de Graaf commented 2023-02-27 00:35:48 +01:00
Review
Copy Link

Done and done.
I negated it because then you can subtract from the Z, which is what other offsets in the shader do, but yeah it was unnecessary so now they just use += for this.

Done and done. I negated it because then you can subtract from the Z, which is what other offsets in the shader do, but yeah it was unnecessary so now they just use += for this.
}
else {
return -ProjectionMatrix[2][2] * offset * hs_w;
}
}
/* Due to some shader compiler bug, we somewhat need to access gl_VertexID
* to make vertex shaders work. even if it's actually dead code. */
#if defined(GPU_INTEL) && defined(GPU_OPENGL)