Skip to content

Commit

Permalink
Replace SkSurface::flush methods with GrDirectContext methods (flutte…
Browse files Browse the repository at this point in the history
…r#42425)

In https://skia-review.googlesource.com/c/skia/+/698237, Skia moved the
SkSurface::flush* methods to GrDirectContext (and skgpu::ganesh::Flush).
This updates Flutter to use those versions, which are drop-in
replacements for the previous functionality.

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide] and the [C++,
Objective-C, Java style guides].
- [ ] I listed at least one issue that this PR fixes in the description
above.
- [x] I added new tests to check the change I am making or feature I am
adding, or Hixie said the PR is test-exempt. See [testing the engine]
for instructions on writing and running engine tests.
- [ ] I updated/added relevant documentation (doc comments with `///`).
- [x] I signed the [CLA].
- [x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#overview
[Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
[Flutter Style Guide]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo
[C++, Objective-C, Java style guides]:
https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
[testing the engine]:
https://github.com/flutter/flutter/wiki/Testing-the-engine
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
[Discord]: https://github.com/flutter/flutter/wiki/Chat
  • Loading branch information
kjlubick authored May 31, 2023
1 parent 682bd0f commit ee53c04
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 18 deletions.
1 change: 1 addition & 0 deletions display_list/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ if (enable_unittests) {
deps = [
":display_list",
":display_list_fixtures",
"//flutter/common/graphics",
"//flutter/display_list/testing:display_list_surface_provider",
"//flutter/display_list/testing:display_list_testing",
"//flutter/testing",
Expand Down
44 changes: 28 additions & 16 deletions display_list/benchmarking/dl_benchmarks.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
#include "third_party/skia/include/core/SkPoint.h"
#include "third_party/skia/include/core/SkSurface.h"
#include "third_party/skia/include/core/SkTextBlob.h"
#include "third_party/skia/include/gpu/GrDirectContext.h"
#include "third_party/skia/include/gpu/GrRecordingContext.h"

namespace flutter {
namespace testing {
Expand Down Expand Up @@ -39,6 +41,16 @@ DlPaint GetPaintForRun(unsigned attributes) {
return paint;
}

static void FlushSubmitCpuSync(const sk_sp<SkSurface>& surface) {
if (!surface) {
return;
}
if (GrDirectContext* dContext =
GrAsDirectContext(surface->recordingContext())) {
dContext->flushAndSubmit(surface, /*syncCpu=*/true);
}
}

void AnnotateAttributes(unsigned attributes,
benchmark::State& state,
const DisplayListAttributeFlags flags) {
Expand Down Expand Up @@ -100,7 +112,7 @@ void BM_DrawLine(benchmark::State& state,
// We only want to time the actual rasterization.
for ([[maybe_unused]] auto _ : state) {
canvas.DrawDisplayList(display_list);
surface->flushAndSubmit(true);
FlushSubmitCpuSync(surface);
}

auto filename = surface_provider->backend_name() + "-DrawLine-" +
Expand Down Expand Up @@ -149,7 +161,7 @@ void BM_DrawRect(benchmark::State& state,
// We only want to time the actual rasterization.
for ([[maybe_unused]] auto _ : state) {
canvas.DrawDisplayList(display_list);
surface->flushAndSubmit(true);
FlushSubmitCpuSync(surface);
}

auto filename = surface_provider->backend_name() + "-DrawRect-" +
Expand Down Expand Up @@ -195,7 +207,7 @@ void BM_DrawOval(benchmark::State& state,
// We only want to time the actual rasterization.
for ([[maybe_unused]] auto _ : state) {
canvas.DrawDisplayList(display_list);
surface->flushAndSubmit(true);
FlushSubmitCpuSync(surface);
}

auto filename = surface_provider->backend_name() + "-DrawOval-" +
Expand Down Expand Up @@ -243,7 +255,7 @@ void BM_DrawCircle(benchmark::State& state,
// We only want to time the actual rasterization.
for ([[maybe_unused]] auto _ : state) {
canvas.DrawDisplayList(display_list);
surface->flushAndSubmit(true);
FlushSubmitCpuSync(surface);
}

auto filename = surface_provider->backend_name() + "-DrawCircle-" +
Expand Down Expand Up @@ -321,7 +333,7 @@ void BM_DrawRRect(benchmark::State& state,
// We only want to time the actual rasterization.
for ([[maybe_unused]] auto _ : state) {
canvas.DrawDisplayList(display_list);
surface->flushAndSubmit(true);
FlushSubmitCpuSync(surface);
}

auto filename = surface_provider->backend_name() + "-DrawRRect-" +
Expand Down Expand Up @@ -403,7 +415,7 @@ void BM_DrawDRRect(benchmark::State& state,
// We only want to time the actual rasterization.
for ([[maybe_unused]] auto _ : state) {
canvas.DrawDisplayList(display_list);
surface->flushAndSubmit(true);
FlushSubmitCpuSync(surface);
}

auto filename = surface_provider->backend_name() + "-DrawDRRect-" +
Expand Down Expand Up @@ -456,7 +468,7 @@ void BM_DrawArc(benchmark::State& state,
// We only want to time the actual rasterization.
for ([[maybe_unused]] auto _ : state) {
canvas.DrawDisplayList(display_list);
surface->flushAndSubmit(true);
FlushSubmitCpuSync(surface);
}

auto filename = surface_provider->backend_name() + "-DrawArc-" +
Expand Down Expand Up @@ -659,7 +671,7 @@ void BM_DrawPath(benchmark::State& state,
// We only want to time the actual rasterization.
for ([[maybe_unused]] auto _ : state) {
canvas.DrawDisplayList(display_list);
surface->flushAndSubmit(true);
FlushSubmitCpuSync(surface);
}

auto filename = surface_provider->backend_name() + "-DrawPath-" + label +
Expand Down Expand Up @@ -805,7 +817,7 @@ void BM_DrawVertices(benchmark::State& state,
// We only want to time the actual rasterization.
for ([[maybe_unused]] auto _ : state) {
canvas.DrawDisplayList(display_list);
surface->flushAndSubmit(true);
FlushSubmitCpuSync(surface);
}

auto filename = surface_provider->backend_name() + "-DrawVertices-" +
Expand Down Expand Up @@ -901,7 +913,7 @@ void BM_DrawPoints(benchmark::State& state,

for ([[maybe_unused]] auto _ : state) {
canvas.DrawDisplayList(display_list);
surface->flushAndSubmit(true);
FlushSubmitCpuSync(surface);
}

auto filename = surface_provider->backend_name() + "-DrawPoints-" +
Expand Down Expand Up @@ -979,7 +991,7 @@ void BM_DrawImage(benchmark::State& state,

for ([[maybe_unused]] auto _ : state) {
canvas.DrawDisplayList(display_list);
surface->flushAndSubmit(true);
FlushSubmitCpuSync(surface);
}

auto filename = surface_provider->backend_name() + "-DrawImage-" +
Expand Down Expand Up @@ -1065,7 +1077,7 @@ void BM_DrawImageRect(benchmark::State& state,

for ([[maybe_unused]] auto _ : state) {
canvas.DrawDisplayList(display_list);
surface->flushAndSubmit(true);
FlushSubmitCpuSync(surface);
}

auto filename = surface_provider->backend_name() + "-DrawImageRect-" +
Expand Down Expand Up @@ -1152,7 +1164,7 @@ void BM_DrawImageNine(benchmark::State& state,

for ([[maybe_unused]] auto _ : state) {
canvas.DrawDisplayList(display_list);
surface->flushAndSubmit(true);
FlushSubmitCpuSync(surface);
}

auto filename = surface_provider->backend_name() + "-DrawImageNine-" +
Expand Down Expand Up @@ -1198,7 +1210,7 @@ void BM_DrawTextBlob(benchmark::State& state,

for ([[maybe_unused]] auto _ : state) {
canvas.DrawDisplayList(display_list);
surface->flushAndSubmit(true);
FlushSubmitCpuSync(surface);
}

auto filename = surface_provider->backend_name() + "-DrawTextBlob-" +
Expand Down Expand Up @@ -1263,7 +1275,7 @@ void BM_DrawShadow(benchmark::State& state,
// We only want to time the actual rasterization.
for ([[maybe_unused]] auto _ : state) {
canvas.DrawDisplayList(display_list);
surface->flushAndSubmit(true);
FlushSubmitCpuSync(surface);
}

auto filename = surface_provider->backend_name() + "-DrawShadow-" +
Expand Down Expand Up @@ -1317,7 +1329,7 @@ void BM_SaveLayer(benchmark::State& state,
// We only want to time the actual rasterization.
for ([[maybe_unused]] auto _ : state) {
canvas.DrawDisplayList(display_list);
surface->flushAndSubmit(true);
FlushSubmitCpuSync(surface);
}

auto filename = surface_provider->backend_name() + "-SaveLayer-" +
Expand Down
7 changes: 6 additions & 1 deletion display_list/testing/dl_rendering_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
#include "third_party/skia/include/core/SkSurface.h"
#include "third_party/skia/include/effects/SkGradientShader.h"
#include "third_party/skia/include/effects/SkImageFilters.h"
#include "third_party/skia/include/gpu/GrDirectContext.h"
#include "third_party/skia/include/gpu/GrRecordingContext.h"

namespace flutter {
namespace testing {
Expand Down Expand Up @@ -490,7 +492,10 @@ class RenderEnvironment {
canvas->restoreToCount(restore_count);

canvas->flush();
surface->flushAndSubmit(true);
if (GrDirectContext* dContext =
GrAsDirectContext(surface->recordingContext())) {
dContext->flushAndSubmit(surface, /*syncCpu=*/true);
}
return std::make_unique<RenderResult>(surface);
}

Expand Down
2 changes: 1 addition & 1 deletion vulkan/vulkan_swapchain.cc
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,7 @@ bool VulkanSwapchain::Submit() {
// Step 0:
// Make sure Skia has flushed all work for the surface to the gpu.
// ---------------------------------------------------------------------------
surface->flushAndSubmit();
skgpu::ganesh::FlushAndSubmit(surface);

// ---------------------------------------------------------------------------
// Step 1:
Expand Down

0 comments on commit ee53c04

Please sign in to comment.