-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Enabling StrokeJoin for paths and rectangles. #10742
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
Conversation
Pulling from main fork.
Pulling to make workspace up to date wrt master.
Bringing master up to date wrt workspace
|
Also, this fixes #7199, and must be committed after the flutter/engine PR. |
|
There's a cost to having these examples, I'm not sure it's really worth it. Most people won't be writing code at this level, and it's not clear to me that this example explains the system in much more detail than the other examples at this layer that we already have. If we want them for pedagogical reasons, I think we're better off using a |
| Shader, | ||
| Size, | ||
| StrokeCap, | ||
| StrokeJoin, |
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.
this change is valuable
|
OK, I'm fine with just reverting the example and adding some sample code somewhere. What's the process for editing the docs? One reason I did the example was to show the popping of the miter joins so that it's clear that miter joins are a no-no on shapes where the angles are animated, but I guess they'll learn that pretty quickly. I did think about how to improve Skia's miter limit code so that it is both easier to understand what the units are for it, and also make it not pop. Basically, I'd just clip it at a length equivalent to the miter limit instead of popping to a bevel. It involved adding some extra math and an extra couple of vertices for clipped miters, and (most damning of all) it's a breaking change, and would probably screw up font rendering or something else, so I decided it wasn't worth it. |
|
The docs are all inline in the source. You already wrote the docs for this enum, this would just be a matter of adding more. You can see examples in other enums in that source file (e.g. TileMode and the others I cited above). I didn't follow what you were saying about popping from your comment above and definitely didn't get that from the source, so we should definitely document that, ideally with diagrams (any diagrams you generate should be generated from source, like the example I cited above, so that we can easily adjust them later and adapt their style to our evolving style). |
|
Oh, I see, sure. I was thinking you wanted it as part of a tutorial or something. I can put it into the source, no problem. Below is an animation of the popping (and the output of my sample code). Look at the top one, that's set to the default miter limit. It's actually "as designed" for Skia (and pretty much any other system that supports a miter limit), and it's fine as long as you're not animating the angle. [This is why I put in the comment about "popping" in the documentation for the API.] |
|
Ah, I see. How did you record the animation? Is it possible to include that in the docs? (Not that precise image, but a cropped one that shows the problem with a smooth cycling animation, say.) |
|
Just used screenrecord on adb, and converted it to a GIF. I'll clean it up and put it in the docs. |
|
Except how would you like me to automate generating that from source? Should I leave around the example code, or move it somewhere else (something like the curves documentation diagrams?) I can also make a script that will build it using |
|
Actually, that's silly. If I'm going to generate the GIF with imagemagick, I can just generate the frames with Flutter. |
|
See https://github.com/flutter/assets-for-api-docs for the way we autogenerate the screenshots today. I've never tried using adb screenrecord, that sounds cool. The key is that someone else should be able to regenerate the image with slight changes (e.g. changing the colors, or just running it again with a newer build to show newer antialiasing or to get a differently sized image or whatever), without having to think. If you look at the existing examples, you'll see they dump out the precise commands to run to get the PNG. |
|
Is there any image format support in Flutter/Dart that would help? Seems like what I'd ideally like to do is use Flutter to draw into a canvas and then render that into a GIF stream and write it out while controlling the animation clock ticks so it's smooth. I imagine that not all parts of that scenario are implemented... :-) |
|
Yeah, we don't yet have that. I've been getting the images using screen shots on actual devices. I believe we have the technology to do automated screenshots from simulators (cc @HansMuller) but I haven't tried using that yet. We don't yet have a way to take a Picture or Image and turn it into pixels or PNGs or something in a byte stream that can then be handled by the code. |
|
Hmm. Seems like an opportunity for a 20%er... :-) . What's the policy on using third party open-source code in plugins? Something that compiles on iOS and Android, and could be made into a Flutter plugin, like FreeImage? Or just a single format, like PNG, that's pretty simple... |
|
Well in due course we want to actually expose encoders as well. In the short term it's fine to use something like adb screenrecord and command line tools, so long as you say what the dependencies are and you give the commands that will regenerate the files. |
|
OK, that's what I'll do then, for now. |
|
I'll create the documentation and the diagram in another PR, if that's OK. (Well, I guess I have to, since it's in another repo). |
|
you bet! |
|
I'll land an engine roll then we can land this. |
|
LGTM on the one-line change that remains. |
|
I've landed the necessary engine roll; I'll land this once the tests clear. |
Merging changes from flutter/flutter
Updating my flutter fork to master.

This enables the Paint.strokeJoin and Paint.strokeMiterLimit properties added to the engine in PR flutter/engine#3777.
Also, added a new example that shows line join/cap types for various line-drawing primitives.