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

Rework Multileader #292

Open
7 tasks done
mme1950 opened this issue Feb 29, 2024 · 6 comments
Open
7 tasks done

Rework Multileader #292

mme1950 opened this issue Feb 29, 2024 · 6 comments
Assignees
Labels
bug Something isn't working

Comments

@mme1950
Copy link
Contributor

mme1950 commented Feb 29, 2024

We recently implemented classes and a reader to handle MultiLeader entities in ACadSharp.

Using the MultiLeader data we learned a lot about the confusing data structures and values. So we try do document the properties to support usage.

And we found some issues, in particular bad or inconsistent naming:

  • MultiLeader.EnableDogleg always true #273
  • Misspelled enum identifier in MultiLeaderPropertyOverrideFlags
  • MultiLeader/MultiLeaderAnnotContext: TextAlignment property etc.
  • MultiLeader/MultiLeaderAnnotContext: AlignmentType, BlockContentConnection property.
  • MultiLeaderAnnotContext: bad property visibility. no setters for IList properties
  • MultiLeaderAnnotContext: Property naming
  • MultiLeaderAnnotContext: Should Location, Rotation doubled to TextLocation, TextRotation and ContentBlockLocation, ContentBlockRotation?

Note: This issue infers a breaking change.

Branch created: https://github.com/nanoLogika/ACadSharp/tree/20240225_mme_MultiLeader_rework

@mme1950 mme1950 added the bug Something isn't working label Feb 29, 2024
@mme1950
Copy link
Contributor Author

mme1950 commented Feb 29, 2024

TextAlignment etc.

The standard value for the TextAlignment property is defined in the MultiLeaderStyle object in the field with the DXF group code 176. This field can have the value 0. So it can be assumed that the enum values are 0=Left, 1=centered, 2=right.

The MultiLeader entity has a field with the DXF group code 175 which is unknown according to the OD_S_DWG documentation, but documented as Text Alignment Type in the DXF reference AC1024 and newer. This field can have the values 0, 1, 2. So it can be assumed that the enum values are 0=Left, 1=centered, 2=right.

A field with the same meaning is found in the MultiLeaderAnnotationContext with the DXF group code 176 documented as Text Align Type with the enum values 0=Left, 1=centered, 2=right in the OD_S_DWG documentation.

Both should be named as TextAlignment.

The multileader entity has a field with the DXF group code 179 documented as Justification in the OD_S_DWG with the values 1=Left, 2=centered, 3=right, documented as Text Attachment Point in the DXF reference.

A field with the same meaning is found in the MultiLeaderAnnotationContext with the DXF group code 171 documented as Alignment with the enum values 1=Left, 2=centered, 3=right in the OD_S_DWG documentation, documented as Text Attachment in the DXF reference.

Both should be named as TextAttachmentPoint.

In AutoCAD the text alignment can be specified in MutliLeaderStyle dialog.
In the property grid of the multileader entity appears one field Alignment.

In DWG and DXF the field with the group code 175 in the multlieader has always the value 0=left, regardless of the Alignment selected in the property grid.

MultiLeaderAnnotationContex.TextAlignment returns the value set in the property grid. We found in all samples that the values returned by MultiLeader.TextAttachmentPoint and MultiLeaderAnnotationContex.TextAttachmentPoint are equal and consistent with MultiLeaderAnnotationContex.TextAlignment.

@mme1950
Copy link
Contributor Author

mme1950 commented Feb 29, 2024

Location, Rotation

DXF uses different group codes for

  • Text Location: 11, 22, 32
  • Block Content Location: 15, 25, 35
    and
  • Text Rotation: 42
  • Block Content Rotation: 46

We decided to implement one Location property for both, and one Rotation property for both.

@DomCR Do you think this is OK?

@mme1950
Copy link
Contributor Author

mme1950 commented Mar 1, 2024

MultiLeaderStyle/group code 177
MultiLeader/group code 176
MultiLeaderAnnotContext/group code 177

are documented as Attachment Type or as Block Content Connection with enum values
AttachmentType: CenterExtents = 0, InsertionPoint = 1 or
BlockContentConnectionType: BlockExtents = 0, BasePoint = 1

are associated with the MultiLeaderPropertyOverrideFlags.BlockContentConnection flag.

--> Use equal property name in BlockContentConnection in MultiLeaderStyle, MultiLeader, MultiLeaderAnnotContext
--> Use BlockContentConnectionType, AttachmentType is deleted.

@DomCR
Copy link
Owner

DomCR commented Mar 4, 2024

Location, Rotation

We decided to implement one Location property for both, and one Rotation property for both.

I don't think is a good solution for the project, but if we have the certainty that this properties are exactly the same we could implement 2 private fields and return the same for each one of them (the same logic would be applied for the set).

The main reason why I think that we should not merge them is because the DxfMapper may not work as expected if we add multiple codes to the same value.

We also had a similar issue with MText:

In this case we though that Rotation and Alignment point were correlated but at the end that was not the case.

Let me know what you think about my approach.

@mme1950
Copy link
Contributor Author

mme1950 commented Mar 5, 2024

A MultiLeader has ether a text label or a block content having a Location and a Rotation.
TextLocation and BlockContentLocation are never relevant at the same time and they are not related.
I expected that the DxfMapper uses the group-code assignment, but I did not know.
It is no problem to implement separate properties for the text-label case and the content-block case.

@mme1950
Copy link
Contributor Author

mme1950 commented Mar 5, 2024

When this issue is completed we will have renamed several properties.
But I think this is acceptable

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

No branches or pull requests

2 participants