Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Z order for selecting coplanar entities is inverted #521

Closed
ghost opened this issue Dec 1, 2019 · 13 comments · Fixed by #787 or #798
Closed

Z order for selecting coplanar entities is inverted #521

ghost opened this issue Dec 1, 2019 · 13 comments · Fixed by #787 or #798
Labels
Milestone

Comments

@ghost
Copy link

ghost commented Dec 1, 2019

System information

  • SolveSpace version: 3.0~690f87c
  • Operating system: Debian 9.x stretch (x86_64)

Expected behavior

If try to select one of co-planar entities, SolveSpace should prioritize to select nearest entity (according display Z axis).

Actual behavior

When tried to select one of co-planar entities, SolveSpace selected remote in space entity (according display Z axis), instead of selecting nearest entity.

pic.1

pic.2

Additional information

For bugs, please attach a savefile that shows the problematic behavior.
You can attach .slvs files by archiving them into a .zip first.

@whitequark whitequark added the bug label Dec 1, 2019
@whitequark
Copy link
Contributor

Related to #161. I recall there was a lot of trouble getting selection logic to work correctly.

@Evil-Spirit
Copy link
Collaborator

Evil-Spirit commented May 13, 2020

Need to integrate "Selection by order" where I implemented to following:

  1. When clicking, create a list of constraints/entities inside click radius.
  2. See if some of them is selected.
  3. Select the next entitiy after selected when click

This allows to select any entity even if them co-planar.

@ruevs
Copy link
Member

ruevs commented May 13, 2020

Need to integrate "Selection by order" where I implemented to following:

I can not find it. Or do you mean in NoteCAD?

@Evil-Spirit
Copy link
Collaborator

This one
Evil-Spirit/solvespace@a7be019

@whitequark
Copy link
Contributor

IIRC "selection by order" solved this problem but introduced new ones.

@phkahler
Copy link
Member

I also get frustrated at times when trying to drag a point and it grabs planes from g001-references and won't move. Perhaps those should only be selected if it's unambiguous?

@whitequark
Copy link
Contributor

We'll get the exact same problem in other groups if we special-case references. I think this can and should be solved properly instead.

@Evil-Spirit
Copy link
Collaborator

Problem with coplanar entities can't be solved, so selection by order is common workaround in other software.

@whitequark
Copy link
Contributor

Right, we can do that, and we can also make it possible to disambiguate the selection somehow (like what KiCAD does).

@phkahler phkahler added this to the 4.0 milestone Oct 23, 2020
@phkahler
Copy link
Member

I think it should prefer entities in the current group, Then entities (points) that lie in the current workplane, and then others.

@phkahler
Copy link
Member

Reopened. The recent fix is not very robust. Better but still not great.

@phkahler phkahler added this to the 4.0 milestone Oct 31, 2020
@phkahler
Copy link
Member

phkahler commented Nov 1, 2020

I really wanted to get this fixed for 3.0 and gave it a try. However, if I read the code correctly and this definition:

    // A hovered entity, with its location relative to the cursor.
    class Hover {
    public:
        int         zIndex;
        double      distance;
        Selection   selection;
    };

I believe distance is the lateral distance from the mouse pointer to an entity, while zIndex is a terrible name because it's based on the style of the entity which differentiates points, curves, and constraints. There is nothing that corresponds to the depth into the screen so proper Z-ordering is not just broken, it doesn't exist. This is great for 2D sketches but obviously has some issues in 3D.

I'm thinking a correct fix is going to add a "depth" member to the Hover, a bit of new calculations in all the picking code to provide the depth value for each entity, and then finally changes to ChooseFromHoverToSelect() and ChooseFromHoverToDrag(). And lastly verify that grid snapping still works!

@ruevs
Copy link
Member

ruevs commented Nov 18, 2020

Works fine now. And I have not noticed any regressions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants