[Flutter GPU] Remove Command/VertexBuffer usage from Flutter GPU. #55893
Conversation
bdero
force-pushed
the
bdero/fluttergpu-remove-command
branch
from
28b6e56 to
e4546c5
Compare
bdero
force-pushed
the
bdero/fluttergpu-remove-command
branch
from
3a5127d to
196c26e
Compare
| /// The number of elements to draw. When only a vertex buffer is set, this is | ||
| /// the vertex count. When an index buffer is set, this is the index count. | ||
| /// | ||
| virtual void SetElementCount(size_t count); |
There was a problem hiding this comment.
New method in RenderPass here.
| /// @return Returns false if the given buffer view is invalid. | ||
| /// | ||
| bool SetVertexBuffer(BufferView vertex_buffer, size_t vertex_count); | ||
| bool SetVertexBuffer(BufferView vertex_buffer); |
There was a problem hiding this comment.
This patch removes vertex_count from SetVertexBuffer. It gets set via SetElementCount now. (This value becomes the index count when an index buffer is present)
| using BufferUniformMap = | ||
| std::unordered_map<const flutter::gpu::Shader::UniformBinding*, | ||
| impeller::BufferAndUniformSlot>; | ||
| using TextureUniformMap = | ||
| std::unordered_map<const flutter::gpu::Shader::TextureBinding*, | ||
| impeller::TextureAndSampler>; | ||
|
|
||
| BufferUniformMap vertex_uniform_bindings; | ||
| TextureUniformMap vertex_texture_bindings; | ||
| BufferUniformMap fragment_uniform_bindings; | ||
| TextureUniformMap fragment_texture_bindings; |
There was a problem hiding this comment.
Uniform bindings are wiped out after each RenderPass::Draw call. Command was hanging on to this state between draws. This is similar to what Command was doing previously, except it also resolves flutter/flutter#155335 by tracking maps with unique binding identities.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
bdero
force-pushed
the
bdero/fluttergpu-remove-command
branch
from
cc0b90b to
a199692
Compare
This comment was marked as outdated.
This comment was marked as outdated.
|
All the bugs are finally fixed on this one. |
| /// bound texture using that sampler. | ||
| /// | ||
| const auto& sampler_gles = SamplerGLES::Cast(*data.sampler); | ||
| const auto& sampler_gles = SamplerGLES::Cast(**data.sampler); |
There was a problem hiding this comment.
Does this count as making you a two star programmer? :)
There was a problem hiding this comment.
One day I'll sneak three stars in and become a legend.
| static void BindIndexBuffer( | ||
| flutter::gpu::RenderPass* wrapper, | ||
| const std::shared_ptr<const impeller::DeviceBuffer>& buffer, | ||
| int offset_in_bytes, |
There was a problem hiding this comment.
It may not matter if these are internal APIs but using signed ints for values that can't be negative is scary :)
There was a problem hiding this comment.
Is there an alternative approach we should consider exploring with Dart FFI?
There was a problem hiding this comment.
dart::ffi methods support signed/unsigned types, and then you validate on the dart side of things. Or else check on the C++ and then cast.
There was a problem hiding this comment.
Ah yeah we should use those unsigned types, especially for enums. Added an issue here: flutter/flutter#157129
…157143) flutter/engine@09598f6...0bb6b1f 2024-10-18 [email protected] Move redispatch_event from FlKeyboardViewDelegate to FlKeyboardManager (flutter/engine#55941) 2024-10-18 [email protected] Remove FlKeyResponder and use the two responder classes directly. (flutter/engine#55925) 2024-10-17 [email protected] Reland "iOS: Migrate FlutterEngine to ARC" (flutter/engine#55937) 2024-10-17 [email protected] [Flutter GPU] Remove Command/VertexBuffer usage from Flutter GPU. (flutter/engine#55893) 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] 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
…utter/engine#55893) Resolves flutter#156764. Resolves flutter#155335. - Remove Command/VertexBuffer from Flutter GPU. - Separate vertex/index count into `impeller::RenderPass::SetElementCount(size_t count)` & accurately document its behavior. - Make Flutter GPU uniform/texture bindings re-assignable. - Remove unnecessary templates.
2 participants
Resolves flutter/flutter#156764.
Resolves flutter/flutter#155335.
impeller::RenderPass::SetElementCount(size_t count)& accurately document its behavior.