Page MenuHome

Transform: Use OpenGL for snapping
Needs ReviewPublic

Authored by Germano Cavalcante (mano-wii) on Aug 17 2017, 7:24 AM.

Details

Summary

The idea of ​​this patch is to draw the elements used to snap with flat colors (for indices 32UI) in a occult framebuffer. And use the glReadPixel to find the nearest element.

The snap to faces continues to use raycast, so this mode is not affected by the patch.

The main purpose of using OpenGL is to be able to occlude the elements not seen through the Z-Buffer.

Other benefits include:

  • Simplify the code;
  • enable multiple elements to snap efficiently and correctly (snap to faces, edges and vertices at the same time);

I tried using the Gawain API and GPU to create Batchs, but I was very limited to using builtin shaders and matrices. also, meshes for the snap-in edit-mode must ignore the sealed or hidden elements.

I also could not use the GPU_frambuffer API because it only works with floats (which is incompatible with glReadPixel for uints).

Since Blender 2.8 is still going to take a long time to get released, I don't find it problematic to apply this patch for now (I'll have time to edit and fix bugs - if the users find one).

Note: The Macro: DRAW_FRAMEBUFFER_ON_SCREEN s enabled. This causes you to see a drawing that should not exist (used for Debug). Something like:

Diff Detail

Repository
rB Blender

Event Timeline

I think this really should be using the GPU_* API for shaders and the framebuffer. If there is something missing the APIs can be extended?

As I mentioned in T52173#448178, I thought that we wanted to move away from OpenGL for selection, and this seems to be going in the opposite direction. Maybe there are good reasons, but can you explain what the downsides of this would be? Could there be issues with missed snaps due to there not being enough pixels to contain all the verts/edges/faces, could there be precision issues due to limited resolution Z buffer or is the snapping always recomputed using the actual vertex coordinates, does it work correctly with multisample AA, ... ? Are there cases where taking occlusion into account is not what you want, is there an option to disable it like for selection?

I got OpenGL errors and a crash cancelling the transform:

GPUShader: compile error:
ERROR: 0:5: Invalid interpolation qualifiers 'flat' in global variable context
ERROR: 0:5: Use of undeclared identifier 'outColor'

GPUShader: compile error:
ERROR: 0:5: '+' does not operate on 'unsigned int' and 'int'
0   org.blenderfoundation.blender 	0x00000001047d0499 GPU_shader_free + 9 (gpu_shader.c:472)
1   org.blenderfoundation.blender 	0x0000000104380d39 ED_transform_snap_object_context_destroy + 121 (transform_snap_object.c:2287)
2   org.blenderfoundation.blender 	0x000000010437e8ea freeSnapping + 26 (transform_snap.c:666)
3   org.blenderfoundation.blender 	0x0000000104352720 transformEnd + 128 (transform.c:647)
4   org.blenderfoundation.blender 	0x000000010437a128 transform_modal + 120 (transform_ops.c:408)
5   org.blenderfoundation.blender 	0x0000000104203016 wm_handler_operator_call + 534 (wm_event_system.c:1775)
Germano Cavalcante (mano-wii) retitled this revision from Transform: Use OpenGL to snap to Transform: Use OpenGL for snapping.EditedAug 17 2017, 6:08 PM
Germano Cavalcante (mano-wii) updated this revision to Diff 9141.

I think this really should be using the GPU_* API for shaders and the framebuffer. If there is something missing the APIs can be extended?

I even tried to expand the API. But it was having many changes, and it began to complicate. For framebuffer what is missing is a readpixel function and get a texture with these parameters:

glTexImage2D(GL_TEXTURE_2D, 0, GL_R32UI, sizex, sizey, 0, GL_RED_INTEGER, GL_UNSIGNED_INT, NULL);

I will try to implement the missing features in another patch

can you explain what the downsides of this would be?

What we could do with CPU that can not do with OpenGL is to get the closest element even if more than one element is on the same pixel.
But this is not really a problem, since the snap system serves to help the user get the position of visible element, (if more than one element are in the same pixel, they are not visibles).
Some things the patch can still resolve are:

  • objects that are not Mesh type (empty, camera, curves) do not yet take into account the occlusion (but the implementation is simple);
  • The snap position of animated objects is obtained as the position where the mouse cursor encountered the boundbox for the first time (with the coordinates stored in vbo);
  • As blender2.8 does not yet have wireframe mode, the snap without occlusion has not yet been enabled.

But I do not want to spend any more time on this patch without knowing if it can be accepted or not.

, could there be precision issues due to limited resolution Z buffer...

