Transect swap orientation

[lbartoletti]

Description

@nicogodet Follow up our discussion on #61545

@DelazJ after this pr, we can rewrite the documentation 😉

I'm not sure about the wording here.

[qgis-bot]

@lbartoletti
This pull request has been tagged as requiring documentation.

A documentation ticket will be opened at https://github.com/qgis/QGIS-Documentation when this PR is merged.

Please update the description (not the comments) with helpful description and screenshot to help the work from documentors.
Also, any commit having [needs-doc] or [Needs Documentation] in will see its message pushed to the issue, so please be as verbose as you can.

Thank you!

[github-actions]

🪟 Windows Qt6 builds

Download Windows Qt6 builds of this PR for testing.
(Built from commit f186cbc)

🍎 MacOS Qt6 builds

Download MacOS Qt6 builds of this PR for testing.
This installer is not signed, control+click > open the app to avoid the warning
(Built from commit f186cbc)

[nicogodet]

Thank you @lbartoletti!
I would have seen a combobox with 2 options Right to Left and Left to Right for better clarity.
Checkbox "Swap direction" is quite obscure as you don't necessary know the default direction so you need in every cases to check the documentation.

[lbartoletti]

Thank you @lbartoletti! I would have seen a combobox with 2 options Right to Left and Left to Right for better clarity. Checkbox "Swap direction" is quite obscure as you don't necessary know the default direction so you need in every cases to check the documentation.

Yes, it makes sense.

[uclaros]

Do you think this should be marked 'optional' to maintain compatibility with older scripts?

[lbartoletti]

indeed, good idea.

[nicogodet]

This is wrong.
pLeft and pRight are inverted.

Current behavior is Right to Left.
So currently, pLeft stores the right point and pRight stores the left point.

[lbartoletti]

So, it should be:

  if ( direction == QgsTransectAlgorithmBase::RightToLeft )
  {
    line.append( pLeft );
    line.append( pRight );
  }
  else
  {
    line.append( pRight );
    line.append( pLeft );
  }

[nicogodet]

I believe so
and rename pLeft and pRight ?

[nicogodet]

Can't write a suggestion but here is what I would do:

QgsGeometry QgsTransectAlgorithmBase::calcTransect( const QgsPoint &point, const double angleAtVertex, const double length, const QgsTransectAlgorithmBase::Side orientation, const double angle, const QgsTransectAlgorithmBase::Direction direction )
{
  // Transect is built from right to left relative to the reference line direction.
  QgsPoint pStart;  // start point of the transect
  QgsPoint pEnd;  // end point of the transect

  QgsPolyline transect;

  switch ( orientation )
  {
    case QgsTransectAlgorithmBase::Right:
      pStart = point.project( length, angle + 180.0 / M_PI * angleAtVertex );
      pEnd = point;
      break;

    case QgsTransectAlgorithmBase::Left:
      pEnd = point.project( -length, angle + 180.0 / M_PI * angleAtVertex );
      pStart = point;
      break;

    case QgsTransectAlgorithmBase::Both:
      pStart = point.project( length, angle + 180.0 / M_PI * angleAtVertex );
      pEnd = point.project( -length, angle + 180.0 / M_PI * angleAtVertex );
      break;
  }

  // Direction determines the transect orientation relative to the reference line direction:
  // - RightToLeft: transect goes from pStart to pEnd (default)
  // - LeftToRight: transect goes from pEnd to pStart (hydraulic convention - from left bank to right bank  looking downstream)
  if ( direction == QgsTransectAlgorithmBase::RightToLeft )
  {
    transect.append( pStart );
    transect.append( pEnd );
  }
  else
  {
    transect.append( pEnd );
    transect.append( pStart );
  }

  return QgsGeometry::fromPolyline( transect );
}

• Do not mention line to avoid confusion with the line from which we are creating transects, mention transect instead
• Use pStart and pEnd during transect construction (mention at beginning that transect is built from right to left relative to the reference line direction.

[github-actions]

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

• that all unit tests are passing

• that all comments by reviewers have been addressed

• that there is enough information for reviewers, in particular

  • link to any issues which this pull request fixes

  • add a description of workflows which this pull request fixes

  • add screenshots if applicable

• that you have written unit tests where possible
  In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
  If there is no further activity on this pull request, it will be closed in a week.