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

Patches from SolveSpace #19

Merged
merged 32 commits into from
Apr 15, 2024
Merged

Patches from SolveSpace #19

merged 32 commits into from
Apr 15, 2024

Conversation

rpavlik
Copy link
Contributor

@rpavlik rpavlik commented Dec 22, 2021

SolveSpace has been using this lib for quite a while, and has made some fixes. I've rebased it onto your tree and trimmed them, you might find these useful. Is there an automated test system we could verify these with, or what is your test procedure?

whitequark and others added 30 commits December 22, 2021 16:54
The old value was likely a copy-paste error.
QCad doesn't display any dimensions where it's set.
In principle there's an algebraic relationship between the DXF line
width value and line thickness in millimeters, and everything except
AutoCAD accepts any value from the range, but AutoCAD ignores all
values except a few chosen ones.
This forced API consumers to implement a lot of useless empty
functions for no good reason.
These are all in our default implementation of DRW_Interface, which
ignores everything it receives, so it's OK.
This caused a double free when deallocating DRW_Hatch.
It's more trouble than it's worth when supporting Windows. Instead,
require the caller to always use the appropriate overload of
std::[io]fstream constructor (possibly the wchar_t* one).
This makes polylines readable in AutoCAD again.
According to documentation $TDCREATE has code 40 and contains "Local date/time of drawing creation (see “Special Handling of Date/Time Variables”)".
When saving in the AC1006 format, several DIMSTYLE groups were
erroneously omitted.
Some standards mandate the default layer to be unused.
Before, the layer was hardcoded to "0" in the writer.
Found via codespell and grep
Found via `codespell`
Having a user-specified assignment operator but default copy-constructor
violates rule of 2/3/5/0.

Found by clazy.
getExtrusion and getName was missing const qualifiers.
Dimension entities support DXF codes 210, 220, 230 and and DRW_Dimension
already had a extPoint member, but the tags were never used when
parsing.
void setExtrusion(const DRW_Coord p) {extPoint =p;}
std::string getName(){return name;} /*!< Name of the block that contains the entities, code 2 */
std::string getName() const {return name;} /*!< Name of the block that contains the entities, code 2 */
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this could actually be the following: std::string const& getName() const {return name;} which would allow avoiding an unneeded copy in many use cases

@marevol
Copy link
Contributor

marevol commented Jan 5, 2022

I'll merge this if passing checks on GitHub Actions.

@hzeller
Copy link

hzeller commented Apr 14, 2024

Did the tests pass in the meantime ? :)

@marevol marevol merged commit 8c57f68 into codelibs:master Apr 15, 2024
marevol added a commit that referenced this pull request Apr 15, 2024
@marevol
Copy link
Contributor

marevol commented Apr 15, 2024

The tests did not pass, so this change has been reverted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants