Mask dialog, can't select just one when close together

[johnfi...@gmail.com]

When points are very close together in a mask, you can't select just one to move.

I understand having the points so close is not typical.  But the code seems to have been written anticipating points being that close together and intending that a single click should select all the nearby points.  Separately, there is a different way to select a group of points, so it seems like a strange feature.

Can someone tell me why the current behavior would be desired?

I'm removing it in my private copy anyway.  I find it annoying when it happens and never useful.  But I would like to know the intent.

The code I'm talking about is

in MaskImageCtrl::OnLeftMouseDown

in             case NO_SELECTION:

SelectPointsInsideMouseRect

[johnfi...@gmail.com]

After playing with it a bit, I understand how it might have been the way it was due to not caring about the behavior of points close together (rather than explicitly wanting them to act that way) while wanting an interface that supported the explicit method of supporting multiple points (dragging).

I expect the project admins won't want this change (won't see any value in better support of points very near each other within a mask).  So committing this to sourceforge would just confuse the other things I want to commit later.

But, if anyone cares, the code below is  what I'm using myself replacing that function.  So far, I'm much happier about the way it behaves vs. struggling with the original.

I hope the code below formats OK in google groups.  The online instructions for doing that "correctly" tell me to click on something that isn't there.  Unlike other forums, I can't preview and I can't edit after posting.  So apologies if the following comes out unreadable (and if so, can someone tell me how to do it right):

// Inserts into UIntSet &points either all (selected) points inside rectangle m_dragStartPos

//  to m_currentPos, or (if there were none) the single nearest and near enough point outside
bool MaskImageCtrl::SelectPointsInsideMouseRect(HuginBase::UIntSet &points,const bool considerSelectedOnly)
{
    // compute one dimension distance outside range (zero if inside range)
    auto distanceHelper = [](double vmin, double vmax, double v)->double
    {
        return (v<vmin) ? (vmin-v) : (v<vmax) ? 0 : (v-vmax);
    };
    double selectionLimit = maxSelectionDistance*maxSelectionDistance;
    unsigned pending = UINT_MAX;
    hugin_utils::FDiff2D p1=applyRotInv(invtransform(m_dragStartPos));
    hugin_utils::FDiff2D p2=applyRotInv(invtransform(m_currentPos));
    double xmin=std::min(p1.x,p2.x);
    double xmax=std::max(p1.x,p2.x);
    double ymin=std::min(p1.y,p2.y);
    double ymax=std::max(p1.y,p2.y);
    const HuginBase::VectorPolygon poly=m_editingMask.getMaskPolygon();
    for(unsigned int i=0;i<poly.size();i++)
    {
        if( !considerSelectedOnly || set_contains(m_selectedPoints,i) )
        {
            double xDistance = distanceHelper(xmin, xmax, poly[i].x);
            double yDistance = distanceHelper(ymin, ymax, poly[i].y);
            double d2 = xDistance*xDistance + yDistance*yDistance;
            if(d2 <= selectionLimit)
            {
                if(selectionLimit==0 && pending!=UINT_MAX)
                    points.insert(pending);
                pending = i;
                selectionLimit = d2;
            }
        }
    }
    if(pending!=UINT_MAX)
    {
        points.insert(pending);
        return true;
    }
    return false;
};

[Bruno Postle]

On Thu, 10 Feb 2022 at 00:01, johnfi...@gmail.com
<johnfi...@gmail.com> wrote:
>
> When points are very close together in a mask, you can't select just one to move.
>
> I understand having the points so close is not typical. But the code seems to have been written anticipating points being that close together and intending that a single click should select all the nearby points. Separately, there is a different way to select a group of points, so it seems like a strange feature.
>
> Can someone tell me why the current behavior would be desired?

I experience this as trying to drag one node, but Hugin insists on
dragging the previous (highlighted) nodes instead, which would make
sense, but somehow the highlighted nodes seem to be more likely to get
moved than the unselected node that is exactly where I clicked.

--
Bruno

[johnfi...@gmail.com]

On Thursday, February 10, 2022 at 6:41:27 PM UTC-5 bruno...@gmail.com wrote:

I experience this as trying to drag one node, but Hugin insists on
dragging the previous (highlighted) nodes instead, which would make
sense, but somehow the highlighted nodes seem to be more likely to get
moved than the unselected node that is exactly where I clicked.

If I understand you correctly:

You are indicating you had mask points so close together that this behavior occurred.  I assume the way you use hugin (as opposed to the way I do) is within the bounds of what hugin ought to support well.

