[Impeller] use primitive restart for faster tessellation: write directly into host buffer. #56173
Conversation
|
|
||
| /// @brief A vertex writer that generates a triangle fan and requires primitive | ||
| /// restart. | ||
| class FanVertexWriter : public VertexWriter { |
There was a problem hiding this comment.
This will only work for 16-bit index types. Perhaps you can take a void* index buffer and an IndexType argument to make this utility more generally usable. The restart marker you select can then depend on the index type passed in. Or you could template it.
This looks like a generally useful utility and its in the common geometry classes, so making it only work with 16 bit index types seems odd. And there are very guardrails against someone casting the wrong pointer type to uint16_t.
There was a problem hiding this comment.
We only use 16 bit index types though. If we need to make this configurable we can template-tize it if needed.
impeller/geometry/path_component.h
Outdated
| size_t count_ = 0; | ||
| size_t index_count_ = 0; | ||
| size_t contour_start_ = 0; | ||
| Point* point_buffer_; |
| if (count_ == 0) { | ||
| return; | ||
| } | ||
| index_buffer_[index_count_++] = 0xFFFF; |
There was a problem hiding this comment.
Probably being paranoid, but can you double check that the restart markers are the same in all APIs?
There was a problem hiding this comment.
Nevermind, I checked that they are.
There was a problem hiding this comment.
I think in GL you can configure a specific restart value with some APIs but I hope to never need that
There was a problem hiding this comment.
Isn't that desktop GL? On ES 3.0 and above, you specify GL_PRIMITIVE_RESTART_FIXED_INDEX to glEnable and get the same behavior as Metal and Vulkan.
There was a problem hiding this comment.
You can't configure the index in ES.
| return false; | ||
| } | ||
|
|
||
| bool CapabilitiesVK::SupportsPrimitiveRestart() const { |
There was a problem hiding this comment.
You can do the same thing unconditionally for Metal. Metal enables primitive restart by default without any additional configuration. In fact, it don't think it is possible to disable it.
There was a problem hiding this comment.
This is wired up for metal in this PR.
jonahwilliams
changed the title
|
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. |
jonahwilliams
added
the
autosubmit
…rite directly into host buffer. (flutter/engine#56173)
…rite directly into host buffer. (flutter/engine#56173)
…rite directly into host buffer. (flutter/engine#56173)
…rite directly into host buffer. (flutter/engine#56173)
…rite directly into host buffer. (flutter/engine#56173)
…rite directly into host buffer. (flutter/engine#56173)
…rite directly into host buffer. (flutter/engine#56173)
…rite directly into host buffer. (flutter/engine#56173)
…rite directly into host buffer. (flutter/engine#56173)
…157926) flutter/engine@c40b0b6...f2154ef 2024-10-31 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Skwasm single threaded (#56206)" (flutter/engine#56264) 2024-10-31 skia-flutter-autoroll@skia.org Roll Skia from 4f8f2ecadfb6 to 3c628426f85f (1 revision) (flutter/engine#56261) 2024-10-31 skia-flutter-autoroll@skia.org Roll Skia from 7e79a516284b to 4f8f2ecadfb6 (1 revision) (flutter/engine#56255) 2024-10-31 skia-flutter-autoroll@skia.org Roll Dart SDK from 6a8058eef22c to f3e3dc44b1dc (1 revision) (flutter/engine#56253) 2024-10-31 skia-flutter-autoroll@skia.org Roll Skia from 3c62d4a94d78 to 7e79a516284b (1 revision) (flutter/engine#56252) 2024-10-31 jonahwilliams@google.com [Impeller] use primitive restart for faster tessellation: write directly into host buffer. (flutter/engine#56173) 2024-10-31 jacksongardner@google.com Skwasm single threaded (flutter/engine#56206) 2024-10-31 jonahwilliams@google.com [Impeller] expose reference to tessellator instead of shared_ptr. (flutter/engine#56244) 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 chinmaygarde@google.com,zra@google.com 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
…tly into host buffer. (flutter/engine#56173) Using primitive restart we can avoid tracking even odd or inserting degenerate triangles. Instead a special index value `0xFFFF` is used to signal a break. This can be combined with triangle fan on vulkan for a dramatically simpler tessellation. Additionally, switches to a two pass system where we first estimate the storage required by the path so tha the host buffer can be written to directly.
2 participants
Using primitive restart we can avoid tracking even odd or inserting degenerate triangles. Instead a special index value
0xFFFFis used to signal a break. This can be combined with triangle fan on vulkan for a dramatically simpler tessellation.Additionally, switches to a two pass system where we first estimate the storage required by the path so tha the host buffer can be written to directly.