Bug 1452620 - Use FfiVec to clean up WrPipelineInfo usage in C++. r?nical,Gankro
Instead of passing a WrPipelineInfo* to C++ code, we can now pass the
WrPipelineInfo directly. C++ code can access the info members without
having to call back into rust code, so the wr_pipeline_info* iterator
methods can be removed. Deleting the structure still requires passing it
back to rust.
MozReview-Commit-ID: HYcPX3mCGKW
--- a/gfx/layers/apz/public/APZUpdater.h
+++ b/gfx/layers/apz/public/APZUpdater.h
@@ -52,17 +52,17 @@ public:
* This function is invoked from rust on the scene builder thread when it
* is created. It effectively tells the APZUpdater "the current thread is
* the updater thread for this window id" and allows APZUpdater to remember
* which thread it is.
*/
static void SetUpdaterThread(const wr::WrWindowId& aWindowId);
static void PrepareForSceneSwap(const wr::WrWindowId& aWindowId);
static void CompleteSceneSwap(const wr::WrWindowId& aWindowId,
- wr::WrPipelineInfo* aInfo);
+ const wr::WrPipelineInfo& aInfo);
static void ProcessPendingTasks(const wr::WrWindowId& aWindowId);
void ClearTree(LayersId aRootLayersId);
void UpdateFocusState(LayersId aRootLayerTreeId,
LayersId aOriginatingLayersId,
const FocusTarget& aFocusTarget);
void UpdateHitTestingTree(LayersId aRootLayerTreeId,
Layer* aRoot,
--- a/gfx/layers/apz/src/APZUpdater.cpp
+++ b/gfx/layers/apz/src/APZUpdater.cpp
@@ -74,43 +74,40 @@ APZUpdater::PrepareForSceneSwap(const wr
{
if (RefPtr<APZUpdater> updater = GetUpdater(aWindowId)) {
updater->mApz->LockTree();
}
}
/*static*/ void
APZUpdater::CompleteSceneSwap(const wr::WrWindowId& aWindowId,
- wr::WrPipelineInfo* aInfo)
+ const wr::WrPipelineInfo& aInfo)
{
RefPtr<APZUpdater> updater = GetUpdater(aWindowId);
if (!updater) {
// This should only happen in cases where PrepareForSceneSwap also got a
// null updater. No updater-thread tasks get run between PrepareForSceneSwap
// and this function, so there is no opportunity for the updater mapping
// to have gotten removed from sWindowIdMap in between the two calls.
return;
}
- wr::WrPipelineId pipeline;
- wr::WrEpoch epoch;
- while (wr_pipeline_info_next_removed_pipeline(aInfo, &pipeline)) {
- LayersId layersId = wr::AsLayersId(pipeline);
+ for (uintptr_t i = 0; i < aInfo.removed_pipelines.length; i++) {
+ LayersId layersId = wr::AsLayersId(aInfo.removed_pipelines.data[i]);
updater->mEpochData.erase(layersId);
}
// Reset the built info for all pipelines, then put it back for the ones
// that got built in this scene swap.
for (auto& i : updater->mEpochData) {
i.second.mBuilt = Nothing();
}
- while (wr_pipeline_info_next_epoch(aInfo, &pipeline, &epoch)) {
- LayersId layersId = wr::AsLayersId(pipeline);
- updater->mEpochData[layersId].mBuilt = Some(epoch);
+ for (uintptr_t i = 0; i < aInfo.epochs.length; i++) {
+ LayersId layersId = wr::AsLayersId(aInfo.epochs.data[i].pipeline_id);
+ updater->mEpochData[layersId].mBuilt = Some(aInfo.epochs.data[i].epoch);
}
- wr_pipeline_info_delete(aInfo);
// Run any tasks that got unblocked, then unlock the tree. The order is
// important because we want to run all the tasks up to and including the
// UpdateHitTestingTree calls corresponding to the built epochs, and we
// want to run those before we release the lock (i.e. atomically with the
// scene swap). This ensures that any hit-tests always encounter a consistent
// state between the APZ tree and the built scene in WR.
//
@@ -520,21 +517,22 @@ apz_pre_scene_swap(mozilla::wr::WrWindow
{
// This should never get called unless async scene building is enabled.
MOZ_ASSERT(gfxPrefs::WebRenderAsyncSceneBuild());
mozilla::layers::APZUpdater::PrepareForSceneSwap(aWindowId);
}
void
apz_post_scene_swap(mozilla::wr::WrWindowId aWindowId,
- mozilla::wr::WrPipelineInfo* aInfo)
+ mozilla::wr::WrPipelineInfo aInfo)
{
// This should never get called unless async scene building is enabled.
MOZ_ASSERT(gfxPrefs::WebRenderAsyncSceneBuild());
mozilla::layers::APZUpdater::CompleteSceneSwap(aWindowId, aInfo);
+ wr_pipeline_info_delete(aInfo);
}
void
apz_run_updater(mozilla::wr::WrWindowId aWindowId)
{
// This should never get called unless async scene building is enabled.
MOZ_ASSERT(gfxPrefs::WebRenderAsyncSceneBuild());
mozilla::layers::APZUpdater::ProcessPendingTasks(aWindowId);
--- a/gfx/webrender_bindings/RenderThread.cpp
+++ b/gfx/webrender_bindings/RenderThread.cpp
@@ -227,27 +227,29 @@ RenderThread::RunEvent(wr::WindowId aWin
}
aEvent->Run(*this, aWindowId);
aEvent = nullptr;
}
static void
NotifyDidRender(layers::CompositorBridgeParentBase* aBridge,
- wr::WrPipelineInfo* aInfo,
+ wr::WrPipelineInfo aInfo,
TimeStamp aStart,
TimeStamp aEnd)
{
- wr::WrPipelineId pipeline;
- wr::WrEpoch epoch;
- while (wr_pipeline_info_next_epoch(aInfo, &pipeline, &epoch)) {
- aBridge->NotifyDidCompositeToPipeline(pipeline, epoch, aStart, aEnd);
+ for (uintptr_t i = 0; i < aInfo.epochs.length; i++) {
+ aBridge->NotifyDidCompositeToPipeline(
+ aInfo.epochs.data[i].pipeline_id,
+ aInfo.epochs.data[i].epoch,
+ aStart,
+ aEnd);
}
- while (wr_pipeline_info_next_removed_pipeline(aInfo, &pipeline)) {
- aBridge->NotifyPipelineRemoved(pipeline);
+ for (uintptr_t i = 0; i < aInfo.removed_pipelines.length; i++) {
+ aBridge->NotifyPipelineRemoved(aInfo.removed_pipelines.data[i]);
}
wr_pipeline_info_delete(aInfo);
}
void
RenderThread::UpdateAndRender(wr::WindowId aWindowId, bool aReadback)
{
--- a/gfx/webrender_bindings/RendererOGL.cpp
+++ b/gfx/webrender_bindings/RendererOGL.cpp
@@ -186,17 +186,17 @@ RendererOGL::SetFrameStartTime(const Tim
if (mFrameStartTime) {
// frame start time is already set. This could happen when multiple
// generate frame requests are merged by webrender.
return;
}
mFrameStartTime = aTime;
}
-wr::WrPipelineInfo*
+wr::WrPipelineInfo
RendererOGL::FlushPipelineInfo()
{
return wr_renderer_flush_pipeline_info(mRenderer);
}
RenderTextureHost*
RendererOGL::GetRenderTexture(wr::WrExternalImageId aExternalImageId)
{
--- a/gfx/webrender_bindings/RendererOGL.h
+++ b/gfx/webrender_bindings/RendererOGL.h
@@ -79,17 +79,17 @@ public:
/// This can be called on the render thread only.
bool Resume();
layers::SyncObjectHost* GetSyncObject() const;
layers::CompositorBridgeParentBase* GetCompositorBridge() { return mBridge; }
- wr::WrPipelineInfo* FlushPipelineInfo();
+ wr::WrPipelineInfo FlushPipelineInfo();
RenderTextureHost* GetRenderTexture(wr::WrExternalImageId aExternalImageId);
wr::Renderer* GetRenderer() { return mRenderer; }
gl::GLContext* gl() const;
protected:
--- a/gfx/webrender_bindings/src/bindings.rs
+++ b/gfx/webrender_bindings/src/bindings.rs
@@ -625,75 +625,67 @@ pub extern "C" fn wr_renderer_current_ep
/// cbindgen:postfix=WR_DESTRUCTOR_SAFE_FUNC
#[no_mangle]
pub unsafe extern "C" fn wr_renderer_delete(renderer: *mut Renderer) {
let renderer = Box::from_raw(renderer);
renderer.deinit();
// let renderer go out of scope and get dropped
}
+// cbindgen doesn't support tuples, so we have a little struct instead, with
+// an Into implementation to convert from the tuple to the struct.
+#[repr(C)]
+pub struct WrPipelineEpoch {
+ pipeline_id: WrPipelineId,
+ epoch: WrEpoch,
+}
+
+impl From<(WrPipelineId, WrEpoch)> for WrPipelineEpoch {
+ fn from(tuple: (WrPipelineId, WrEpoch)) -> WrPipelineEpoch {
+ WrPipelineEpoch {
+ pipeline_id: tuple.0,
+ epoch: tuple.1
+ }
+ }
+}
+
+#[repr(C)]
pub struct WrPipelineInfo {
- epochs: Vec<(WrPipelineId, WrEpoch)>,
- removed_pipelines: Vec<PipelineId>,
+ epochs: FfiVec<WrPipelineEpoch>,
+ removed_pipelines: FfiVec<PipelineId>,
}
impl WrPipelineInfo {
fn new(info: PipelineInfo) -> Self {
WrPipelineInfo {
- epochs: info.epochs.into_iter().collect(),
- removed_pipelines: info.removed_pipelines,
+ epochs: FfiVec::from_vec(info.epochs.into_iter().map(WrPipelineEpoch::from).collect()),
+ removed_pipelines: FfiVec::from_vec(info.removed_pipelines),
}
}
}
#[no_mangle]
-pub unsafe extern "C" fn wr_renderer_flush_pipeline_info(renderer: &mut Renderer) -> *mut WrPipelineInfo {
+pub unsafe extern "C" fn wr_renderer_flush_pipeline_info(renderer: &mut Renderer) -> WrPipelineInfo {
let info = renderer.flush_pipeline_info();
- Box::into_raw(Box::new(WrPipelineInfo::new(info)))
-}
-
-#[no_mangle]
-pub unsafe extern "C" fn wr_pipeline_info_next_epoch(
- info: &mut WrPipelineInfo,
- out_pipeline: &mut WrPipelineId,
- out_epoch: &mut WrEpoch
-) -> bool {
- if let Some((pipeline, epoch)) = info.epochs.pop() {
- *out_pipeline = pipeline;
- *out_epoch = epoch;
- return true;
- }
- return false;
-}
-
-#[no_mangle]
-pub unsafe extern "C" fn wr_pipeline_info_next_removed_pipeline(
- info: &mut WrPipelineInfo,
- out_pipeline: &mut WrPipelineId,
-) -> bool {
- if let Some(pipeline) = info.removed_pipelines.pop() {
- *out_pipeline = pipeline;
- return true;
- }
- return false;
+ WrPipelineInfo::new(info)
}
/// cbindgen:postfix=WR_DESTRUCTOR_SAFE_FUNC
#[no_mangle]
-pub unsafe extern "C" fn wr_pipeline_info_delete(info: *mut WrPipelineInfo) {
- Box::from_raw(info);
+pub unsafe extern "C" fn wr_pipeline_info_delete(_info: WrPipelineInfo) {
+ // _info will be dropped here, and the drop impl on FfiVec will free
+ // the underlying vec memory
}
-#[allow(improper_ctypes)] // this is needed so that rustc doesn't complain about passing the *mut WrPipelineInfo to an extern function
extern "C" {
fn apz_register_updater(window_id: WrWindowId);
fn apz_pre_scene_swap(window_id: WrWindowId);
// This function takes ownership of the pipeline_info and is responsible for
// freeing it via wr_pipeline_info_delete.
- fn apz_post_scene_swap(window_id: WrWindowId, pipeline_info: *mut WrPipelineInfo);
+ fn apz_post_scene_swap(window_id: WrWindowId, pipeline_info: WrPipelineInfo);
fn apz_run_updater(window_id: WrWindowId);
fn apz_deregister_updater(window_id: WrWindowId);
}
struct APZCallbacks {
window_id: WrWindowId,
}
@@ -710,17 +702,17 @@ impl SceneBuilderHooks for APZCallbacks
unsafe { apz_register_updater(self.window_id) }
}
fn pre_scene_swap(&self) {
unsafe { apz_pre_scene_swap(self.window_id) }
}
fn post_scene_swap(&self, info: PipelineInfo) {
- let info = Box::into_raw(Box::new(WrPipelineInfo::new(info)));
+ let info = WrPipelineInfo::new(info);
unsafe { apz_post_scene_swap(self.window_id, info) }
}
fn poke(&self) {
unsafe { apz_run_updater(self.window_id) }
}
fn deregister(&self) {
--- a/gfx/webrender_bindings/webrender_ffi.h
+++ b/gfx/webrender_bindings/webrender_ffi.h
@@ -88,17 +88,17 @@ struct FontInstanceFlags {
struct WrWindowId;
struct WrPipelineInfo;
} // namespace wr
} // namespace mozilla
void apz_register_updater(mozilla::wr::WrWindowId aWindowId);
void apz_pre_scene_swap(mozilla::wr::WrWindowId aWindowId);
-void apz_post_scene_swap(mozilla::wr::WrWindowId aWindowId, mozilla::wr::WrPipelineInfo* aInfo);
+void apz_post_scene_swap(mozilla::wr::WrWindowId aWindowId, mozilla::wr::WrPipelineInfo aInfo);
void apz_run_updater(mozilla::wr::WrWindowId aWindowId);
void apz_deregister_updater(mozilla::wr::WrWindowId aWindowId);
} // extern "C"
// Some useful defines to stub out webrender binding functions for when we
// build gecko without webrender. We try to tell the compiler these functions
// are unreachable in that case, but VC++ emits a warning if it finds any
--- a/gfx/webrender_bindings/webrender_ffi_generated.h
+++ b/gfx/webrender_bindings/webrender_ffi_generated.h
@@ -269,18 +269,16 @@ struct Transaction;
struct UnknownUnit;
template<typename T>
struct Vec;
// Geometry in the document's coordinate space (logical pixels).
struct WorldPixel;
-struct WrPipelineInfo;
-
struct WrProgramCache;
struct WrState;
struct WrThreadPool;
struct IdNamespace {
uint32_t mHandle;
@@ -324,16 +322,85 @@ struct WrWindowId {
bool operator<(const WrWindowId& aOther) const {
return mHandle < aOther.mHandle;
}
bool operator<=(const WrWindowId& aOther) const {
return mHandle <= aOther.mHandle;
}
};
+// This type carries no valuable semantics for WR. However, it reflects the fact that
+// clients (Servo) may generate pipelines by different semi-independent sources.
+// These pipelines still belong to the same `IdNamespace` and the same `DocumentId`.
+// Having this extra Id field enables them to generate `PipelineId` without collision.
+using PipelineSourceId = uint32_t;
+
+// From the point of view of WR, `PipelineId` is completely opaque and generic as long as
+// it's clonable, serializable, comparable, and hashable.
+struct PipelineId {
+ PipelineSourceId mNamespace;
+ uint32_t mHandle;
+
+ bool operator==(const PipelineId& aOther) const {
+ return mNamespace == aOther.mNamespace &&
+ mHandle == aOther.mHandle;
+ }
+};
+
+using WrPipelineId = PipelineId;
+
+struct Epoch {
+ uint32_t mHandle;
+
+ bool operator==(const Epoch& aOther) const {
+ return mHandle == aOther.mHandle;
+ }
+ bool operator<(const Epoch& aOther) const {
+ return mHandle < aOther.mHandle;
+ }
+ bool operator<=(const Epoch& aOther) const {
+ return mHandle <= aOther.mHandle;
+ }
+};
+
+using WrEpoch = Epoch;
+
+struct WrPipelineEpoch {
+ WrPipelineId pipeline_id;
+ WrEpoch epoch;
+
+ bool operator==(const WrPipelineEpoch& aOther) const {
+ return pipeline_id == aOther.pipeline_id &&
+ epoch == aOther.epoch;
+ }
+};
+
+template<typename T>
+struct FfiVec {
+ const T *data;
+ uintptr_t length;
+ uintptr_t capacity;
+
+ bool operator==(const FfiVec& aOther) const {
+ return data == aOther.data &&
+ length == aOther.length &&
+ capacity == aOther.capacity;
+ }
+};
+
+struct WrPipelineInfo {
+ FfiVec<WrPipelineEpoch> epochs;
+ FfiVec<PipelineId> removed_pipelines;
+
+ bool operator==(const WrPipelineInfo& aOther) const {
+ return epochs == aOther.epochs &&
+ removed_pipelines == aOther.removed_pipelines;
+ }
+};
+
template<typename T, typename U>
struct TypedSize2D {
T width;
T height;
bool operator==(const TypedSize2D& aOther) const {
return width == aOther.width &&
height == aOther.height;
@@ -391,36 +458,16 @@ struct TypedPoint2D {
bool operator==(const TypedPoint2D& aOther) const {
return x == aOther.x &&
y == aOther.y;
}
};
using WorldPoint = TypedPoint2D<float, WorldPixel>;
-// This type carries no valuable semantics for WR. However, it reflects the fact that
-// clients (Servo) may generate pipelines by different semi-independent sources.
-// These pipelines still belong to the same `IdNamespace` and the same `DocumentId`.
-// Having this extra Id field enables them to generate `PipelineId` without collision.
-using PipelineSourceId = uint32_t;
-
-// From the point of view of WR, `PipelineId` is completely opaque and generic as long as
-// it's clonable, serializable, comparable, and hashable.
-struct PipelineId {
- PipelineSourceId mNamespace;
- uint32_t mHandle;
-
- bool operator==(const PipelineId& aOther) const {
- return mNamespace == aOther.mNamespace &&
- mHandle == aOther.mHandle;
- }
-};
-
-using WrPipelineId = PipelineId;
-
// A 2d Rectangle optionally tagged with a unit.
template<typename T, typename U>
struct TypedRect {
TypedPoint2D<T, U> origin;
TypedSize2D<T, U> size;
bool operator==(const TypedRect& aOther) const {
return origin == aOther.origin &&
@@ -754,32 +801,16 @@ struct MutByteSlice {
uintptr_t len;
bool operator==(const MutByteSlice& aOther) const {
return buffer == aOther.buffer &&
len == aOther.len;
}
};
-struct Epoch {
- uint32_t mHandle;
-
- bool operator==(const Epoch& aOther) const {
- return mHandle == aOther.mHandle;
- }
- bool operator<(const Epoch& aOther) const {
- return mHandle < aOther.mHandle;
- }
- bool operator<=(const Epoch& aOther) const {
- return mHandle <= aOther.mHandle;
- }
-};
-
-using WrEpoch = Epoch;
-
struct WrDebugFlags {
uint32_t mBits;
bool operator==(const WrDebugFlags& aOther) const {
return mBits == aOther.mBits;
}
};
@@ -945,17 +976,17 @@ extern void AddNativeFontHandle(WrFontKe
void *aHandle,
uint32_t aIndex);
extern void DeleteFontData(WrFontKey aKey);
extern void apz_deregister_updater(WrWindowId aWindowId);
extern void apz_post_scene_swap(WrWindowId aWindowId,
- WrPipelineInfo *aPipelineInfo);
+ WrPipelineInfo aPipelineInfo);
extern void apz_pre_scene_swap(WrWindowId aWindowId);
extern void apz_register_updater(WrWindowId aWindowId);
extern void apz_run_updater(WrWindowId aWindowId);
extern void gecko_printf_stderr_output(const char *aMsg);
@@ -1368,31 +1399,20 @@ extern void wr_notifier_external_event(W
extern void wr_notifier_new_frame_ready(WrWindowId aWindowId);
extern void wr_notifier_new_scroll_frame_ready(WrWindowId aWindowId,
bool aCompositeNeeded);
extern void wr_notifier_wake_up(WrWindowId aWindowId);
WR_INLINE
-void wr_pipeline_info_delete(WrPipelineInfo *aInfo)
+void wr_pipeline_info_delete(WrPipelineInfo aInfo)
WR_DESTRUCTOR_SAFE_FUNC;
WR_INLINE
-bool wr_pipeline_info_next_epoch(WrPipelineInfo *aInfo,
- WrPipelineId *aOutPipeline,
- WrEpoch *aOutEpoch)
-WR_FUNC;
-
-WR_INLINE
-bool wr_pipeline_info_next_removed_pipeline(WrPipelineInfo *aInfo,
- WrPipelineId *aOutPipeline)
-WR_FUNC;
-
-WR_INLINE
void wr_program_cache_delete(WrProgramCache *aProgramCache)
WR_DESTRUCTOR_SAFE_FUNC;
WR_INLINE
WrProgramCache *wr_program_cache_new()
WR_FUNC;
WR_INLINE
@@ -1401,17 +1421,17 @@ bool wr_renderer_current_epoch(Renderer
WrEpoch *aOutEpoch)
WR_FUNC;
WR_INLINE
void wr_renderer_delete(Renderer *aRenderer)
WR_DESTRUCTOR_SAFE_FUNC;
WR_INLINE
-WrPipelineInfo *wr_renderer_flush_pipeline_info(Renderer *aRenderer)
+WrPipelineInfo wr_renderer_flush_pipeline_info(Renderer *aRenderer)
WR_FUNC;
WR_INLINE
WrDebugFlags wr_renderer_get_debug_flags(Renderer *aRenderer)
WR_FUNC;
WR_INLINE
void wr_renderer_readback(Renderer *aRenderer,