GTK shape support WIP #348
Conversation
… to mimick the SwiftUI api which also has it
| import CGDK | ||
| import TokamakCore | ||
|
|
||
| func createPath(from elements: [Path.Element], in cr: OpaquePointer) { |
There was a problem hiding this comment.
Perhaps this can be reused for clipping. I have an idea for a fun clipping hack:
It appears that if you connect a 'draw' signal handler, it will be called before the widget code gets a chance to draw.
This means that is should be possible to implement shape clipping by modifying the cairo context to add extra clipping for any kind of widget.
I assume that the context is 'restored' after drawing a child widget, so I expect the extra clipping to be automatically be cleaned up... Just guessing by reading docs though.
Also I think that some widgets may be drawing into their own context and not just reusing the shared window context... Will have to read up and experiment...
MaxDesiatov
added
GTK renderer
| @@ -139,6 +143,27 @@ public struct Path: Equatable, LosslessStringConvertible { | |||
| case let .roundedRect(fixedRoundedRect): return fixedRoundedRect.rect | |||
| case let .stroked(strokedPath): return strokedPath.path.boundingRect | |||
| case let .trimmed(trimmedPath): return trimmedPath.path.boundingRect | |||
| case let .path(pathBox): | |||
| // Note: Copied from TokamakStaticHTML/Shapes/Path.swift | |||
There was a problem hiding this comment.
Is this copied from that file or actually moved? Is the overall duplication of code reduced then?
There was a problem hiding this comment.
I made a single change. I'll take a look and see if I can get a better understanding of the existing logic and try to reuse the same logic both places.
There was a problem hiding this comment.
I think that there was an error in the previous sizing and that it's fixed in the Path.boundingRect function, but I am not 100% certain.
The previous elementsSize in StaticHTML/Shapes/Path.swift had:
return CGSize(width: abs(maxX - min(0, minX)), height: abs(maxY - min(0, minY)))
but I replaced that with
return CGSize(width: maxX - minX, height: maxY - minY)
because otherwise the size depends on the origin, which I guess it shouldn't - unless the usage of the size needs this special behavior.
But the similar special casing was previously not applied for .rect storage type - here the size was just the rect's size (no matter the origin) - so the change seems more consistent.
@carson-katri Do you remember a use case for the special case for the previous element path sizing?
There was a problem hiding this comment.
BTW, all our renderer-independent code in TokamakCore is easy to test and there's already a test target for it, which runs only on macOS for now. I'll make it run on Ubuntu too in a future PR. Thus if you think you've found a bug in something like this, please provide a test case. Then we can test the new code against it, if that makes sense 🙂
There was a problem hiding this comment.
Another comment about the _Sizing concept:
Right now built-in shapes like Rectangle are .flexible in size while custom Shapes are .fixed.
A custom shape may as well be flexible - it all depends on whether or not the func path(in rect:) returns a path that is relative to the rect or not...
I don't know if the _Sizing concept could be made obsolete somehow by doing something else entirely...
There was a problem hiding this comment.
I consider bringing the layout stuff in a separate PR from https://github.com/objcio/S01E235-swiftui-layout-explained-layout-priorities, would that be related in any way?
There was a problem hiding this comment.
I appear to get ok results by removing the Path._Sizing enum entirely and choosing the code paths that corresponded to the previous 'flexible' sizing...
That removes the need for the fixed size entirely - and I get quite similar results with SwiftUI and Tokamak(DOM) drawing shapes with fixed sizes - both when I create them using a 'Shape' or a 'Path' directly.
I am still uncertain if I may be lacking some context, but at least I suspect that this could be a way to go about it...
There was a problem hiding this comment.
If fixing this requires some substantial changes, I suggest doing it in a separate PR to make changes more granular and easier to track and review.
There was a problem hiding this comment.
Excellent. I’ll leave the sizing as it is for now. I don’t think it’s substantially more correct or incorrect than the previous version.
And regarding your question above which I just saw now:
Yes, my knowledge about the layout system indeed comes from the reverse engineering work of objc.io Swift Talks.
I really hope to get time to look into that for GTK.
I believe that the full layout system requires you to be able to measure view sizes before they are rendered - I think that can be done in gtk, but I am not familiar with how it might be done in HTML/JavaScript although there appears to be some hacks available.
…t gdk is installed as part of gtk when using brew or apt
…s applied, and added a mention to issue #322
| final class DualParamClosureBox<T, U, V> { | ||
| let closure: (T, U) -> V | ||
|
|
||
| init(_ closure: @escaping (T, U) -> V) { self.closure = closure } |
There was a problem hiding this comment.
It's a shame that Swift doesn't support variadic generics, or macros, or something of that nature that could simplify this boilerplate. I guess GYB is more hassle than it's worth, but we could try it later you want 🙂
There was a problem hiding this comment.
Right, there are quite a few good use cases for variadic generics. I hope that we'll see them one day.
I haven't looked too much into the various events/signals in gtk, but my guess is that they don't have callbacks with a whole lot of arguments... So until we see some more of them I guess this is acceptable.
SECOND UPDATE:
The final thing on my todo for this PR is to test the
Pathsize changes inTokamakStaticHTML.UPDATE:
I marked the PR as ready for review.
There are a few issues that I am uncertain of:
_ShapeView, so I have hacked around that for now, which is of course not very good.GSignalchanges sound? Do we just keep adding overloads ofconnectfor each new signal handler shape?_SubPathacceptable? (I think it makes sense).CGDKsystemPackage definition ok? It appears to do the trick (on macOS at least).I hope to get time to install virtualbox and play around with everything on Ubuntu too.
Adding support for
Shapein GTK usinggtk_drawing_area.Modified
Path.swiftfrom TokamakCore to support a few more path modifications.I also had to remove the
_SubPathtype in the process since I could not find a way for this to work together with path modification.The code proposed in this PR behaves (as far as I can tell) like SwiftUI in the following respects:
Adding path elements to an existing path will convert the storage to be the generic
.pathcase.Adding path storage building blocks to an empty path just sets the storage.
For instance:
adding a rectangle to an empty path will result in a path with .rectangle storage.
The current implementation of
PathBoxis just a class containing an array ofElement... I wonder what the intention of the type is in SwiftUI - I'd expect that aCGPathor an array ofElementdirectly as the associated value would suffice.Still lacking is:
.trimmed and .stroked are incomplete.
In SwiftUI it's possible to get the stroked path and the trimmed path (in fact it appears that the
.descriptionreturns a description of the 'rendered' path).Adding stroking and trimming is very, very hard. Stroking requires the possibility of offsetting a quadratic and cubic curve, which there is not a single correct way to do.
Trimming requires measuring the length of curves, which also turns out to be difficult (but can be done by approximating a curve with line segments) as well as splitting curves.
So basically the implementation requires a lot of what is already built in to
CGPath.I guess that it can be added eventually by getting inspiration from something like Cairo or other open source drawing APIs.
Since the current 'backends' can stroke paths, that part isn't a huge issue, but stroking a path twice would require the functionality.
Another issue with the current implementation is that in SwiftUI, a Shape can draw outside of the frame of the shape, but that is not currently possible with the gtk version.
UPDATE: a shape can now draw out of bounds - implemented by resetting the clipping area of the widget in the drawing routine. This could probably mess up some graphics updates. I will look for a better solution once I get started on mimicking SwiftUI view sizing and layout.