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

[WIP] Typeclasses all the way!!! #2

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

tg44
Copy link

@tg44 tg44 commented Aug 22, 2019

Hy!

First of all nice lib!

Second:
What I was not really liked here is the coupled solid-renderer code. When I tried to add new openScad capabilities, everything started to get messy.
So I started to refactor the code with Typeclasses, and bcs I reached a point when I see how it goes, I want to ask your opinion if its an improvement or a drawback.

The basic idea:
Instead of a big match-case in every renderer, we need to implement Renderer[A] classes. So we would know at compile time for example if JCSG could render a BeltMold.inner or not.

The pros:

  • We don't need inheritance anymore, everybody can implement everything in separate libs for example.
  • We can add multiple implementations to the same object if in some renderers there is a build in support for it (for example we can build a more optimal centered cube).
  • We know if the renderer can or can't capable of rendering the given object.
  • It's not breaking the already written (model) code (or at least it only needs minimal modifications check the test-code in the first successful render for reference).
  • We can create "renderer exclusive" Solids, so we could use for example already written openscad code with "copy-paste"

The cons:

  • The new "general model" codes are polluted with a lot of implicits (check the last commit for example)
  • IntelliJ sometimes highlights wrong (it hates the + operator)
  • More magical scala errors (bcs of implicit helpers of implicit converted implicit implicits :D )

Basically the first 3 commits are the interesting ones.

  1. The first is just baselineing with some outputs, so later I can cross-refactor the old and new renderers.
  2. The second is the openScad implementation with the new typeclasses, and the first test which was the WhiteboardMaker.top func "copy-pasted".
  3. The third was a gateway between the "old" renderers and the new one, the new classes can be implicit converted to the Solid counterparts, making the next refactors more friendly (it would be more nastier without it, but its still a mess)
  4. Making our first test using the same infra with both renderers
  5. More implicits
  6. A lot more implicits :D

The next steps if we move forward:

  1. copy + refactor the other baseline tests
  2. refactor all the utils
  3. refactor all the examples
  4. rewrite all the renderers
  5. remove all the .toSolid calls
  6. remove the old Solid, and old renderers

I would like to hear the authors voice before I move forward, or give up :D
I reached the point when I think that for the stuff I want to do, (and with the code I already written) I would be ok in a separate project too. But it would be nice if I could contribute to an existing thing and not reinvent a concurrent just bcs I was lazy to refactor :D

