-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Change how "Get Z value from project terrain" tool is presented in Mesh Editing #60709
base: master
Are you sure you want to change the base?
Conversation
🪟 Windows buildsDownload Windows builds of this PR for testing. 🪟 Windows Qt6 buildsDownload Windows Qt6 builds of this PR for testing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice change, the tool now blends nicely with the Transform Mesh Vertices panel!
Co-authored-by: Stefanos Natsis <[email protected]>
Co-authored-by: Stefanos Natsis <[email protected]>
Co-authored-by: Stefanos Natsis <[email protected]>
@JanCaha Thanks for the follow-up
Hum... The comment you are referring to was instead agreeing to add the option to the Z widget 🤔
So that means a user can enter value for X, Y and check "Get Z value from terrain" at the same time? Then, when is the Z value picked from the terrain: at the initial position (before moving xy), or after the move is done? We should clarify this in the GUI IMHO (and for docs). Would you know why "Apply transform" is disabled in the screenshot? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So that means a user can enter value for X, Y and check "Get Z value from terrain" at the same time?
Yes.
Then, when is the Z value picked from the terrain: at the initial position (before moving xy), or after the move is done? We should clarify this in the GUI IMHO (and for docs).
It doesn't really make much sense to get the z from the initial position, however we could clarify that on a tooltip
Would you know why "Apply transform" is disabled in the screenshot?
Apply transform is enabled after preview is clicked
if ( vertexTransformed ) | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm, I wonder if it is safe to skip vertices that failed the transform, or if mNewZValues
needs to be matching mNewXYValues
in size and we should just return false on exception.
if ( mZFromTerrain ) | ||
{ | ||
if ( terrainProvider ) | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Group those two?
if ( mZFromTerrain && terrainProvider )
{
|
||
try | ||
{ | ||
point = transformation.transform( vert.x(), vert.y() ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Harissou has a point, if calcX || calcY
we should be transforming mNewXYValues.constLast()
instead
Description
Changed the way how Get Z value from project terrain is presented to the user according to #60512 (comment).
The Z Value and Get Z value from project terrain checkboxes are mutually exclusive, so user can either select one or the other.
Works with both Preview Transform and Apply Transform.