-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Fix for issues #5422, #3411, and #5443 -- DXF insert scaling fix and colour fix #5426
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Edited out line suspected to be causing changes in position when changes in scale are made to inserted BLOCKS
Changed order of operations for insert positioning and scaling. Need to position the inserts before scaling it, otherwise the position ends up up being position*scale
seanth
changed the title
Fix for issue #5422 -- DXF conversions no longer incorrectly place inserts incorrectly when scale changes
Fix for issue #5422 -- DXF conversions no longer incorrectly place inserts when scale changes
Jan 18, 2024
The previous table was in an incorrect order, leading to index references in DXF producing the wrong colours when converted. Also added other entries to bring the total number of ACI colours up to the number that can be used in DXF files
seanth
changed the title
Fix for issue #5422 -- DXF conversions no longer incorrectly place inserts when scale changes
Fix for issues #5422, #3411, and #5443 -- DXF insert scaling fix and colour fix
Jan 26, 2024
- Use constexpr instead of const.
kimkulling
added
Bug
Global flag to mark a deviation from expected behaviour
DXF
Bugs related to the DXF format.
labels
Jan 27, 2024
kimkulling
approved these changes
Jan 27, 2024
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.
Looks fine.
Merged, thanks a lot for your contribution. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a very simple one line change.
In the previous version, if a dfx had multiple inserts that had different scales, assimp would place the insert in xyz space in locations that were incorrect. Outputs from assimp would have files that would be very different from what apps like sketchup or the online autodesk viewer would produce.
The bug was because scaling of the insert was being done before placement, resulting in the placement being placement•scale. For example, the insert would be placed at (pos.x•scale.x),(pos.y•scale.y),(pos.z•scale.z) instead of pos.x, pos.y, pos.z
Moving the line of code responsible for determining placement to before the line of code dealing with scaling fixes this.
I suspect this was an edge-case as the code has been in use for 10 or 12 years now and there hasn't been a bug report, but I suspect it explains why a subset of dxf viewers/converters out there mangle my files in exactly the same way. I think they relied on code from assimp