Details

Reviewers

Clément Foucault (fclem)
Campbell Barton (campbellbarton)

Commits

rBS681661dbed12: GPU: Simplify select shaders.
rB681661dbed12: GPU: Simplify select shaders.

Summary

This patch allows the drawing of the mesh selection ids to be done on a texture with 32UI format.
This simplifies the shader that previously acted on the backbuffer and had to do an uint to rgba conversion.
The size of framebuber and glViewport is now limited to the size of the threshold.

TODO:
The threshold keeps changing between selecting one element type for another, for better optimization, maintaining the same threshold would avoid having to re-create the framebuffer textures.

Diff Detail

Repository: rB Blender

Event Timeline

Germano Cavalcante (mano-wii) created this revision.Feb 13 2019, 6:15 PM

Harbormaster completed remote builds in B2918: Diff 13660.Feb 13 2019, 6:15 PM

Patch stopped working for some reason.

Fix stupid mistake

Harbormaster completed remote builds in B2919: Diff 13661.Feb 13 2019, 7:06 PM

Rename functions that erroneously consider reading cache.

Harbormaster completed remote builds in B2924: Diff 13666.Feb 13 2019, 10:40 PM

cleanup: unsigned int -> uint and identation;
Fix crash when edit particles.

Harbormaster completed remote builds in B2925: Diff 13667.Feb 13 2019, 11:34 PM

Generally looks like the right direction, although @Clément Foucault (fclem) knows these details better, running some tests though.

Weight/vertex paint vertex select has an offset.

Default cube
Subdivide 4 times
Enter weight paint mode.
VKey
BKey to select vertices.
Notice it's not working properly (rectangle is offset).

Otherwise this patch seems to work well.

