[Impeller] give geometry classes similar API as entity / implement strokepathgeom #36693
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
Converting to a draft for now, I pushed up what the changes would look like with (most of) SolidStrokeGeometry implemented |
jonahwilliams
changed the title
|
|
||
| auto options = OptionsFromPassAndEntity(pass, entity); | ||
| if (!color_.IsOpaque()) { | ||
| if (geometry_result.prevent_overdraw) { |
There was a problem hiding this comment.
I think this is the right check :)
Once we merge solid fill + stroke, we'll need to retain these clips/options I'm assuming.
bdero
left a comment
bdero
left a comment
There was a problem hiding this comment.
Great! This LGTM assuming AiksTest.SolidStrokesRenderCorrectly and EntityTest.StrokeCapAndJoinTest are looking good.
|
|
||
| auto options = OptionsFromPassAndEntity(pass, entity); | ||
| if (!color_.IsOpaque()) { | ||
| if (geometry_result.prevent_overdraw) { |
| ISize render_target_size) = 0; | ||
| virtual GeometryResult GetPositionBuffer(const ContentContext& renderer, | ||
| const Entity& entity, | ||
| RenderPass& pass) = 0; |
There was a problem hiding this comment.
Passing around the ContentContext & Entity SGTM. The filter graph started off with minimal context being passed around and quickly ended up in a similar place.
|
I've verified everything is working correctly on those tests and updated to ToT. Going to land this now so we can close out this refactor. If anyone notices a problem with strokes though, this PR is the culprit 😆 |
jonahwilliams
added
the
autosubmit
…plement strokepathgeom (flutter/engine#36693)
3 participants
While working on making a StrokePathGeometry, I found that I needed to update the API again to provide more data for the smoothing approximation: https://github.com/flutter/engine/blob/main/impeller/entity/contents/solid_stroke_contents.cc#L213
Rather than pass through each object we need one at a time, lets just pass the same context that the Entity::Render method gets, at least for now.