blackears / cyclopsLevelBuilder

A Godot plugin to let you block in levels inside the Godot editor.
MIT License
1.09k stars 44 forks source link

Picking of edge handles in Edge Edit mode is broken if one of the edge ends is behind the camera #204

Closed distractedmosfet closed 1 day ago

distractedmosfet commented 3 days ago

Initial confession: I have not tested this on the most recent versions so perhaps this is no longer true but the related code appears to still be there so I assume it is. If my assumption is wrong then my apologies for the false alarm.

The issue seems to be the use of Camera3D.unproject_position inside of pick_closest_handle in tool_edit_edge.gd. Apparently this function uses the camera transform to project points from 3d space into camera space and that has a limitation that the documentation hints at but does not outright state: if the position is behind the camera then the transformation is wrong, reduz seems to be saying something to that effect here.

Currently you project the edge ends onto the screen, then do some math to determine the distance between the click position and that 2d segment. An alternative approach is to take the point_on_seg in 3d space that you already calculate, project that onto the screen and use that to test distance to the click position. Obviously that projection can also be incorrect, but you already check if that point is behind the camera and discard it later so that should catch that. This modification seems to work for me.

blackears commented 3 days ago

The pick distance is being done in screen space so that the radius of the pick vertex is the same regardless of how far away from the camera it is.

I've added a frustum check to the vertex, edge and face tools to make sure you're not picking something behind the camera. I'm not sure how I could test it though.

If you check out the head, I'm in the middle of changing how UVs are handled, so some UV related functions may not be working properly for now.

distractedmosfet commented 2 days ago

Sorry I realize I was so focused on discussing my findings of the cause of my issue that I really didn't elaborate on the actual user experience problem beyond the title. My apologies. Especially because I think that has led to a miscommunication.

The issue isn't that it's possible to select things that are behind the camera, your existing logic seems to work fine for that. The issue is that any edge that is partially in front and partially behind the camera can't be selected with a single click. I can confirm this still happens on the head. All you have to do to reproduce this is create a somewhat long box, get the camera close to it and look a bit down it's length like so:

A screenshot of a godot editor looking down a cyclops block, two edges are highlighted as being unselectable

It's worth pointing out that a drag-select can select those handles, the issue is with just a single click tap.

The reason for this is what I elaborate on above: it's because the screen-space distance check uses Camera3D.unproject_position to determine the screen positions of the edge-ends, but that produces incorrect positions if you ask it for a point that is behind the camera. So you then do a distance check using this incorrect screen position, which gets an incorrect distance.

In a local fork I have changed pick_closest_handle to do this:

# DISTRACTEDMOSFET EDIT
var point_on_seg:Vector3 = MathUtil.closest_point_on_segment(pick_origin, pick_dir, p0_world, p1_world)
var point_on_screen:Vector2 = viewport_camera.unproject_position(point_on_seg)
var dist_to_seg_2d_sq = point_on_screen.distance_squared_to(position)#MathUtil.dist_to_segment_squared_2d(position, p0_screen, p1_screen)

#print("<<2>>")
if dist_to_seg_2d_sq > radius * radius:

Basically I just moved up the calculation of point_on_seg, and I project that onto the screen and use that to do screen-distance. I could be wrong but I believe this is always equivalent, except that it always works as long as the nearest point is front of the camera, and if it's not a check below discards the edge anyway.

If I'm wrong and this alternative produces other issues, then I guess the solution would be finding some method to clip the edge ends to inside the frustum first so that the unproject_position always produces a sensible value.

And for reference, I'm only aware of this sort of thing being an issue with the edge tool specifically. Haven't noticed any issues with the vertex tool yet but I also haven't looked at that code. I wouldn't be surprised if it's fine.

blackears commented 1 day ago

Thanks for catching that. I've made a patch to the edge edit tool in the main branch.