TrenchBroom / TrenchBroom

Cross-Platform Level Editor
kristianduske.com/trenchbroom
GNU General Public License v3.0
1.88k stars 224 forks source link

Vertices becoming unstuck when dragging #2195

Closed JackViney96 closed 5 years ago

JackViney96 commented 5 years ago

System Information

TrenchBroom v2.0.4 on Windows 10

Expected Behavior

When dragging a vertex that is shared by multiple selected brushes, each brush should be edited together. This only seems to work intermittently.

Steps to Reproduce

I'm not sure exactly what causes this issue. I am uploading two videos + a .map containing brushes that are affected by this bug.

https://streamable.com/lgato https://streamable.com/y57me vertdrag.map.txt

kduske commented 5 years ago

This is probably due to a recent change that introduced more precision to how vertices are handled and identified. I‘ll look into it.

kduske commented 5 years ago

The initial solution was flawed, as pointed out be @ericwa here: https://github.com/kduske/TrenchBroom/pull/2198#issuecomment-410536448

kduske commented 5 years ago

Outline of solution

Currently, the vertex handle manager maps vertex positions to the brushes that have vertices at these positions (and likewise for edge and face handles). The vertex positions are not exact to allow for "clumping" the vertices so that close by vertices are treated as one. When vertices are then moved, the vertex tool uses the inexact positions to move identify the vertices to be moved, so the vertex moving code has to compensate for the inaccurate vertex positions. So there are two places in TB where this "vertex clumping" is implemented: the controller level and the model level (brush).

The problem here is that "close by" cannot be achieved easily, which is why we are seeing these issues. No matter which definition of close by is chosen, there will be some issues.

The alternate solution is to use exact positions in the vertex handle manager. Then, when the user clicks on a handle to select it or to drag it, the handle manager will automatically find all handles within a certain radius of the clicked handle and return these "close by handles" to the controller. Then, the controller will use this information to send exact data to the functions that perform the actual vertex moves. This eliminates the need to compensate for inexact vertex positions on the level of brushes and constrains this "vertex clumping" requirement to the controller level where it belongs.

kduske commented 5 years ago

I think the first attempted fix broke edge dragging. Verify that edge and face dragging works fine after the second fix attempt, otherwise create a new issue.