I don't think you are saying that was a DESIRABLE behavior.

The code (usage of the function I changed) does (under certain conditions) look first for selected points near enough to the click and only if that fails looks for unselected points.  So the behavior you described is exactly as expected from the code.  I hadn't tried that case yet, and had not thought it through.  My code change only makes the problem you described a little easier to work around.  It does not directly fix that problem.  A slight rearrangement of conditions inside my code would directly fix the problem you described.

Since you have seen symptoms of the basic issue I described, does that mean you would like it corrected in hugin, and if so, is the basic approach I selected (corrected for the case you mentioned) acceptable?

[johnfi...@gmail.com]

On Thursday, February 10, 2022 at 7:00:05 PM UTC-5 johnfi...@gmail.com wrote:

 A slight rearrangement of conditions inside my code would directly fix the problem you described.

Since you have seen symptoms of the basic issue I described, does that mean you would like it corrected in hugin, and if so, is the basic approach I selected (corrected for the case you mentioned) acceptable?

I made that change, new version below.  I only tested a tiny amount, but including the case you mentioned that I hadn't tested before, which now works the way I assume most people would consider correct.

 // Inserts qualifying points int UIntSet &points, returns true if any found
// 1) Any number of points inside the rectangle m_dragStartPos to m_currentPos might qualify
// 2) If no points are in the rectangle, the single point nearest the rectangle, if near enough, might qualify
//     (unspecified which one in case of ties, but only one)
// 3) If considerSelectedOnly, only selected points actually qualify.
// Even if considerSelectedOnly is true, an unselected point may disqualify some selected point that is further

bool MaskImageCtrl::SelectPointsInsideMouseRect(HuginBase::UIntSet &points,const bool considerSelectedOnly)
{
    // compute one dimension distance outside range (zero if inside range)
    auto distanceHelper = [](double vmin, double vmax, double v)->double
    {
        return (v<vmin) ? (vmin-v) : (v<vmax) ? 0 : (v-vmax);
    };
    double selectionLimit = maxSelectionDistance*maxSelectionDistance;
    unsigned pending = UINT_MAX;
    hugin_utils::FDiff2D p1=applyRotInv(invtransform(m_dragStartPos));
    hugin_utils::FDiff2D p2=applyRotInv(invtransform(m_currentPos));
    double xmin=std::min(p1.x,p2.x);
    double xmax=std::max(p1.x,p2.x);
    double ymin=std::min(p1.y,p2.y);
    double ymax=std::max(p1.y,p2.y);
    const HuginBase::VectorPolygon poly=m_editingMask.getMaskPolygon();
    for(unsigned int i=0;i<poly.size();i++)
    {

        double xDistance = distanceHelper(xmin, xmax, poly[i].x);
        double yDistance = distanceHelper(ymin, ymax, poly[i].y);
        double d2 = xDistance*xDistance + yDistance*yDistance;
        if(d2 <= selectionLimit)
        {
            if(selectionLimit==0 && pending!=UINT_MAX)
                points.insert(pending);

            else
                pending = UINT_MAX;
            if( !considerSelectedOnly || set_contains(m_selectedPoints,i) )

[T. Modes]

johnfi...@gmail.com schrieb am Freitag, 11. Februar 2022 um 01:33:47 UTC+1:

I made that change, new version below.  I only tested a tiny amount, but including the case you mentioned that I hadn't tested before, which now works the way I assume most people would consider correct.

In this implementation the tolerance is depended on the sequence of the polygon points. First points of the polygon will be have a higher probability to be accepted because of the a higher tolerance. The latter points of the polygon will have a lower or even null tolerance to be accepted and will therefore earlier rejected. So this is even more confusing for a user.

  

[John Fine]

You have misunderstood the code.  It does not do what you say.

[T. Modes]

if(d2 <= selectionLimit)

…
selectionLimit = d2;

first point: point within original selection limit -> accepted

second point: inside strict rect -> selectionLimit=0

third point: point within original selection limit -> rejected because selection limit is now 0

[johnfi...@gmail.com]

In case someone chooses to test this (push several points of a mask close to each other, then try to select one to move or delete, etc.), I should point out that

I did not yet post the related correction to the code used when you CTRL click on a line to create a new point.  That is the same basic issue, but in a different function.  In practical use that bad behavior is less likely to matter than the bad behavior fixed by my code above.  So I thought posting both corrections together would be confusing.  So in testing be aware that the similar malfunction with CTRL down is actually different.