source/blender/editors/space_view3d/view3d_draw_legacy.c
371	doesn't compile on Linux, use `min_ii()`
420	Not returning anything (shouldn't this show as a compiler warning/error?)

Campbell Barton (campbellbarton) requested changes to this revision.Feb 15 2019, 6:49 AM

This revision now requires changes to proceed.Feb 15 2019, 6:49 AM

Fix warnings and cleanup;
Make read_nearest work a bit better with rectangles;
Use Odd Thresholds;
Fix weight/vertex paint vertex selection;

Harbormaster completed remote builds in B2943: Diff 13696.Feb 15 2019, 3:58 PM

Germano Cavalcante (mano-wii) marked 2 inline comments as done.Feb 15 2019, 4:03 PM

Germano Cavalcante (mano-wii) added inline comments.

source/blender/editors/space_view3d/view3d_draw_legacy.c
420	In fact some warnings appeared. I have not noticed because in Visual Studio (except for py and glsl) the warnings only appear the first time the file is modified. I have to edit the settings to ignore warnings of GLSL and Python codes. I will be more careful next time.

Campbell Barton (campbellbarton) mentioned this in T61373: Often crashes in edit mode.Feb 18 2019, 7:04 AM

Dmitrii (WeaselOnAStick) added a subscriber: Dmitrii (WeaselOnAStick).Feb 18 2019, 8:07 AM

Fix use of uninitialized variables, move rect expanding into validate_object_select_id

Harbormaster completed remote builds in B2947: Diff 13718.Feb 18 2019, 9:21 AM

Checking over this patch, it looks close to being ready, and issue with it is V3D_INVALID_BACKBUF is no longer meaningful because each request is a different size.

Could it be configured to update the entire texture but only read a small region - as it does for existing code?

This revision now requires changes to proceed.Feb 18 2019, 9:26 AM

In D4350#99111, @Campbell Barton (campbellbarton) wrote:

Checking over this patch, it looks close to being ready, and issue with it is V3D_INVALID_BACKBUF is no longer meaningful because each request is a different size.

Could it be configured to update the entire texture but only read a small region - as it does for existing code?

I made some local edits, but I'm not sure if keeping a framebuffer with Blender's screen-size cached is advantageous.
It would be less efficient and would also occupy much more space in Vram.

(I can try another solution by removing the use of V3D_INVALID_BACKBUF and maintaining a shared context between functions).

@Germano Cavalcante (mano-wii) keeping a buffer means that for example - when moving the cursor it doesn't need to redraw on every mouse-move that requests the selection result.

Unless there is a good reason we should remove this - it would be nice to finish this patch off without changing behavior of how the result is cached.

In D4350#100707, @Campbell Barton (campbellbarton) wrote:

@Germano Cavalcante (mano-wii) keeping a buffer means that for example - when moving the cursor it doesn't need to redraw on every mouse-move that requests the selection result.

Yes, I agree with @Campbell Barton (campbellbarton). Keeping 8MB of data (for a 1080p fullscreen viewport) seems reasonable. However I'm not sure whether we should keep it in Video Memory or System Memory.

update to master;

Harbormaster completed remote builds in B3031: Diff 13958.Mar 1 2019, 10:26 PM

remove unused RegionView3D::gpuoffscreen;
move the select framebuffer to the viewport struct.

Harbormaster completed remote builds in B3032: Diff 13959.Mar 2 2019, 1:50 AM

There are still some things to fix. Like the circle selection.
I'll investigate better later.

Cleanup and use dist instead of size;
Pass mval as a parameter of ED_view3d_select_id_read_nearest;
Move select id utilities to GPU_viewport.

Harbormaster completed remote builds in B3033: Diff 13968.Mar 2 2019, 4:17 PM

Fix Select Circle.

Harbormaster completed remote builds in B3034: Diff 13969.Mar 2 2019, 4:49 PM

Campbell Barton (campbellbarton) accepted this revision.Mar 2 2019, 4:51 PM

This revision is now accepted and ready to land.Mar 2 2019, 4:51 PM

Cleanup comments;
Remove GPU_select_index_set, GPU_select_to_index_array, GPU_select_to_index, GPU_select_to_index_array and GPU_color_depth.

Harbormaster completed remote builds in B3035: Diff 13970.Mar 2 2019, 6:06 PM

Fix select Lasso.

Harbormaster completed remote builds in B3036: Diff 13971.Mar 2 2019, 6:53 PM

Dmitrii (WeaselOnAStick) removed a subscriber: Dmitrii (WeaselOnAStick).Mar 3 2019, 5:18 AM

Dmitrii (WeaselOnAStick) added a subscriber: Dmitrii (WeaselOnAStick).

GPU_texture: Add a new utility function to set a new texture size.
Move the functions to configure the select_id framebuffer back to the DRW_manager.

Harbormaster completed remote builds in B3045: Diff 13994.Mar 5 2019, 3:29 AM

Make sure that the depth_only framebuffer is created before the select_id.

Harbormaster completed remote builds in B3046: Diff 13995.Mar 5 2019, 3:45 AM

Clément Foucault (fclem) requested changes to this revision.Mar 5 2019, 4:54 AM

Clément Foucault (fclem) added inline comments.

source/blender/draw/intern/draw_manager.c
2047–2049	What's the point in this function? If you want to resize a texture just discard and recreate it. Adding new functions to the GPU API for something used only in one place (and that can be done without it) for no real performance gain is not really a good idea. If you doubt about the speed at which the driver allocates textures, resizing the viewport discard all GPU textures from all the TextureLists and it's still very fluid. Making the GPU API more complex means that we will have more work to do when porting to vulkan/other API. Also just as a note, I would prefer GPU_texture_new_size() to be named GPU_texture_size_set().

This revision now requires changes to proceed.Mar 5 2019, 4:54 AM

Germano Cavalcante (mano-wii) added inline comments.Mar 5 2019, 2:28 PM

source/blender/draw/intern/draw_manager.c
2047–2049	Even considering the possibility of changing the rendering API, I recognize that I know well the other APIs to make the decision to create this function (I'll remove). However, this change was not due to the peformance of the OpenGL functions, but rather to the complexity of the code, since two framebuffers share the same texture. So if the pointer of a texture is invalidated, it has to be attached in both framebuffers, forcing thus the creation of both even if the second does not need to be used. This creates a bit of duplicate code and confuses a bit. But it really is not an essential change.

Revert "GPU_texture: Add a new utility function to set a new texture size."
Recreate texture instead of resize.

Harbormaster completed remote builds in B3053: Diff 14004.Mar 5 2019, 2:29 PM

Germano Cavalcante (mano-wii) marked an inline comment as done.Mar 5 2019, 2:31 PM

@Clément Foucault (fclem) / @Germano Cavalcante (mano-wii) all issues addressed now?

Update comments.

Harbormaster completed remote builds in B3082: Diff 14072.Mar 8 2019, 1:42 PM

Clément Foucault (fclem) requested changes to this revision.Mar 8 2019, 2:22 PM

Clément Foucault (fclem) added inline comments.

source/blender/draw/intern/draw_manager.c
2526	This is a state change that is done on the main context, not the DRW context. Just remove the line. Any use of glReadPixels should bind the corect buffer to glReadBuffer beforehand.

This revision now requires changes to proceed.Mar 8 2019, 2:22 PM

Remove wrong status change (glReadBuffer).

Harbormaster completed remote builds in B3084: Diff 14074.Mar 8 2019, 2:37 PM

Germano Cavalcante (mano-wii) marked an inline comment as done.Mar 8 2019, 2:43 PM

Disable RV3D_CLIPPING after use.

Harbormaster completed remote builds in B3086: Diff 14076.Mar 8 2019, 2:58 PM

Clément Foucault (fclem) accepted this revision.Mar 15 2019, 1:45 AM

This revision is now accepted and ready to land.Mar 15 2019, 1:45 AM

Closed by commit rB681661dbed12: GPU: Simplify select shaders. (authored by Germano Cavalcante (mano-wii)). · Explain WhyMar 15 2019, 9:03 PM

This revision was automatically updated to reflect the committed changes.

Simplify `GPU_SHADER_3D_FLAT_SELECT_ID` and `GPU_SHADER_3D_UNIFORM_SELECT_ID` shaders.
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 14205

source/blender/blenloader/intern/readfile.c

source/blender/draw/DRW_engine.h

source/blender/draw/intern/draw_manager.c

source/blender/editors/include/ED_mesh.h

source/blender/editors/include/ED_view3d.h

source/blender/editors/mesh/editface.c

source/blender/editors/mesh/editmesh_select.c

source/blender/editors/mesh/meshtools.c

source/blender/editors/physics/particle_edit.c

source/blender/editors/sculpt_paint/paint_utils.c

source/blender/editors/sculpt_paint/paint_vertex_weight_ops.c

source/blender/editors/space_view3d/drawobject.c

source/blender/editors/space_view3d/space_view3d.c

source/blender/editors/space_view3d/view3d_draw_legacy.c

source/blender/editors/space_view3d/view3d_intern.h

source/blender/editors/space_view3d/view3d_select.c

source/blender/gpu/GPU_draw.h

source/blender/gpu/GPU_extensions.h

source/blender/gpu/intern/gpu_draw.c

source/blender/gpu/intern/gpu_extensions.c

source/blender/gpu/shaders/gpu_shader_3D_selection_id_vert.glsl

source/blender/gpu/shaders/gpu_shader_selection_id_frag.glsl

source/blender/makesdna/DNA_view3d_types.h

Simplify `GPU_SHADER_3D_FLAT_SELECT_ID` and `GPU_SHADER_3D_UNIFORM_SELECT_ID` shaders.ClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 14205

source/blender/blenloader/intern/readfile.c

source/blender/draw/DRW_engine.h

source/blender/draw/intern/draw_manager.c

source/blender/editors/include/ED_mesh.h

source/blender/editors/include/ED_view3d.h

source/blender/editors/mesh/editface.c

source/blender/editors/mesh/editmesh_select.c

source/blender/editors/mesh/meshtools.c

source/blender/editors/physics/particle_edit.c

source/blender/editors/sculpt_paint/paint_utils.c

source/blender/editors/sculpt_paint/paint_vertex_weight_ops.c

source/blender/editors/space_view3d/drawobject.c

source/blender/editors/space_view3d/space_view3d.c

source/blender/editors/space_view3d/view3d_draw_legacy.c

source/blender/editors/space_view3d/view3d_intern.h

source/blender/editors/space_view3d/view3d_select.c

source/blender/gpu/GPU_draw.h

source/blender/gpu/GPU_extensions.h

source/blender/gpu/intern/gpu_draw.c

source/blender/gpu/intern/gpu_extensions.c

source/blender/gpu/shaders/gpu_shader_3D_selection_id_vert.glsl

source/blender/gpu/shaders/gpu_shader_selection_id_frag.glsl

source/blender/makesdna/DNA_view3d_types.h

Simplify `GPU_SHADER_3D_FLAT_SELECT_ID` and `GPU_SHADER_3D_UNIFORM_SELECT_ID` shaders.
ClosedPublic
Actions

Revision Contents
Changeset List