Skip to content

Commit

Permalink
Import missing styles from linked files.
Browse files Browse the repository at this point in the history
  • Loading branch information
@bcmpinc @whitequark
bcmpinc authored and whitequark committed Feb 19, 2019
1 parent 9e51288 commit 8d07a6b
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 1 deletion.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ Bugs fixed:
* Paste Transformed with a negative scale does not invert arcs.
* The tangent arc now modifies the original entities instead of deleting
them, such that their constraints are retained.
* When linking a sketch file, missing custom styles are now imported from
the linked file.

2.x
---
Expand Down
8 changes: 7 additions & 1 deletion src/file.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -732,7 +732,13 @@ bool SolveSpaceUI::LoadEntitiesFromFile(const Platform::Path &filename, EntityLi
} else if(strcmp(line, "AddConstraint")==0) {

} else if(strcmp(line, "AddStyle")==0) {

// Linked file contains a style that we don't have yet,
// so import it.
if (SK.style.FindByIdNoOops(sv.s.h) == nullptr) {
SK.style.Add(&(sv.s));

This comment has been minimized.

Sign in to view

Copy link
@Evil-Spirit

Evil-Spirit Feb 22, 2019

Collaborator

Strange behaviour. We should search not by id, but by fields of style comparing all the properties. When found exactly the same, we should remap style ids.

This comment has been minimized.

Sign in to view

Copy link
@whitequark

whitequark Feb 22, 2019

Contributor

Hm, true. Or maybe by name? That seems more predictable.

This comment has been minimized.

Sign in to view

Copy link
@Evil-Spirit

Evil-Spirit Feb 22, 2019 •
edited

Collaborator

Name is likely can be the same (for default styles) but contents - not. So, if we want to import exactly what we save (without messing styles), we should compare all the properties of style or (simplier way) we can just create new styles for every imported style (leads to duplicating the same styles), but we should do remapping in both ways.

This comment has been minimized.

Sign in to view

Copy link
@whitequark

whitequark Feb 22, 2019

Contributor

I think both of these behaviors make sense in different circumstances. If we unify by name then we can change styles in the imported file. If we unify by content we always get exact representation.

This comment has been minimized.

Sign in to view

Copy link
@bcmpinc

bcmpinc Feb 22, 2019

Author Contributor

It still makes more sense than the old behavior, which simply created default styles if they did not yet exist and also imported by ID.

Anyway, this implementation worked for my use case. I have ~100 sketch files, and those are imported by others up to 3 levels deep. In some of these imported files I have engraving patterns on the surface of objects, which I have given a separate style to make them extra visible. I then use 'export 2d section' to export those (which I've patched so it includes those lines with a custom style):
path6
Which I can then use on a laser cutter.

Sure, I have to keep in mind that all files use the same style id for the same type of line. But I'm okay with that. It helps keeping stuff organized. Importing by name would also work for me, however, that means we have to start enforcing unique names for custom styles.

Importing based on style properties would not work for me. As I can no longer easily say in a file "this should be an engraving line". That the lines might look different in different files is not an issue for me. I'm interested in what function the line has, not in its appearance.
}
sv.s = {};
Style::FillDefaultStyle(&sv.s);
} else if(strcmp(line, VERSION_STRING)==0) {

} else if(StrStartsWith(line, "Triangle ")) {
Expand Down

0 comments on commit 8d07a6b

Please sign in to comment.