Page MenuHome

Fix EEVEE not using the PBVH for rendering in Sculpt Mode
Needs ReviewPublic

Authored by Pablo Dobarro (pablodp606) on Aug 13 2020, 12:57 AM.
Tags
None
Tokens
"Love" token, awarded by bnzs."Love" token, awarded by monio."Burninate" token, awarded by DiogoX2."100" token, awarded by Kronk."Burninate" token, awarded by andruxa696."The World Burns" token, awarded by filibis."Piece of Eight" token, awarded by lopoIsaac."Love" token, awarded by Serva."Manufacturing Defect?" token, awarded by EAW."100" token, awarded by Frozen_Death_Knight."Mountain of Wealth" token, awarded by franMarz.

Details

Summary

I'm not sure if there is any other limitation that I'm not aware of, but
this seems like a bug. Using the PBVH drawing in Multires from EEVEE
seems to work fine, so there should not be any reason for disabling it
for meshes without shape keys and modifiers

Mesh with 1.5M vertices, lookdev with a SSS material:
Before:

After:

Diff Detail

Repository
rB Blender
Branch
fix-eevee-pbvh-drawing (branched from master)
Build Status
Buildable 9497
Build 9497: arc lint + arc unit

Event Timeline

Pablo Dobarro (pablodp606) requested review of this revision.Aug 13 2020, 12:57 AM
Pablo Dobarro (pablodp606) created this revision.

The issue was the lack of the UV, tangent, etc... layers. We used the slow path for the reason. Did anything change regarding this?

I don't think so, but Multires has the same data available and has the PBVH rendering always enabled. I'm not sure if it is possible to fix this properly for meshes and multires and always render the full version of the material, but even if sculpt mode has some limitations regarding the material features, having this performance with EEVEE enabled is insane.

Yes. I don't have a strong opinion but we should have a warning of some sort indicating the limitation. We had some reports about this.

Also you should try to see if normal maps behave correctly and dont render the surface broken.

@Pablo Vazquez (pablovazquez) Any thoughts on this?

I'm testing it and it works like in Multires. It does not display textures and normal maps, but the surface looks fine. It would be nice if this could be solved in the future, but usually when you are working with meshes at this resolution you are using the voxel remesher and vertex colors for shading, so not having textures for a while should not be a problem.

I'm not sure if it is possible to fix this properly for meshes and multires and always render the full version of the material

I've started supporting UVs in D5884. As far as i remember, the state of that patch is that SubdivCCG stores all required data, which "just" needs to be passed to shared. Was hoping @Clément Foucault (fclem) will finish that part ;)

Yes. I don't have a strong opinion but we should have a warning of some sort indicating the limitation. We had some reports about this.

Showing warning will be rather annoying.


@Pablo Dobarro (pablodp606) , did you consider using full_shading for rendered (OB_RENDER) viewport shading mode?

source/blender/blenkernel/intern/paint.c
2028–2030

Think the full_shading should effectively become false here (as in, don't do full shading for wire and boundbox shading modes). This leaves us with return !(ss->shapekey_active || ss->deform_modifiers_active);

Now, on a related topic (NOT to be addressed in this patch). It should be possible to use PBVH drawing for shapekeys and deform modifiers when SCULPT_ONLY_DEFORM is enabled. This is because PBVH is deformed to correspond to the key or the deform modifier. Would need to explicitly store whether there are active non-deform-only modifiers in the SculptSession.

P.S. See note about OB_RENDER in the non-inlined comment.

I also made D8564 which enables PBVH rendering as an UI option.
I'm not sure what would be better or less confusing. Right now Multires always renders materials without textures, so we can make this an option for meshes or we can make meshes always render the same as Multires by default.

@Sergey Sharybin (sergey) If we decide to merge D8564 will also be possible to check for SCULPT_ONLY_DEFORM for meshes too.

This is not really a bug fix per-se, so I would say it may be better to do in master, not 2.90.

source/blender/blenkernel/intern/paint.c
2029–2030

@Pablo Dobarro (pablodp606) Sergey was suggesting the following (considering that you are fine never having UV and normals for sculpting). If we go this way v3d needs to be UNUSED() or removed too of course.

We could also check which attributes are needed by the material, and only use the slower drawing if UVs or vertex colors are needed.

source/blender/blenkernel/intern/paint.c
2029–2030

So you would like this to be always enabled instead of having the option to disable it for meshes if necessary?

As an addendum, this could be an option from the get-go:

  • When in rendered mode, *always* shows UV and normal.
  • When in look dev mode, a new option (off by default) to draw normals (and uvs if we can't have them) - this option belongs to the "Viewport Overlays" popover, besides the Mask and Face Sets.

But if I understood this correctly, the fact it is not showing the UVs right now is just because of a TODO, not a limitation. As @Brecht Van Lommel (brecht) said, we should be able to detect if the object needs UV and tangents for the materials and make this always work for all viewport modes. If that is the case, I don't see a reason for adding the option.

@Pablo Dobarro (pablodp606) right, UV is this oddball that may be supported soon. But normals won't from what I understood

In that case, the option should be part of the shading popover, right? Rendering the normals is not an overlay, is part of the material.

@Pablo Dobarro (pablodp606) this is a tricky question - and @Pablo Vazquez (pablovazquez) is welcome to pitch in.

My reasoning for leaving this in the overlay panel:

As of now we have no per-mode shader setting. The design behind this is that the modes are overlays on top of the render engine rendered underneath. The engine underneath could be pitch black and someone should still be able to edit the models.

When we need to show a fast version of the lookdev to help sculpting, this should be understood (from the user point of view) as an overlay, the sculpt engine overlay. The fact that we don't draw the fully shaded model underneath is more an optimization detail.

Alright, had another round of discussions with Sergey about that, this is our current proposal:

Rendered Mode:

  1. Always draws correct - done already
  2. Use fast PBVH drawing when UV and normals are not needed - what this patch could do - target 2.91
  3. UV support for PBVH_GRIDS; use fast PBVH drawing even when UVs are needed - finish D5884 - target 2.91
  4. Part normals recalculation in PBVH; use fast PBVH drawing always- it would still have some cost - target 2.92, 2.93

Look Dev:

  • Wait until 1-3 is done, to see if we need an option or not. Specially if (4) goes into the agenda.

For normals, are we talking about custom normals or normal maps? I don't think either of those are important in the context of sculpting high poly models. Either ignoring them or falling back to slow drawing seems reasonable.

Generated texture coordinates are more important, when you use a procedural texture in a material. Those could be made fast with two changes:

  • Add mesh option to disable normalization of coordinates to 0..1 and disable it by default. This behavior is not particularly useful anyway, and actually adds texture stretching.
  • Optimize code internally so that if there are no deforming modifiers, no texture coordinates are generated and object space coordinates are used instead.

Detecting if a material uses UVs is easy after the GPU shaders are compiled. But BKE_sculptsession_use_pbvh_draw is used in some places before that are outside the drawing code. It could check the GPU materials that have been compiled so far (get evaluated object from the depsgraph, loop over it's materials and their gpumaterials). But there may need to be some kind of refresh once the material is compiled.

@Dalai Felinto (dfelinto) From what @Brecht Van Lommel (brecht) said, in that case wouldn't it make sense to skip the step 2 and try step 3 directly? Otherwise, we are going to need a refactor and add all the code to check the object materials in that function just to remove it when step 3 is done, an even with that, Multires is still not going to be supported in step 2 (which is what most people will use the UVs, meshes from the voxel remesher don't have textures). If we want this to be working as soon as possible, then we can just simply enable PBVH rendering always for meshes, even if it renders wrong materials, which is what multires is doing in 2.90.