(Also this could be a good reading if you are not familiar with it: https://tpolecat.github.io/2015/04/29/f-bounds.html)

@dzufferey
Copy link
Owner

Thanks!

The current design is indeed pretty bad. It started as a quick hack when I started working on a Open CASCADE backend (https://github.com/dzufferey/scadla-oce-backend/). The common CSG core is shared but other operations are completely different. Unfortunately, that part is not really documented yet.

I did not know about the link you shared on f-bounds. After a quick glance that looks like a good solution but I have not yet had the time to read it in details.

I'll try to read the article and look at you pull request in the coming two weeks. I expect to accept the pull request because it looks like a good solution to the current hack. But I still want to read the code and understand it.

@tg44
Copy link
Author

tg44 commented Aug 25, 2019

Thanks for the reply!

In the last days, I didn't convince myself that this is the good way to solve this problem.
It has bright sides: when you add a new "thing" from the language you don't need to implement it everywhere (linear extrude was ~3 minutes). But it has a really dark side too, it comes with a LOT of implicit hackeries, which makes the whole thing messy and hard to understand...
And the top of that I tried to add heightmaps and that messed up everything :D (you need to pass the workdir somehow to the renderer too, so when you save the .dat it can be relative to the .scad).

I like this project, it gives tools to solve a problem, and give totally other kind of problems to solve :D

BTW: I never learned shapeless, but for me, it seems like the problem that maybe shapeless could solve too. When I will have time I will try to experience with that too to solve the "too much implicits" problem.

@dzufferey
Copy link
Owner

Sorry for taking time. I thought about this more. I think the typeclasses are parbably part of the solutions but not quite enough.

Ideally, what is needed is something that behaves well w.r.t. subtyping like the type classes but a bit more flexible like union type. I'm thinking that the union type could describe the operations used within a solid. This would allow different backend to support arbitrary subset of the operations. Your intuition of looking at shapeless is good. I don't know shapeless either but it seems that it can tackle this. I need to look deeper into this.

I started to look a bit more into what there is currently in terms of elements and backends.
Unless there is a comment, the element are supported by all the backend at the time of this writting.

  • primitives
    • Cube
    • Shpere
    • Cylinder
    • Polyhedron with triangular faces (Open CASCADE can deal with more genreal polyhedron and is not so good with many triangles)
    • Open CASCADE native shape (only Open CASCADE)
  • transform, mostly supported by all backends
    • translation
    • rotation
    • scaling (Open CASCADE only handles properly uniform scaling)
    • mirror
    • matrix (OpenSCAD, technically supported by Open CASCADE but not fully general)
  • operation
    • CSG: union, intersection, and difference
    • Hull (OpenSCAD and JCSG)
    • Minkowski (OpenSCAD)
    • Fillet (Open CASCADE)
    • Chamfer (Open CASCADE)
    • Offset (Open CASCADE)
    • any operation than can by applied to Open CASCADE shapes (Open CASCADE)

At that point, JCSG supports a subset of OpenSCAD and Open CASCADE is quite different.
However, this could change with the integration of a 2D subsystem for extrusion and projection.

Also there is the question of 2D over what. As far as I can tell, OpenSCAD is 2D with and underlying planar surface and OpenCASCADE, for some operations, can work with 2D manifold.

At the moment, I trying to learn more about shapeless.

@tg44
Copy link
Author

tg44 commented Sep 21, 2019

I tried shapeless, and I failed with it.

Out of the tutorial implementing some basic things this worked flawlessly:

case class Union[T <: HList](t: T)
case class Sphere(radius: Length)
def asdf = {
    Union(Sphere(5) :: Sphere(6) :: HNil)
  }

println(Renderable.render(asdf, 0))

The problem starts to appear when you try to migrate "dynamic" code like:

val c = Cube(length,sideWidth,baseHeight).move(-length, -sideWidth/2.0, 0)
val hexa = Union((1 to 6).map(i => c.rotateZ(Degrees(i * 60))) :_*)

You cant move a List[A] to a H :: T bcs you need to know the size of the list at compile time. (The upper code kinda solvable with a tuple, but than the map will be your next problem, bcs ypu can't add the c inside the mapping function so easily, and you need to build a whole spaceship just to solve this problem (or at least my current understanding its not easy, and you loose at least as many as you win))

I also tried to ask the community: https://users.scala-lang.org/t/set-of-implicits-automatically-or-more-implicit-implicits/5075 but not much help so far. (But at least you can check my repo :) )

I think union types will not work, or at least will not solve the core problem of why I get started with the whole refactoring idea. If the implementation you gave has a missing something (like heightmaps), you need to add that to the "union". I think the beauty of the typeclass solution would be that you can came up with anything and you can render it without the need to implement a renderer function for all language. Kind of inverse of control, you have a something, the renderer can render sth, and at compile time the language can tell you which renderer functions you need to write if you want to render your things. That would make the whole solution much easily extendable, and would give a lot of room for optimizations without killing the generalization and reusability. (For example we can have a centered cube, and we can center it by translations, or if the lang (for ex openscad) can render it natively, we can write an openscad specific renderer function for it which could be more optimal.)

If we start to use union types I think we close ourselves to the same box as we have now with the match-cases.

I'm still can't let this problem go, I try to bring in some programmers (in meetups and local chats), maybe sb has a good idea how I can create more implicit implicits :D

@dzufferey
Copy link
Owner

Hi,

Two days ago I was lucky to meet with some people involved in the Scala development (EPFL connections 😃 ). They old me that that dotty (Scala 3) is trying to tackle that issue and will provide native support for union types: https://dotty.epfl.ch/docs/reference/new-types/union-types.html I need to give that a try.

@tg44
Copy link
Author

tg44 commented Nov 3, 2019

I still didn't quite sure why union types will rescue as. I think union type will only help if we already know all the subtypes.

So if I want to union a cube and a sphere, the union case class should have a List[Union | Cube | Sphere] and when I want to add a new abstract I need to modify this list type, which kills the modularity.

On the other hand, if I wrap the Renderer[A] with an object A in a RenderableOps I can iterate in a List[RenderableOps] and render everything with the prepacked Renderer[A]. And If I came up with a new primitive, I just need to create a case-class and a Renderer[B] instance for it. If I have both of them in scope I can implicitly wrap them to a RenderableOps and everything is working properly. So this concept seems good on the surface. The case-class, and the renderer logic are separated (I can in-theory choose a new renderer to the class if I choose a new output syntax). But when I compile/run the code they are tied together again (as if the case-class would have a render function, also making sure that I use only objects whose I can render in the chosen syntax).
The problem cames when you don't want to define the renderer-syntax early, bcs you need to keep all the Renderable[A] implicits for every used object type on your implicit param list :(

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.

2 participants