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

Stroke Information is Provided as Fill Like Information in Serialized Binary Documents when Boolean Operations are applied. #806

Open
johnoneil opened this issue Feb 16, 2024 · 5 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@johnoneil
Copy link
Contributor

johnoneil commented Feb 16, 2024

EDIT: Revising this as I find time to investigate.

I'm seeing an issue where a the paths for a Figma node which is the product of a boolean operation are more complex than expected. The stroke in this case seems to be replaced with a path describing fills. I'll add some images below that illustrate this better:

So we have an arrow which is the product of a boolean operation (here a union):

Screenshot from 2024-02-21 10-52-56

When rendering from the serialized binary document, the path provided for the fill look somewhat correct (the following image is just the fills):

Screenshot from 2024-02-21 10-20-11

But the stroke for this shape seems to describe some sort of fill. Here we're drawing the stroke information as a stroke (i.e. not as a fill).

Screenshot from 2024-02-21 10-21-43

So I believe the nature of this bug is that "sometimes (when boolean operations are applied?) Figma stroke information is represented by fill like paths in the serialized figma doc."

I'll do more research in this area when I'm focused on this next sprint.

@johnoneil johnoneil changed the title Serialized Figma Docs Lack Boolean Operation Nodes Stroke Information is Provided as Fill Like Information in Serialized Binary Documents when Boolean Operations are applied. Feb 21, 2024
@iamralpht iamralpht added this to the 0.26 milestone Mar 13, 2024
@iamralpht iamralpht added the enhancement New feature or request label Mar 13, 2024
@iamralpht iamralpht modified the milestones: 0.26, 0.27 Apr 17, 2024
@iamralpht
Copy link
Collaborator

@johnoneil Does this render incorrectly in the Kotlin runtime?

We have some complicated code for generating stroke paths from fills, or using the strokes that we got from Figma here: https://github.com/google/automotive-design-compose/blob/main/designcompose/src/main/java/com/android/designcompose/ComputePaths.kt#L173-L193

Behind the scenes, this is using Skia PathOps library to do the calculation. I know that Flutter has a recipe to build PathOps independently of Skia (it makes like a ~150KB binary) and I think PathOps is unique in what it does.

We need to be able to dynamically calculate stroke paths because layout can change the size of a stroked element... but there might be an opportunity to do more cleanup work during conversion from Figma to DCF format so that there's less to do at render time.

@johnoneil
Copy link
Contributor Author

@iamralpht Thanks Ralph. I have yet to verify this is correct in the Kotlin runtime but I expect it is.
I'll be doing a deeper analysis on this soon, and will check this design on Kotlin as part of that.
As yet I'm unsure if the information packed into the Design Compose serialized file is correct to do Kotlin similar behavior, but I'll determine that as part of this investigation.

@johnoneil
Copy link
Contributor Author

I took some time to go through this yesterday and doing a more methodical walk through the existing rendering code I'm able to accurately render strokes using the information provided in current serialized docs.

I still haven't done a clean port of the existing rendering code but was able to draw out where we're going wrong and implement some basic fixes. The areas where we were going wrong were:

  1. Realizing that packed stroke and fill paths are post-processed via ComputePaths.kt.
  2. Drawing "paths" as fills where necessary.
  3. Replicating path "overdraw and clipping" where necessary to do correct stroke widths in inside/outside/center cases.

Most of our issues went away from the above, but I still will work through the remaining issues next week.

Here's an image of the test case above with the above "fixes"

Screenshot from 2024-05-07 15-37-48

I'm as yet unsure why the outline isn't white, but I believe we still have some mixup in stroke vs. fill backgroudn colors.

I will probably close this issue as no-action when I work through what remains.

@iamralpht
Copy link
Collaborator

Hm, I'm not sure why the outline isn't white either, but if you have the element isolated in a doc then you can grab the JSON and see how we could be mis-parsing it. We don't have a DCF -> string dumper tool, but we probably should add one...

1, 2, and 3, are correct and necessary for correct stroke rendering. I think it might be possible to optimize this by using a mask layer instead of performing path ops to get the correct stroke path, though.

@johnoneil
Copy link
Contributor Author

I'll try to land a DCF-> string dumper in time. I've been using an internal trace of our display list construction, but analyzing the .dcf would be helpful.

For now I can see our incorrect colors here are due to unimplimented (or incorrectly implemented blend modes):

These are the two colors, one for fill and one for the path:

DL Path name:"Union": is_mask: false
Fill 0 Solid: Color { color: (53, 245, 60, 204) }
Stroke 0 Solid: Color { color: (255, 255, 255, 229) }

Both are slightly transparent, which I didn't expect. I believe correctly implementing blend modes (which I anticipate doing next month) will correct this

Will close this issue when I land my current fixes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Needs Triage
Development

No branches or pull requests

3 participants