[Impeller] Curve components in stroke path use start directions as their initial offsets #46203
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
chunhtai
changed the title
This comment was marked as outdated.
This comment was marked as outdated.
|
hmm looks like I broke some golden again. will take a look |
This comment was marked as outdated.
This comment was marked as outdated.
chunhtai
changed the title
This comment was marked as outdated.
This comment was marked as outdated.
chunhtai
force-pushed
the
issues/135225
branch
from
62c37ac to
ed7435b
Compare
This comment was marked as outdated.
This comment was marked as outdated.
chunhtai
force-pushed
the
issues/135225
branch
from
ed7435b to
14f262d
Compare
This comment was marked as outdated.
This comment was marked as outdated.
chunhtai
force-pushed
the
issues/135225
branch
from
14f262d to
220c873
Compare
This comment was marked as outdated.
This comment was marked as outdated.
chunhtai
force-pushed
the
issues/135225
branch
from
220c873 to
b7857c3
Compare
This comment was marked as outdated.
This comment was marked as outdated.
chinmaygarde
changed the title
chunhtai
force-pushed
the
issues/135225
branch
from
b7857c3 to
6c74888
Compare
|
Golden file changes are available for triage from new commit, Click here to view. |
chunhtai
force-pushed
the
issues/135225
branch
from
6c74888 to
be3122d
Compare
|
Golden file changes are available for triage from new commit, Click here to view. |
| ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); | ||
| } | ||
|
|
||
| TEST_P(AiksTest, CanRenderThickCurvedStrokes) { |
There was a problem hiding this comment.
May I have some advice? Not entirely sure why this produce clipped golden
locally it runs fine
There was a problem hiding this comment.
I think this is just due to the max size of golden images. So if you draw the donut a bit smaller it should be able to capture the whole thing.
| } | ||
| }; | ||
|
|
||
| auto add_vertices_for_curve_compoent = |
There was a problem hiding this comment.
There are still one more issue with curve component. the overdraw from the few start points will draw outside of the start point. I would like to address this in a later pr.
The start of the curve is not exactly horizontal due to this issue.
| offset = Vector2{-direction.y, direction.x} * stroke_width * 0.5; | ||
| }; | ||
|
|
||
| auto add_vertices_for_linear_compoent = |
There was a problem hiding this comment.
| auto add_vertices_for_linear_compoent = | |
| auto add_vertices_for_linear_component = |
| ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); | ||
| } | ||
|
|
||
| TEST_P(AiksTest, CanRenderThickCurvedStrokes) { |
There was a problem hiding this comment.
I think this is just due to the max size of golden images. So if you draw the donut a bit smaller it should be able to capture the whole thing.
…o… (flutter#46334) This reverts commit 1d15b35.
…eir initial offsets
chunhtai
force-pushed
the
issues/135225
branch
from
be3122d to
f21166f
Compare
|
Golden file changes are available for triage from new commit, Click here to view. |
|
Huh, I just pulled down caed4a6 and I can't seem to reproduce the failed golden behavior for impeller_Play_AiksTest_CanRenderCurvedStrokes_Metal. Perhaps there's a golden tracking issue. 
|
|
Golden file changes are available for triage from new commit, Click here to view. |
chunhtai
added
the
autosubmit
…ions as their initial offsets (flutter/engine#46203)
…137180) flutter/engine@602b513...d6a48e9 2023-10-24 [email protected] [Impeller] Allocate exact descriptor count, populate in one go. (flutter/engine#47200) 2023-10-24 [email protected] [Impeller] Curve components in stroke path use start directions as their initial offsets (flutter/engine#46203) 2023-10-24 [email protected] Roll Fuchsia Mac SDK from OUWiWfUxyMyDOlEfA... to YqSO1OByhoexFJSCr... (flutter/engine#47273) 2023-10-24 [email protected] [Impeller] Enable MSAA for OpenGLES: Take 2. (flutter/engine#47030) 2023-10-24 [email protected] Manual roll Dart SDK from 901e92d10627 to 360370ff93b0 (3 revisions) (flutter/engine#47271) 2023-10-24 [email protected] Roll Dart SDK from 901e92d10627 to 360370ff93b0 (3 revisions) (flutter/engine#47269) Also rolling transitive DEPS: fuchsia/sdk/core/mac-amd64 from OUWiWfUxyMyD to YqSO1OByhoex If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
The golden changes should test the change
fixes flutter/flutter#135225
This is what it should look like
but because the first point offset uses the direction to the second point, the line draws like this
so it miss the yellow part( the reason the missing pixel in the original issue) and overdraw the purple part
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.