Regression: Wire frame display doesn't draw every edge in some cases #102365

New Issue

heini · 2022-11-08T22:08:29+01:00

heini commented

2022-11-08 22:08:29 +01:00

System Information
Operating system: Windows-10-10.0.19045-SP0 64 Bits
Graphics card: NVIDIA GeForce GTX 1060 6GB/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 526.47

Blender Version
Broken: version: 3.5.0 Alpha, branch: master, commit date: 2022-11-08 01:18, hash: fddcdcc20c
Worked: 3.4
Caused by 10131a6f62

Short description of error
in some meshes the wireframe display doesn't show all edges. Most obvious in 2d meshes but also with meshes that have stretched or concave face areas. Happens in wireframe mode, wireframe overlay and object properties->VP display: wireframe

this is the same object in object and edit mode

wireframe_overlay_bug.blend

select object 3, enter edit mode, return to object mode - wire overlay is gone.

save and open in blender 3.4, select any, enter edit/sculpt mode - peekaboo, wireframe is back in objectz mode

**System Information** Operating system: Windows-10-10.0.19045-SP0 64 Bits Graphics card: NVIDIA GeForce GTX 1060 6GB/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 526.47 **Blender Version** Broken: version: 3.5.0 Alpha, branch: master, commit date: 2022-11-08 01:18, hash: `fddcdcc20c` Worked: 3.4 Caused by 10131a6f62 **Short description of error** in some meshes the wireframe display doesn't show all edges. Most obvious in 2d meshes but also with meshes that have stretched or concave face areas. Happens in wireframe mode, wireframe overlay and object properties->VP display: wireframe ![image.png](https://archive.blender.org/developer/F13870733/image.png) this is the same object in object and edit mode [wireframe_overlay_bug.blend](https://archive.blender.org/developer/F13870747/wireframe_overlay_bug.blend) select object 3, enter edit mode, return to object mode - wire overlay is gone. save and open in blender 3.4, select any, enter edit/sculpt mode - peekaboo, wireframe is back in objectz mode

heini commented

2022-11-08 22:08:29 +01:00

Added subscriber: @heini

blender-admin commented

2022-11-08 22:08:29 +01:00

#102407 was marked as duplicate of this issue

Richard Antalik changed title from ~~wire frame display doesn't draw every edge in some cases~~ to Regression: Wire frame display doesn't draw every edge in some cases

2022-11-08 22:21:57 +01:00

Richard Antalik commented

2022-11-08 22:21:57 +01:00

Changed status from 'Needs Triage' to: 'Confirmed'

Richard Antalik commented

2022-11-08 23:33:16 +01:00

Added subscribers: @HooglyBoogly, @iss

Richard Antalik commented

2022-11-08 23:33:16 +01:00

Bisect seems to point at 10131a6f62

CC @HooglyBoogly

Bisect seems to point at 10131a6f62 CC @HooglyBoogly

Hans Goudey commented

2022-11-08 23:34:47 +01:00

Hmm, interesting. Thanks for the report.

Philipp Oeser commented

2022-11-09 11:00:44 +01:00

Added subscriber: @lichtwerk

Philipp Oeser commented

2022-11-09 11:00:44 +01:00

This is because bmesh_quick_edgedraw_flag clears the ME_EDGEDRAW flag if the angle between faces is too small, see the comment:

/* This is a cheap way to set the edge draw, its not precise and will

pick the first 2 faces an edge uses.

The dot comparison is a little arbitrary, but set so that a 5 subdivisions

ico-sphere won't vanish but 6 subdivisions will (as with pre-bmesh Blender). */

This seems to be broken since 2.8 however (drawing code was using ME_EDGERENDER instead) , so the whole business with Viewport Display > Show > All Edges is also broken since 2.8 (shows all edges anyways).
But now we are in the unfortunate situation that bmesh_quick_edgedraw_flag clears the ME_EDGEDRAWbut we cant even get all edges to display with Viewport Display > Show > All Edges

This is because `bmesh_quick_edgedraw_flag` clears the `ME_EDGEDRAW` flag if the angle between faces is too small, see the comment: > /* This is a cheap way to set the edge draw, its not precise and will > * pick the first 2 faces an edge uses. > * The dot comparison is a little arbitrary, but set so that a 5 subdivisions > * ico-sphere won't vanish but 6 subdivisions will (as with pre-bmesh Blender). */ This seems to be broken since 2.8 however (drawing code was using `ME_EDGERENDER` instead) , so the whole business with `Viewport Display` > `Show` > `All Edges` is also broken since 2.8 (shows all edges anyways). But now we are in the unfortunate situation that `bmesh_quick_edgedraw_flag` clears the `ME_EDGEDRAW`but we cant even get all edges to display with `Viewport Display` > `Show` > `All Edges`

