[Impeller] Start a more generic stroke tessellator for path #40690
Conversation
|
Argh, I had not rerun gn check on this, I'll fix up the build rules. |
|
This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again. |
|
|
||
| //------------------------------------------------------------------------------ | ||
| /// @brief A utility that generates triangles of the specified fill type | ||
| /// given a polyline. This happens on the CPU. |
There was a problem hiding this comment.
nit: what happens on the CPU? Polyline generation right?
There was a problem hiding this comment.
I'll fix this, it's an unintentional miss from copying and pasting.
| //---------------------------------------------------------------------------- | ||
| /// @brief Generates triangles from the path. | ||
| /// | ||
| /// @return A buffer view that will be filled with the tessellation result. |
There was a problem hiding this comment.
Seems to return a status?
| /// | ||
| /// @return A buffer view that will be filled with the tessellation result. | ||
| /// | ||
| Status Tessellate( |
There was a problem hiding this comment.
How does the synchronization work here? I give you a buffer view then once the completion callback has fired then this particular compute workload is done right?
There was a problem hiding this comment.
I'll update the doc here.
On Metal, synchronization just works between passes because we're not using MTLHeap allocations. If that changes there will need to be some fences/events handed around to make sure the passes are synchronized on heap resources. I assume for Vulkan we'll need that too.
The callback is if you otherwise need to synchronize on the GPU and don't want to use a blit pass, e.g. because you have a host visible buffer. This is how the unit tests are working right now.
impeller/compiler/compiler.cc
Outdated
There was a problem hiding this comment.
@chinmaygarde please review this
There was a problem hiding this comment.
(Particularly for the Vulkan change)
| @@ -13266,6 +13263,131 @@ | |||
| } | |||
| } | |||
| }, | |||
| "flutter/impeller/renderer/path_polyline.comp.vkspv": { | |||
There was a problem hiding this comment.
I have no idea why the changes above are happening. I tried commenting out chagnes in compiler.cc and they still were there.
| std::make_shared<fml::NonOwnedMapping>(impeller_scene_shaders_data, | ||
| impeller_scene_shaders_length), | ||
| std::make_shared<fml::NonOwnedMapping>( | ||
| impeller_tessellation_shaders_data, |
There was a problem hiding this comment.
Nit: Consider naming these something other than tessellation shaders since these don't setup tessellation pipeline stages in the rasterizer.
impeller/renderer/BUILD.gn
Outdated
| } | ||
|
|
||
| if (impeller_enable_compute) { | ||
| impeller_shaders("tessellation_shaders") { |
There was a problem hiding this comment.
compute_shaders perhaps because of the overloading if the tessellation term?
impeller/renderer/BUILD.gn
Outdated
| @@ -4,18 +4,84 @@ | |||
|
|
|||
| import("//flutter/impeller/tools/impeller.gni") | |||
|
|
|||
| impeller_component("renderer") { | |||
| impeller_component("allocation") { | |||
There was a problem hiding this comment.
This does more than just allocation and contains stuff like formats and descriptors. Perhaps //impeller/core or //impeller/device? (I like the former).
There was a problem hiding this comment.
I was originally going to call this "buffers" but it also has textures and formats and an allocator. Right now it's a target in //impeller/renderer still. I'm fine naming it whatever, moving to core
dnfield
added
the
autosubmit
auto-submit
bot
removed
the
autosubmit
|
auto label is removed for flutter/engine, pr: 40690, due to - The status or check suite Linux Web Framework tests has failed. Please fix the issues identified (or deflake) before re-applying this label. |
dnfield
added
the
autosubmit
This is the start of a more generic tessellator for strokes.
Still missing are:
Some other things that are still left to do here:
It does now support arbitrarily mixed path data and ingestion from a
Pathobject. I've tested it on one relatively simple path.