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

Change of hierarchy #75

Open
saardrimer opened this issue Apr 28, 2020 · 12 comments
Open

Change of hierarchy #75

saardrimer opened this issue Apr 28, 2020 · 12 comments

Comments

@saardrimer
Copy link
Contributor

I'm thinking of changing how PCBmodE processes input files and the process in which it generates the board.

PCBmodE v4 takes input files and processes each component one by one, creating class objects for each as it goes. Then it places them on the board (SVG).

So we have the following hierarchy:

module -> board -> component -> footprint -> shape -> svgpath

The idea is that footprint doesn't have any of the modifiers that of the hierarchy 'above' it (scale, rotation, etc.) so that it can be reused for multiple instantiations. That requires, however, to modify each footprint object according to those modifiers after it has been created. So the footprint hierarchy down to svgpath is mutable. This sort of worked.

So far with v5 I've made svgpath mutable so if we wanted to rotate it after it has been created, we need to create a new object with rotation applied.

I was working on rotation recently and things got a bit out of hand because the process/implementation isn't too clear and I think I also got myself confused. This clearly isn't good.

So my thinking is this. First create a dictionary structure of the board with all the processing done (defaults, global/board settings, etc.) and apply the modifiers defined across the hierarchy. So in the case of rotate we can easily stack it up and apply it at the dictionary level before creating any objects. Then the next step would be to create the objects from this dictionary where all objects are immutable.

I think that this will simplify the logic and make the code more understandable and maintainable. It will also help add future modifiers and hierarchy (like multiple modules perboard and then add panel on top of that. It's a significant change so I'd like to hear your thoughts, comments, and ideas.

@Neon22
Copy link

Neon22 commented Apr 28, 2020

Can I ask for a little more explanation of the final process you're aiming towards within and without Inkscape ?
E.g. So if the initial automatic process had created a transistor with a specific footprint and located it on the board - then how do you see a user moving/rotating it into a new position manually and how would that information get back up the structure to where it needed to be recorded ?

Would an autorouting pass then reroute any affected traces ? At what level of granularity are we talking ? How do yo imagine the users's process to be ?

@saardrimer
Copy link
Contributor Author

@Neon22 This goes beyond the scope of my proposed changes. What I'm suggesting won't affect how the user interacts with the design in Inkscape, it would only affect how the SVG is generated.

@bistromath
Copy link

Ok, a lot of this is thinking out loud, and it's from a place of mostly ignorance since I'm new to the codebase.

I think it might be better to look at transformations (rotation, translation, scaling) less as operations and more as properties of each instantiation. I think this sounds like what you're describing above, so let me restate it in terms that make sense to me.

An example: a Footprint object contains instantiations of a bunch of Shapes; say there are two rectangles on the top copper layer, and some silkscreen text. Each Shape is an instantiation of a Path, with transformations (rotation, translation, scaling) as properties of the instantiation. The Footprint in turn has transformations for each instantiated Shape. The hierarchy goes all the way up to Module.

To retrieve a final svgpath for the Module, you just traverse the hierarchy from the bottom up, applying transformations from the instantiations of Shape, Footprint, Component, Board, and finally Module. Nothing is mutable here, nothing modified in place. Objects can be constructed once and their subpaths reused -- for instance, once a Footprint is created, its svgpaths include the transformations applied to the underlying objects. The "rendered" Footprint should contain its own list of paths, separate from the immutable underlying objects. Thus the rendering operation doesn't affect underlying paths, it just creates new ones.

In fact, when I look at it like this, there's no reason for the Footprint to still contain the underlying objects (Shapes) and their associated properties. All it really needs is a list of paths which are to be modified by objects higher in the hierarchy (Component, in this case).

It doesn't seem that there's anything special about each level of the hierarchy, besides their name. They're all just collections of underlying objects. They all get processed in the same way. So an Object class could encapsulate all of this behavior, and any level-specific behavior added as an inherited class. This supports your idea of adding Module and Panel classes.

@saardrimer
Copy link
Contributor Author

@bistromath I think we're saying/thinking the same thing ;)

The main difference between the existing way and the proposed one is that the processing of the hierarchy and the creation of objects will be two distinct steps. This will allow the traversal for applying properties before creating the objects.

In fact, when I look at it like this, there's no reason for the Footprint to still contain the underlying objects (Shapes) and their associated properties. All it really needs is a list of paths which are to be modified by objects higher in the hierarchy (Component, in this case).

It doesn't need to contain the objects, but a list of shape properties. Is that what you mean?

@saardrimer
Copy link
Contributor Author

So to 'create' a board we'll have two steps from the top level (I think that's what @bistromath was getting at with the last paragraph):

  1. 'process' the entire hierarchy. Return 'dict'.
  2. 'render' the hierarchy. Return 'svg'.

This makes sense to me and will simplify the processing logically so it'll be easier to understand and maintain.

I've created branch issue-75 to start work on this.

@bistromath
Copy link

bistromath commented May 1, 2020

That works. I was thinking of doing it more as class objects, rather than a dict. Betrays my C++ background, I guess. But there's less syntactical magic involved, and because each level of the hierarchy is equivalent, all objects (shape, footprint, component, etc.) could be members of a base Object class or derived from it. Unless you meant that the dict is to contain class objects, referenced by their instantiation name as the key?

It occurs to me that the transform operations -- translation, scaling, and rotation -- are all linear transformations, and as such they are cascadable. So we don't necessarily need to go back to svgpaths each time we process an object -- a transformation matrix can be kept instead, and the matrix applied to the svgpath only once, at the end. Sure is a lot cleaner and easier to just do matrix multiplies, instead of transforming the svgpath again and again at every level.

Like you pointed out in the chat, the description needs to more clearly handle order of operations within a single level of the hierarchy. Should we lock it to "scale first, rotate second, translate third", since that's the most common use case for component placement? Is there a use case for the opposite order? Is there a use case for more complex cascades at a single level of the hierarchy? One could imagine a rectangle which is rotated, translated, and then rotated again to achieve rotation not about the center. Maybe this is esoteric. But it seems that transformations could be made into a list, rather than a set of properties. Rather than:

"rotate": 45, "position": (1.5,2.0), "scale": 2.0

...you could do:
"transforms": [ ["scale", 2.0], ["rotate", 45], ["translate", (1.5,2.0)]]

(edited, clearly i don't understand JSON very well)

...which would also let you do rotations about an arbitrary point, etc. These transforms could be cascaded by matrix multiplication exactly as they are at each level of the hierarchy -- it's all the same.

@saardrimer
Copy link
Contributor Author

We might be at a disconnect somewhere. Here's my current way of thinking about it:

  1. Combine all the shape definitions into a dictionary of the board/module/panel (while applying defaults, settings, etc.)
  2. Create Shape objects from shape definitions in that dictionary
  3. Create the SVG using these objects

What were you thinking of that has more Class? ;)

Like you pointed out in the chat, the description needs to more clearly handle order of operations within a single level of the hierarchy.

This is how SVG does it!

https://developer.mozilla.org/en-US/docs/Web/SVG/Attribute/transform

I was contemplating adopting this a few weeks ago and decided against it because I thought that the current way is cleaner and that it's too much of a departure, so I'd leave it for the next version.

Given the current discussion, however, I might reconsider. Matching up with SVG definitely has advantages in terms of familiarity, usability, and implementation.

@saardrimer
Copy link
Contributor Author

saardrimer commented May 1, 2020

I've just pushed

https://github.com/boldport/pcbmode/blob/issue-75/pcbmode/utils/create_svg.py

where there's a skeleton for the new process (it's not clean yet ;). You can work on it with rotate-test.json from this project:

https://github.com/boldport/gent-pcbmode-v5-test

It just processes the outline for now...

@saardrimer
Copy link
Contributor Author

I've decided to go the SVG way and do something like:

"transform": "translate(44,22) rotate(55,6,7) scale(2)"

Here's the transform parser regex ;)

    num = "[-+]?[0-9]*\.?[0-9]+(?:[eE][-+]?[0-9]+)?" 
    regex = f"(translate|rotate|scale|matrix)\(({num})(?:,?({num}))*\)"

@bistromath
Copy link

i really like that. seems like a JSON list might be cleaner, but that's just nitpicking.

@Neon22
Copy link

Neon22 commented May 1, 2020

FWIW keeping the transform in svg will make implementation easier. Using json might be good for storing but if inkscape, or one of the svg libraries, is going to do the transformation work then I agree with leaving it in svg. The primary reason for putting it in json would be for interoperability but I don't think that's the issue here.

@saardrimer
Copy link
Contributor Author

OK, so this turned into a major re-write ;)

It will be better code all around when I'm done, though. I'm trying to unify things (like shapes) with as few special cases as possible.

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

No branches or pull requests

3 participants