Philipp Oeser self-assigned this 2022-11-09 11:46:31 +01:00

Philipp Oeser commented

2022-11-09 11:46:31 +01:00

Will look into this (the idea is to fix both the recent regression as well as the older 2.8 regression in one go)

Philipp Oeser removed their assignment 2022-11-09 14:11:18 +01:00

Philipp Oeser commented

2022-11-09 14:11:18 +01:00

Hm, this might require a bigger refactor than anticipated, so would like input from other devs first.

file for the Icosphere case below:
T102365_display_as_wire_normal_threshold.blend

To recap:

before 10131a6f62, the following situation was broken (since 2.8, was working in 2.79):
- Icosphere with 6 subdivisions, object display_type set to Wire has all edges displayed
- (even with Viewport Display > Show > All Edges OFF -- that setting is only used for the wireframe overlay in 2.8)
- (bmesh_quick_edgedraw_flag would "correctly" clear the ME_EDGEDRAW flag based on face angle, but for drawing ME_EDGERENDER was used instead in extract_edge_fac_iter_poly_mesh, setting MeshExtract_EdgeFac_Data data to 255 mostly)
after 10131a6f62, the Icosphere case now actually behaves like in 2.79
- Icosphere with 6 subdivisions, object display_type set to Wire has no edges displayed
- BUT: we cannot get all edges to display (even with Viewport Display > Show > All Edges ON -- that setting is only used for the wireframe overlay in 2.8)
- to make that work, wed have to use something like wires_all_grp` outside the overlay as well?
after 10131a6f62, the overlay is also broken
- (bmesh_quick_edgedraw_flag would "correctly" clear the ME_EDGEDRAW flag based on angle, drawing now uses this in extract_edge_fac_iter_poly_mesh, setting MeshExtract_EdgeFac_Data data to 0 if the flag was cleared)
- (Viewport Display > Show > All Edges ON does not have an affect since it only results in wires_all_grp to be used, MeshExtract_EdgeFac_Data is still in effect, if it is zero, get_edge_sharpness will still return negative

One idea was to somehow pass OB_DRAW_ALL_EDGES to bmesh_quick_edgedraw_flag to not clear ME_EDGEDRAW, but there might be cleaner solutions.
Prefer if someone with a broader view takes a look here

Hm, this might require a bigger refactor than anticipated, so would like input from other devs first. file for the Icosphere case below: [T102365_display_as_wire_normal_threshold.blend](https://archive.blender.org/developer/F13875388/T102365_display_as_wire_normal_threshold.blend) To recap: - before 10131a6f62, the following situation was broken (since 2.8, was working in 2.79): - Icosphere with 6 subdivisions, object `display_type` set to `Wire` has **all** edges displayed - (even with `Viewport Display` > `Show` > `All Edges` OFF -- that setting is only used for the wireframe **overlay** in 2.8) - (`bmesh_quick_edgedraw_flag` would "correctly" clear the `ME_EDGEDRAW` flag based on face angle, but for drawing `ME_EDGERENDER` was used instead in `extract_edge_fac_iter_poly_mesh`, setting `MeshExtract_EdgeFac_Data` data to 255 mostly) - after 10131a6f62, the Icosphere case now actually behaves like in 2.79 - Icosphere with 6 subdivisions, object `display_type` set to `Wire` has **no** edges displayed - BUT: we cannot get all edges to display (even with `Viewport Display` > `Show` > `All Edges` ON -- that setting is only used for the wireframe **overlay** in 2.8) - to make that work, we`d have to use something like `wires_all_grp` outside the overlay as well? - after 10131a6f62, the overlay is also broken - (`bmesh_quick_edgedraw_flag` would "correctly" clear the `ME_EDGEDRAW` flag based on angle, drawing now uses this in `extract_edge_fac_iter_poly_mesh`, setting `MeshExtract_EdgeFac_Data` data to 0 if the flag was cleared) - (`Viewport Display` > `Show` > `All Edges` ON does not have an affect since it only results in `wires_all_grp` to be used, `MeshExtract_EdgeFac_Data` is still in effect, if it is zero, `get_edge_sharpness` will still return negative One idea was to somehow pass `OB_DRAW_ALL_EDGES` to `bmesh_quick_edgedraw_flag` to not clear `ME_EDGEDRAW`, but there might be cleaner solutions. Prefer if someone with a broader view takes a look here

Philipp Oeser commented

2022-11-09 14:11:45 +01:00

Added subscribers: @fclem, @Jeroen-Bakker

Philipp Oeser commented

2022-11-09 14:11:45 +01:00

CC @fclem
CC @Jeroen-Bakker

CC @fclem CC @Jeroen-Bakker

Jeroen Bakker commented

2022-11-09 16:19:58 +01:00

Added subscriber: @pablovazquez

Jeroen Bakker commented

2022-11-09 16:19:58 +01:00

IMO the technical bits and pieces should follow a design. As there is no clear idea how this should work I would say that the blender manual is the design. But current way is not consistent or clear to users.

In that case I would start with, what does the user want. and how can we make this consistent and predictive.

Blender 3.3.1

Object->Subdivision->Optimum display	This should be done by depsgraph evaluation and should be the ground thruth.	Is confusing with `All Edges` option...
Object->Viewport Display->Wireframe	When in solid, mat, render mode add a wireframe overlay	But only when displaying overlays.
Object->Viewport Display->All Edges	Not sure what it actually does...	Also feels like a bad design.
Object->Viewport Display->Display As=Wire	Will force the object to be drawn with only wires.	But only when displaying overlays.
Viewport->Overlays->Wireframe	Overal all objects with a wireframe overlay.
Viewport->Overlays->Wireframe slider	Hide or show more wires	Currently isn't connected with object viewport display settings
Viewport->Overlays->Opacity slider	This changes the opacity of all wires in the scene.	Also works in solid mode and option is greyed out

Yes it is a mess!

To me some options are substractive, other are additive and other are overrides, which is not communicated to users. I rather have a single base and clearly defined additive and substractive functionality. IMO the base is what depsgraph outputs.

I don't know the details what should be done to the current code-base to get this working and that we have situations covered. So would like to get some feedback from others as well. @pablobazquez

Personally we should stick with previous implementation until we have a good design and the time to implement it. Doing to many changes after each other is not helping our userbase.

IMO the technical bits and pieces should follow a design. As there is no clear idea how this should work I would say that the blender manual is the design. But current way is not consistent or clear to users. In that case I would start with, what does the user want. and how can we make this consistent and predictive. # Blender 3.3.1 | Object->Subdivision->Optimum display| This should be done by depsgraph evaluation and should be the ground thruth.| Is confusing with `All Edges` option... | | -- | -- | -- | | Object->Viewport Display->Wireframe | When in solid, mat, render mode add a wireframe overlay | But only when displaying overlays. | | Object->Viewport Display->All Edges | Not sure what it actually does...| Also feels like a bad design. | | Object->Viewport Display->Display As=Wire | Will force the object to be drawn with only wires. | But only when displaying overlays. | | Viewport->Overlays->Wireframe| Overal all objects with a wireframe overlay.| | | Viewport->Overlays->Wireframe slider| Hide or show more wires | Currently isn't connected with object viewport display settings| | Viewport->Overlays->Opacity slider| This changes the opacity of all wires in the scene. | Also works in solid mode and option is greyed out | Yes it is a mess! To me some options are substractive, other are additive and other are overrides, which is not communicated to users. I rather have a single base and clearly defined additive and substractive functionality. IMO the base is what depsgraph outputs. I don't know the details what should be done to the current code-base to get this working and that we have situations covered. So would like to get some feedback from others as well. @pablobazquez Personally we should stick with previous implementation until we have a good design and the time to implement it. Doing to many changes after each other is not helping our userbase.

Hans Goudey commented

2022-11-09 16:27:22 +01:00

To me it looks like the best solution is to remove bmesh_quick_edgedraw_flag. That is only applied when leaving edit mode, which makes it a non-starter for anything procedural, and is inconsistent with all other uses of the draw flag.

The one similar feature, Viewport->Overlays->Wireframe slider should be calculated as part of the mesh rendering process. I believe this is already implemented with extract_mesh_vbo_edge_fac.cc.
It doesn't appear to be working at the moment, I thought I tested that, sorry about that.

The "All Edges" option should be removed, since it's redundant with the other settings.

To me it looks like the best solution is to remove `bmesh_quick_edgedraw_flag`. That is only applied when leaving edit mode, which makes it a non-starter for anything procedural, and is inconsistent with all other uses of the draw flag. The one similar feature, `Viewport->Overlays->Wireframe slider` should be calculated as part of the mesh rendering process. I believe this is already implemented with `extract_mesh_vbo_edge_fac.cc`. It doesn't appear to be working at the moment, I thought I tested that, sorry about that. The "All Edges" option should be removed, since it's redundant with the other settings.

Jeroen Bakker commented

2022-11-09 16:34:10 +01:00

Classifying it as a 3.5 only bug to make the reporting gods happy. Not sure if we should lower the prio.

Philipp Oeser commented

2022-11-10 09:35:57 +01:00

In #102365#1443221, @Jeroen-Bakker wrote:

Blender 3.3.1

| Object->Viewport Display->All Edges | Not sure what it actually does...| Also feels like a bad design. |
| Viewport->Overlays->Wireframe slider| Hide or show more wires | Currently isn't connected with object viewport display settings|

In #102365#1443224, @HooglyBoogly wrote:
The "All Edges" option should be removed, since it's redundant with the other settings.

Well, All Edges is away to display all edges on a per object basis (even if the wireframe overlay slider reduced this for the whole scene, so the slider is connected with object viewport display settings) .
I think this is usefull and I dont think there is another way to achieve this (nothing else triggers using wires_all_grp), so it is not redundant.

In #102365#1443221, @Jeroen-Bakker wrote:
Personally we should stick with previous implementation until we have a good design and the time to implement it. Doing to many changes after each other is not helping our userbase.

Since this really needs some design (and while I applaud the directin of getting rid of edge flags), I really think the design should be done first.
My vote for reverting 10131a6f62 for now.

> In #102365#1443221, @Jeroen-Bakker wrote: > # Blender 3.3.1 > > | Object->Viewport Display->All Edges | Not sure what it actually does...| Also feels like a bad design. | > | Viewport->Overlays->Wireframe slider| Hide or show more wires | Currently isn't connected with object viewport display settings| > In #102365#1443224, @HooglyBoogly wrote: > The "All Edges" option should be removed, since it's redundant with the other settings. Well, `All Edges` is away to display **all** edges on a **per object basis** (even if the wireframe overlay slider reduced this for the whole scene, so the slider **is connected** with object viewport display settings) . I think this is usefull and I dont think there is another way to achieve this (nothing else triggers using `wires_all_grp`), so it is not redundant. > In #102365#1443221, @Jeroen-Bakker wrote: > Personally we should stick with previous implementation until we have a good design and the time to implement it. Doing to many changes after each other is not helping our userbase. Since this really needs some design (and while I applaud the directin of getting rid of edge flags), I really think the design should be done **first**. My vote for reverting 10131a6f62 for now.

Jeroen Bakker commented

2022-11-10 12:04:36 +01:00

In #102365#1443590, @lichtwerk wrote:

Well, All Edges is away to display all edges on a per object basis (even if the wireframe overlay slider reduced this for the whole scene, so the slider is connected with object viewport display settings) .
I think this is usefull and I dont think there is another way to achieve this (nothing else triggers using wires_all_grp), so it is not redundant.

But does it also has more priority than that the depsgraph has (Optimize Display)... All Wires might be a tag to ignore the wireframe slider but it is not clear.

I agree to revert it the change for now to reduce confusion later on.

> In #102365#1443590, @lichtwerk wrote: > Well, `All Edges` is away to display **all** edges on a **per object basis** (even if the wireframe overlay slider reduced this for the whole scene, so the slider **is connected** with object viewport display settings) . > I think this is usefull and I dont think there is another way to achieve this (nothing else triggers using `wires_all_grp`), so it is not redundant. > But does it also has more priority than that the depsgraph has (Optimize Display)... All Wires might be a tag to ignore the wireframe slider but it is not clear. I agree to revert it the change for now to reduce confusion later on.

Hans Goudey commented

2022-11-10 17:46:53 +01:00

Reverting would be an option, but I'd like to avoid reverting the commit if necessary, I don't think the code churn is necessary here.
As far as I can tell, restoring the previous behavior is quite simple. I've implemented that in D16451.

Reverting would be an option, but I'd like to avoid reverting the commit if necessary, I don't think the code churn is necessary here. As far as I can tell, restoring the previous behavior is quite simple. I've implemented that in [D16451](https://archive.blender.org/developer/D16451).

Hans Goudey self-assigned this 2022-11-10 18:47:35 +01:00

Richard Antalik commented

2022-11-10 20:52:13 +01:00

Added subscriber: @PancakeMSTR

blender-admin commented

2022-11-16 23:48:09 +01:00

This issue was referenced by 145839aa42

This issue was referenced by 145839aa42f775f161f5c6a05687731fb5ae513e

Hans Goudey commented

2022-11-16 23:49:52 +01:00

Changed status from 'Confirmed' to: 'Resolved'

Hans Goudey closed this issue

2022-11-16 23:49:52 +01:00

Thomas Dinges added this to the 3.5 milestone 2023-02-07 18:56:02 +01:00

Sign in to join this conversation.

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Regression: Wire frame display doesn't draw every edge in some cases #102365

Blender 3.3.1

Blender 3.3.1