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

MText Rotation Not Being Read #81

Closed
DJGosnell opened this issue Dec 8, 2022 · 6 comments · Fixed by #82
Closed

MText Rotation Not Being Read #81

DJGosnell opened this issue Dec 8, 2022 · 6 comments · Fixed by #82
Labels
bug Something isn't working

Comments

@DJGosnell
Copy link
Contributor

In issue #80, I ran across the issue with the rotation not being read on MText entities on both the DWG and DXF readers. See the sample file below here.

I'll start to dig into this when I have some time.

@DomCR DomCR added the bug Something isn't working label Dec 9, 2022
@DomCR
Copy link
Owner

DomCR commented Dec 9, 2022

In the Dwg Reader there is this line:

//X-axis dir 3BD 11 Apparently the text x-axis vector. (Why not just a rotation?) ACAD maintains it as a unit vector.
mtext.AlignmentPoint = this._objectReader.Read3BitDouble();

So I'm guessing that the rotation is calculated using the AlignmentPoint and the InsertPoint.

For the Dxf we need to detect if the angle is provided or not, and then calculate the AlignmentPoint (which it should be renamed as a vector) or the angle depending on which one is provided.

@DJGosnell
Copy link
Contributor Author

DJGosnell commented Dec 9, 2022

That is exactly the information that I need. Confirmed that the AligmentPoint and InsertPoint were both properly set to determine the angle.

Since the angle is determined If this is the case, is there any time that MText.Rotation is actually used? I see it has DxfCodeValue(50) attribute so I'm presuming that it is read at some point, but I feel like whoever uses this library needs to determine which value to use.

In this case, do we add any "helper" methods (or property) like MText.Get2dRotation and MText.Set2dRotation on the class to ease the use of this or do we just leave it up to the Library consumer to determine which to use (AligmentPoint and InsertPoint vs Rotation)?

I'm willing to do this if I have a better understanding if/when the Rotation DXF code is used.

I deal with 2D DWGs regularly so I forgot this was probably done this way to calculate the rotation in 3 dimensions...

@DomCR
Copy link
Owner

DomCR commented Dec 9, 2022

It seems that the DWG works only with the AlignmentPoint because checking the documentation the rotation is not read at any point, so for DWG when AlignmentPoint is set, the Rotation automatically should get it's value.

For DXF it seems that you have both options, but I presume that both values must be coherent with each other, if not, Autocad just picks the last value read:
Documentation: "A group code 50 (rotation angle in radians) passed as DXF input is converted to the equivalent direction vector (if both a code 50 and codes 11, 21, 31 are passed, the last one wins). This is provided as a convenience for conversions from text objects"

For me the best way to approach this issue would be to to modify the { get; set; } for the Rotation and the AlignmentPoint being this last one a normalized vector:

First scenario:
The costumer/reader sets the Rotation, the set calculates the new AlignmentPoint and changes the value so both of them are coherent with each other.

Second scenario:
The costumer/reader sets the AlignmentPoint, the set calculates the new Rotation angle and sets the new value.

This way both values are always linked and they will "make sense" at any moment.

This is the solution that I propose but I accept any ideas or alternatives.

@DomCR
Copy link
Owner

DomCR commented Dec 9, 2022

IMPORTANT NOTE:
I've notice that the common fields for TextEntity and MText have the same text in the xml documentation, so for MText the documentation is not valid and needs to be updated, the branch linked to this issue will refactor this class.

@DJGosnell
Copy link
Contributor Author

Excellent. Thank you for digging into this. I agree that modifying the properties set methods is the best way. These are pretty high priority for me so I will go ahead and make a PR for this modification less the documentation.

@DomCR DomCR linked a pull request Dec 9, 2022 that will close this issue
@DomCR
Copy link
Owner

DomCR commented Dec 9, 2022

PR created, feel free to push your branch to the PR's issue if you want, I'll give a second look before merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants