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

preserve MDAL mesh metadata when editing mesh layers #57378

Closed
wants to merge 1 commit into from

Conversation

uclaros
Copy link
Contributor

@uclaros uclaros commented May 8, 2024

Description

When editing mesh layers, all MDAL mesh metadata are dropped from the modified file. This can break formats that rely on that metadata, like the mike21 format.

This PR makes sure the MDAL mesh metadata are read when loading the mesh and stored back to the output file when editing is finished.

@uclaros uclaros added the Mesh Related to general mesh layer handling (not specific data formats) label May 8, 2024
Copy link

github-actions bot commented May 8, 2024

🪟 Windows builds ready!

Windows builds of this PR are available for testing here. Debug symbols for this build are available here.

(Built from commit 7ce7d5e)

@github-actions github-actions bot added this to the 3.38.0 milestone May 8, 2024
@nyalldawson
Copy link
Collaborator

Can you add a test for this?

@uclaros
Copy link
Contributor Author

uclaros commented May 10, 2024

Can you add a test for this?

At the moment the MDAL mesh metadata are only used by the ply format which is not good for preserving them perfectly (eg comments can be renamed, leading/trailing whitespaces may be ignored etc)

There is a pending PR that adds metadata handling to mike21 format, I can add a test when this change lands into QGIS.

@nyalldawson nyalldawson added the Freeze Exempt Feature Freeze exemption granted label May 17, 2024
@nyalldawson
Copy link
Collaborator

Is the clang tidy warning correct here?

@uclaros
Copy link
Contributor Author

uclaros commented May 22, 2024

Is the clang tidy warning correct here?

This warning is about existing code and not related to this PR.

There seem to be calls to virtual methods during construction, but the proper method is called on runtime. Is that because the base class is abstract and has no implementation for them?

@nyalldawson
Copy link
Collaborator

@uclaros I'd add a "final" annotation to the virtual method override -- that should fix the warning

Copy link

github-actions bot commented Jun 6, 2024

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.

@github-actions github-actions bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Jun 6, 2024
Copy link

While we hate to see this happen, this PR has been automatically closed because it has not had any activity in the last 21 days. If this pull request should be reconsidered, please follow the guidelines in the previous comment and reopen this pull request. Or, if you have any further questions, just ask! We love to help, and if there's anything the QGIS project can do to help push this PR forward please let us know how we can assist.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Freeze Exempt Feature Freeze exemption granted Mesh Related to general mesh layer handling (not specific data formats) stale Uh oh! Seems this work is abandoned, and the PR is about to close.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants