-
Notifications
You must be signed in to change notification settings - Fork 48
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
Added basic transformations, reversing #46
Conversation
I didn't bother making proper matrix changes, but it won't do skew or variant scale (with different x vs. y scales). I didn't bother adding some rather common elements like scale(x,y,px,py) which scales with respect to the given position. really it's |
Wow. That's coverage!
|
Also, Arc is somewhat hard to capture. It seems like it needs to use just one parametrize method and stick to that. It doesn't actually work well with internal data manipulation. |
Thanks for this, I will review this as soon as I can, don't despair at my slowness. Loads of things going on now. |
Np. The most suspect code is the Arc. There's not a real obvious way to only internally modify one of those. Even though you could easily modify the points with the invariant scale and and translate kind of complex math there, there's no clear and obvious way to fix everything again except perhaps by rebuilding from the internal values of center, start, and end. It actually appears to reuse both parameterizations and as such internally it's basically going to need to reparameterize everything without really having paid attention to the operation performed. Everything else was just actual points and could just be rotated, scaled, or translated without issue. And if the idea is to use the stuff with svg, it'll actually need some functions that break the point values into .real and .imag and modify them independently like for variable scales or skews. At which point they might as well just be your own rolled up point classes, since the cleverness of calling the complex plane the x,y real plane basically only achieves a bit of confusion at that point |
I actually needed this code for a production thing and hit some snags. https://github.com/meerk40t/meerk40t/blob/master/path.py And with the hybrid parameterization this is basically death. You can't really scale them accordingly. There's no plausible way to do skew or rotate when you have a rotation, rx, and ry. You could apply a skew and change and the rx and rotation. The same is true if you apply a variant scale. And have mercy if you apply an unknown matrix alteration. The only plausible way I could come up with to do all of these elements was to store 5 points. Start, End. Center, Point_rx, Point_ry and the the sweep which I kept in tau-radians (no point can really tell you which way or how many times you went around the ellipse). Then when I need the I also had to give it a real Matrix and Point classes without the whole using a complex trick. It still takes them as inputs but they get sent to point which cuts them up into x and y. And my use also had a strong need for a set of line and bezier plotters and so Move() as a turns out to have real need to actually store where it's moving from since I needed them to make the shapes entirely continuous even when they were actually discontinuous. Moves work like lines that don't draw anything. |
self.start -= other | ||
self.end -= other | ||
|
||
def __mul__(self, other): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although multiplications on complex numbers are cool, they don't really translate into anything particularly useful in graphics. I don't think we need it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Multiplication does scaling. If you take that element and you multiply by 2 it scales by 2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It also moves it by two, so that's not what you want in most cases.
I think we probably should implement this by first implementing the matrix transformation, and then possibly implementing others as shortcuts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't move it by 2. It multiplies the .imag and .real by that value which is a uniform scaling operation, relative to the origin. To scaled based on a given point you'd subtract by a complex, multiply by a real, then add the original complex again.
(x + yj) * s = (sx, sy). That's a properly scaled point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, and that's what is necessary. If you have a path that makes a square on point 1+1j, 1+2j, 2+2j and 2+1j and you multiply that by 2 you get the points 2+2j, 2+4j, 4+4j and 4+2j.
That's scaled AND moved, and that's not a useful graphical operation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@regebro, nope. That's scaled.
You are confusing scaled with scaled relative to a location. Whereas the operation scaled is relative to the origin. To perform a scale relative to the center you would do path
path -= 1.5 + 1.5j
path *= 2
path += 1.5 + 1.5j
That is scale and scale is by definition a very useful graphical operation. I think the confusion is in thinking scale cannot move elements. It totally can. I'm not sure how you would make all point relative to other points larger without moving the points around.
A rectangle at (1,1), (1,2), (2,2), (2,1) scaled by 2 has points (2,2), (2,4), (4,4), (4,2) that's not an argument for wrongness, that's the definition of scaled in a geometric sense. All scale operations are relative to the origin. You need to do "transform(-x,-y), scale(s), transform(x,y)" to do this relative to a point but that's really just move the origin, scale, move the origin back.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you want to wrap your head around it, consider the distance between any point and any other point in the original versus scaled version. You will find they are in every case exactly twice as far away.
from svg.elements import *
print(Path("M1,1 1,2 2,2 2,1z") * "scale(2)")
M 2,2 L 2,4 L 4,4 L 4,2 Z
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have wrapped my head around it. Thanks.
self.start += other | ||
self.end += other | ||
|
||
def __sub__(self, other): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And this is just a negative transform, really...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is, but also dunder methods would allow you to say:
new_line = line - (6+3j)
Which is generally easy to understand as a command as to what you'd want.
Generally I think the Arc code suffers problems that, currently with this implementation make everything impossible. I had to completely redesign it, because the values used for it are basically impossible to multiply with a matrix. And the use of complex numbers limits the valid matrix operations to translation and uniform scaling. But, for arc this either has to be done in a form that can be transformed or the operation becomes highly complex. I ended up parameterizing arcs as 5 points. Center, rx, ry, start, end. And I stored the value of sweep. And even the sweep value storage had issues when matrix multiplied with a scale_x or scale_y value that was negative it required the sweep be inverted too. I also took a look at the unit-circle matrix definition but didn't use it. Basically an arc is defined by an affine transformation matrix of what is assumed to be a unit circle. xx + yy = 1. The best I could do with the tools given there was unit scaling and translation. This is equal to multiplying by real numbers and adding complex numbers to all the points. The arcs, as you have them, are dual parameterized in both center and path parameterizations. Which may not be correctly reparameterized. |
I think this code doesn't really go far enough or would require some radical changes to get it to work fully. I had intended to pitch MeerK40t's path code in a spin off to you here. It might be too radical of changes though, or be better off as a switch-out replacement bit of code. And contains a bunch of stuff like Bresenham Bezier Curve Plotters which I doubt will be highly needed in general. I tried to maximize the backwards compatibility here. So while I added a Point class, the class is fully compatible as complex numbers. Most of that was so I could use your error checking suite to make sure everything worked fine. The complex numbers trick is really nice, but it starts to fail when you need all the proper operations that a matrix provides, operations which SVG paths kind of requires. I even went to the trouble of rewriting my original matrix class to use the same form SVGMatrix is supposed to use per the spec. And I added a bunch of other SVG parsing stuff since my project kind of requires a lot of fully realized SVG stuff. So it does things like matrix equivalence of the viewport which is given in the SVG spec. This pull here, will do uniform-scaling and translation. And other than the arc parameterizations, got all the low hanging fruit. There's a lot more that can be done. But, maybe it would be better to limit the scope of the project. The matrix transformations that aren't covered by simple complex number operations would need to have the complex numbers turned into points, then fed through a bit of proper matrix code, then turned back into complex numbers. But, doing that would require importing Point and Matrix and doing some conversions back and forth but at that point the methods I did in MeerK40t's path.py might be easier and more reasonable. I think I could do a better job at the arc code. I fear I may not have done enough to fix the inherent problems in not having a consistent parameterization. Though my solution would be to convert the arc to a different arc parameterization apply the transformation operation and then solve for all the parameters again. But, doing that could at least catch all the low hanging fruit, namely you'd be able to easily do the two affine transformation operations that conform to 4-function operations of complex and real numbers (uniform scale, translation). There is however a clear argument for doing nothing rather than doing something that cannot be done completely or correctly, especially as doing the complete code would require a few more things like a point class and and a matrix class (both of which conform to SVGPoint and SVGMatrix). Transformations could be moved to a different project that simple serves to extend this one, but doing that would require that this project decide on one rather than two different parameterizations for the Arc objects. You would basically want to parameterize it one way, and just provide |
OK, thanks for trying! |
@regebro I'm making a different project based on the work. With much more robust svg parsing, and better spec conforming. https://github.com/meerk40t/svg.elements There's not an easy way to overcome the arc parameterization problem without a rewrite. I could code you up a functional matrix that could work with complex numbers but you'd still hit a wall at arc. Even when I break the real and complex parts apart, and then perform the matrix multiplication stuff. But, applying that to arc would still be broken because what that matrix does to the parameters differs greatly by the parameter and the operation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, I'm not convinced, but good luck.
There might be some other elements in Arc that I missed with the transformation. The dual parameterizations being both used is a problem there. But, for the most part added some overloads for what it means to multiply or add something. And also added reverse stuff. Since I was bothering.