Skip to content

Commit e263222

Browse files
author
Jonah Williams
authored
[Impeller] reland: switch Pipeline to use raw ptr instead of shared ptr for recorded references. (flutter/engine#57086)
Prev: flutter/engine#57015 There is a unit test that is clearing out the pipeline storage, manually null out captured PipelineRef.
1 parent d8cdd81 commit e263222

File tree

21 files changed

+235
-167
lines changed

21 files changed

+235
-167
lines changed

engine/src/flutter/ci/licenses_golden/licenses_flutter

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42962,6 +42962,8 @@ ORIGIN: ../../../flutter/impeller/core/platform.cc + ../../../flutter/LICENSE
4296242962
ORIGIN: ../../../flutter/impeller/core/platform.h + ../../../flutter/LICENSE
4296342963
ORIGIN: ../../../flutter/impeller/core/range.cc + ../../../flutter/LICENSE
4296442964
ORIGIN: ../../../flutter/impeller/core/range.h + ../../../flutter/LICENSE
42965+
ORIGIN: ../../../flutter/impeller/core/raw_ptr.cc + ../../../flutter/LICENSE
42966+
ORIGIN: ../../../flutter/impeller/core/raw_ptr.h + ../../../flutter/LICENSE
4296542967
ORIGIN: ../../../flutter/impeller/core/resource_binder.cc + ../../../flutter/LICENSE
4296642968
ORIGIN: ../../../flutter/impeller/core/resource_binder.h + ../../../flutter/LICENSE
4296742969
ORIGIN: ../../../flutter/impeller/core/runtime_types.cc + ../../../flutter/LICENSE
@@ -45891,6 +45893,8 @@ FILE: ../../../flutter/impeller/core/platform.cc
4589145893
FILE: ../../../flutter/impeller/core/platform.h
4589245894
FILE: ../../../flutter/impeller/core/range.cc
4589345895
FILE: ../../../flutter/impeller/core/range.h
45896+
FILE: ../../../flutter/impeller/core/raw_ptr.cc
45897+
FILE: ../../../flutter/impeller/core/raw_ptr.h
4589445898
FILE: ../../../flutter/impeller/core/resource_binder.cc
4589545899
FILE: ../../../flutter/impeller/core/resource_binder.h
4589645900
FILE: ../../../flutter/impeller/core/runtime_types.cc

engine/src/flutter/impeller/core/BUILD.gn

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ impeller_component("core") {
2323
"platform.h",
2424
"range.cc",
2525
"range.h",
26+
"raw_ptr.cc",
27+
"raw_ptr.h",
2628
"resource_binder.cc",
2729
"resource_binder.h",
2830
"runtime_types.cc",
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// Copyright 2013 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
#include "impeller/core/raw_ptr.h"
6+
7+
namespace impeller {
8+
9+
//
10+
11+
}
Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
// Copyright 2013 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
#ifndef FLUTTER_IMPELLER_CORE_RAW_PTR_H_
6+
#define FLUTTER_IMPELLER_CORE_RAW_PTR_H_
7+
8+
#include <memory>
9+
10+
namespace impeller {
11+
12+
/// @brief A wrapper around a raw ptr that adds additional unopt mode only
13+
/// checks.
14+
template <typename T>
15+
class raw_ptr {
16+
public:
17+
explicit raw_ptr(const std::shared_ptr<T>& ptr)
18+
: ptr_(ptr.get())
19+
#if !NDEBUG
20+
,
21+
weak_ptr_(ptr)
22+
#endif
23+
{
24+
}
25+
26+
raw_ptr() : ptr_(nullptr) {}
27+
28+
T* operator->() {
29+
#if !NDEBUG
30+
FML_CHECK(weak_ptr_.lock());
31+
#endif
32+
return ptr_;
33+
}
34+
35+
const T* operator->() const {
36+
#if !NDEBUG
37+
FML_CHECK(weak_ptr_.lock());
38+
#endif
39+
return ptr_;
40+
}
41+
42+
T* get() {
43+
#if !NDEBUG
44+
FML_CHECK(weak_ptr_.lock());
45+
#endif
46+
return ptr_;
47+
}
48+
49+
T& operator*() {
50+
#if !NDEBUG
51+
FML_CHECK(weak_ptr_.lock());
52+
#endif
53+
return *ptr_;
54+
}
55+
56+
const T& operator*() const {
57+
#if !NDEBUG
58+
FML_CHECK(weak_ptr_.lock());
59+
#endif
60+
return *ptr_;
61+
}
62+
63+
template <class U>
64+
inline bool operator==(raw_ptr<U> const& other) const {
65+
return ptr_ == other.ptr_;
66+
}
67+
68+
template <class U>
69+
inline bool operator!=(raw_ptr<U> const& other) const {
70+
return !(*this == other);
71+
}
72+
73+
explicit operator bool() const { return !!ptr_; }
74+
75+
private:
76+
T* ptr_;
77+
#if !NDEBUG
78+
std::weak_ptr<T> weak_ptr_;
79+
#endif
80+
};
81+
82+
} // namespace impeller
83+
84+
#endif // FLUTTER_IMPELLER_CORE_RAW_PTR_H_

engine/src/flutter/impeller/entity/contents/color_source_contents.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,8 +104,7 @@ class ColorSourceContents : public Contents {
104104
using PipelineBuilderMethod = std::shared_ptr<Pipeline<PipelineDescriptor>> (
105105
impeller::ContentContext::*)(ContentContextOptions) const;
106106
using PipelineBuilderCallback =
107-
std::function<std::shared_ptr<Pipeline<PipelineDescriptor>>(
108-
ContentContextOptions)>;
107+
std::function<PipelineRef(ContentContextOptions)>;
109108
using CreateGeometryCallback =
110109
std::function<GeometryResult(const ContentContext& renderer,
111110
const Entity& entity,

engine/src/flutter/impeller/entity/contents/content_context.cc

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -569,8 +569,7 @@ void ContentContext::SetWireframe(bool wireframe) {
569569
wireframe_ = wireframe;
570570
}
571571

572-
std::shared_ptr<Pipeline<PipelineDescriptor>>
573-
ContentContext::GetCachedRuntimeEffectPipeline(
572+
PipelineRef ContentContext::GetCachedRuntimeEffectPipeline(
574573
const std::string& unique_entrypoint_name,
575574
const ContentContextOptions& options,
576575
const std::function<std::shared_ptr<Pipeline<PipelineDescriptor>>()>&
@@ -580,7 +579,7 @@ ContentContext::GetCachedRuntimeEffectPipeline(
580579
if (it == runtime_effect_pipelines_.end()) {
581580
it = runtime_effect_pipelines_.insert(it, {key, create_callback()});
582581
}
583-
return it->second;
582+
return raw_ptr(it->second);
584583
}
585584

586585
void ContentContext::ClearCachedRuntimeEffectPipeline(

0 commit comments

Comments
 (0)