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

[MappingApplication] Fixing pure 2D geometry in 2D-3D projection mapper #12847

Conversation

loumalouomega
Copy link
Member

📝 Description

The 2D/3D mapper performs projections based on a normal, which is computed in the 2D mesh. Previously, I was supplying a 2D mesh in a 3D space. However, when the space is 2D, the mapper fails due to an error in the Normal implementation. This error occurs when the geometry checks if the local space is smaller than the working space.

To fix this, I added a check to determine if the geometry is a pure 2D mesh. If so, a normal vector is now explicitly defined in the Z direction, ensuring the projection works correctly even in 2D space.

🆕 Changelog

@loumalouomega loumalouomega added Applications Hotfix FastPR This Pr is simple and / or has been already tested and the revision should be fast Bugfix labels Nov 14, 2024
@loumalouomega loumalouomega requested a review from a team as a code owner November 14, 2024 08:20
@ddiezrod
Copy link
Contributor

approving as this looks as a small detail and its kind of urgent for us. If you have any comments, we can create a new PR @philbucher

ddiezrod
ddiezrod previously approved these changes Nov 14, 2024
@loumalouomega
Copy link
Member Author

@roigcarlo do you think it is possible to make a minor, tiny, little, release?

@roigcarlo
Copy link
Member

Once it's merged, no problem

@loumalouomega loumalouomega merged commit 8072010 into master Nov 14, 2024
11 checks passed
@loumalouomega loumalouomega deleted the mapping/fixing-pure-2d-geometry-in-pure-2D-3D-projection-mapper branch November 14, 2024 16:42
@loumalouomega
Copy link
Member Author

@roigcarlo this is merged. It is actually only two commits. Due to a merge conflicts it looks like more:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Applications Bugfix FastPR This Pr is simple and / or has been already tested and the revision should be fast Hotfix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants