Details

Reviewers

Brecht Van Lommel (brecht)
Clément Foucault (fclem)

Maniphest Tasks

T66175: TexturePaint: Improve Drawing Face Mask

Commits

rB76aff66301b1: PaintingModes: Facemask Wireframe Drawing

Summary

The wireframe drawing for face masks is intrusive as selected wires
were solid white and always drawn. This made it hard for users to see
the exact color near edges.

This patch draws only the border of the selected faces,
edges between two selected faces are not drawn at all.

Before applying this patch

After applying this patch

Diff Detail

Repository: rB Blender

Event Timeline

Jeroen Bakker (jbakker) created this revision.Jun 28 2019, 12:11 PM

Harbormaster completed remote builds in B3920: Diff 16103.Jun 28 2019, 12:11 PM

I checked with @Julien Kaspar (JulienKaspar) and would like to do similar changes to the vertex/weight paint face mask selection.

Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)Jun 28 2019, 12:16 PM

Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)Jun 28 2019, 12:37 PM

Added similar drawing to weightpainting and vertex painting

Harbormaster completed remote builds in B3921: Diff 16104.Jun 28 2019, 12:41 PM

Cleanup the patch

Harbormaster completed remote builds in B3922: Diff 16105.Jun 28 2019, 12:44 PM

Jeroen Bakker (jbakker) retitled this revision from TexturePainting: Wireframe Drawing to PaintingModes: Facemask Wireframe Drawing .Jun 28 2019, 12:52 PM

Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)

Jeroen Bakker (jbakker) added reviewers: Brecht Van Lommel (brecht), Clément Foucault (fclem).

I wouldn't do the stippling, it looks ugly on diagonal lines and is not really needed.

Is it possible to avoid drawing the black unselected edges?

source/blender/draw/intern/draw_cache_impl_mesh.c
3877–3879	It's more efficient to have a single hash with the value being the number of selected faces.
3902	It would be more efficient to iterator over the hash.
3922	Which case is this? Can we add an explanation why this is not needed now?

This revision now requires changes to proceed.Jun 28 2019, 1:30 PM

Removed second edgehash

Harbormaster completed remote builds in B3924: Diff 16110.Jun 28 2019, 2:41 PM

Remove mapping-use

Harbormaster completed remote builds in B3925: Diff 16111.Jun 28 2019, 3:12 PM

Jeroen Bakker (jbakker) added inline comments.Jun 28 2019, 3:23 PM

source/blender/draw/intern/draw_cache_impl_mesh.c
3840	I am not sure how we could use the EdgeHashIterator as we need the vertex loop index in the index buffer.
3922	Was copy paste mistake, will remove the if statement to use mapped data

Ok, the edge hash iterator won't work then.

Erick Tukuniata (erickblender) awarded a token.Jun 28 2019, 3:30 PM

Removed unused code example for edgehash iterator

Harbormaster completed remote builds in B3926: Diff 16112.Jun 28 2019, 3:41 PM

Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)Jun 28 2019, 3:41 PM

Brecht Van Lommel (brecht) accepted this revision.Jun 28 2019, 4:06 PM

Jeroen Bakker (jbakker) added inline comments.Jun 28 2019, 5:19 PM

source/blender/draw/intern/draw_cache_impl_mesh.c
3839	Postprocess edgehash to optimize the inner loop lookups.

@Brecht Van Lommel (brecht)

On my way home I did some math. and I think we should revert the changes to the original solution (two edgehashes). In the first solution with the two edgehashes the inner loop would only check against an edgehash that contained the edges of the border of the face mask selection. In the current solution the edgehash contains all edges. as the inner loop is per poly per edge a check against a smaller edgehash would be faster.
Don't think the speedup will be noticeable to the user, but we might want the fastest solution as this is done in the drawing code.

If the solution was possible with the edgehashIterator the second solution would have been optimal...

I think the current solution is fine, hashes are O(1) regardless of the size.

Jeroen Bakker (jbakker) added inline comments.Jul 1 2019, 9:04 AM