In fact the zbuffer is not very precise, (in the patch the chosen resolution was still 16 bits). But his goal is just to make the snapping ignore non-visible elements. So the precision is not very important in this case.

is the snapping always recomputed using the actual vertex coordinates[?]

The coordinates are obtained from VBO. It could be obtained from the Mechs as well, but it would not be so safe since the vertices indices of animated Mesh can change.

does it work correctly with multisample AA, ... ?

No, but the framebuffer is not multisampled. So I guess it's fine.

is there an option to disable it like for selection?

For simplicity, I've removed all the previous code. But if users find problems, the patch can be reverted.

I have not been able to reproduce the mentioned crash, but I think I fixed it with the information passed.

  • Patch updated to fix crash;

So it seems that precision would not be an issue in this case, since it's only used to identify the nearest vertex/edge/face, and the snapping happens later. I'm not sure what the latest ideas or plans were regarding OpenGL drawing for selection and snapping, so will let others weigh in.

For cases where you might not want the occlusion tests, I was thinking of a case like selecting a vertex of the cube, going into ortho side view, and snapping to another vertex. With occlusion it would only be able to find one of the two overlapping vertices, which is not necessarily the one nearest to the moving vertex. But it seems to randomly select one of the two in the existing code as well, so that wasn't any better.

I still get a crash when snapping, here's the --debug-gpu output, seems there is some issue with the framebuffer as well. This is on Linux + NVidia.

GPUShader: compile error:
0(5) : error C7011: implicit cast from "int" to "uint"

GL API other: Framebuffer detailed info: The driver allocated storage for renderbuffer 1.
GL API other: Framebuffer info: Framebuffer 18 is unsupported, because the combination of attachment formats is unsupported.
GL API other: Program/shader state info: GLSL program 266 failed to link
GL API other: Framebuffer info: Framebuffer 18 is unsupported, because the combination of attachment formats is unsupported.
GL API error: GL_INVALID_FRAMEBUFFER_OPERATION error generated. Operation is not valid because a bound framebuffer is not framebuffer complete.

@Brecht Van Lommel (brecht), even with --debug-gpu I'm not getting the output error on framebuffer :/

I made some changes to code hoping to fix the problem:

  • GPU_texture API extended to work with GL_R32UI;
  • fix GPUShader compile error;

The diff was not being updated

Ok, I needed to make this change, and then the framebuffer error is gone too. Seems it was a cascading failure, would be good if the code handled NULL return values and other types of errors when creating OpenGL data.

Probably the GLSL code should also be in gpu/shaders so it's easier to search/edit/refactor along with all the other code.

source/blender/editors/transform/transform_snap_object.c
2256

This should be outColor = 0u;

Germano Cavalcante (mano-wii) marked an inline comment as done.EditedAug 17 2017, 8:20 PM
Germano Cavalcante (mano-wii) updated this revision to Diff 9145.

I was sure I had put that "u"!

  • Moved GLSL code, which is not related to debug, to gpu/shaders;
  • Added GPU_matrix functions;

To avoid adding many new shaders, the shader with the line: " gl_Position.z -= 0.001;" was replaced by one that does not apply offset to the depth.

I'll have to think of a solution that changes the depth externally;

  • change range_depth of the elements with indices;
  • Use texelFetch in Debug shaders (better than uv);
  • Increases the size of the framebuffer to allow snap to elements beyond the limit;
  • Only init framebuffer creation when the snap flag for edges or vertices is found. This avoids unnecessarily memory allocation;
  • fix upper and left edge being clipped with offset to nearest out of bound;
  • Fix error when finding the bound box due to recent changes;
  • Optimize by avoiding recalculation of ModelViewProjectionMatrix;
  • Set the point size equal 2;

I think this really should be using the GPU_* API for shaders and the framebuffer. If there is something missing the APIs can be extended?
As I mentioned in T52173#448178, I thought that we wanted to move away from OpenGL for selection, and this seems to be going in the opposite direction. Maybe there are good reasons, but can you explain what the downsides of this would be? Could there be issues with missed snaps due to there not being enough pixels to contain all the verts/edges/faces, could there be precision issues due to limited resolution Z buffer or is the snapping always recomputed using the actual vertex coordinates, does it work correctly with multisample AA, ... ? Are there cases where taking occlusion into account is not what you want, is there an option to disable it like for selection?

Would prefer if we keep transform / snapping code outside of pixel-space / OpenGL. Since snapping is working at least reasonably well in master and 2.8x.

With this patch we will need to maintain 2x code paths - where some operations continue to use ray-cast, and picking elements uses OpenGL.

It's worth noting that selection picking has historically been a pain-point where different graphics cards work slightly differently - causing all sorts of problems and many bug reports.