Skip to content

Commit f5f6870

Browse files
author
Sandor Molnar
committed
Revert "Bug 1998657 [Wayland] Create EGLWindow over offscreen wl_surface and make it always available r=emilio" for causing valgrind failures
This reverts commit de6ac36. Revert "Bug 1998657 [Wayland] Make WaylandSurface ready to draw right after its created r=emilio" This reverts commit b39b0ef.
1 parent 83b903c commit f5f6870

File tree

8 files changed

+410
-70
lines changed

8 files changed

+410
-70
lines changed

widget/gtk/MozContainerWayland.cpp

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,11 @@ static void moz_container_wayland_invalidate(MozContainer* container) {
9999
gdk_window_invalidate_rect(window, nullptr, true);
100100
}
101101

102+
void moz_container_wayland_add_or_fire_initial_draw_callback(
103+
MozContainer* container, const std::function<void(void)>& initial_draw_cb) {
104+
MOZ_WL_SURFACE(container)->AddOrFireReadyToDrawCallback(initial_draw_cb);
105+
}
106+
102107
void moz_container_wayland_unmap(GtkWidget* widget) {
103108
g_return_if_fail(IS_MOZ_CONTAINER(widget));
104109

@@ -138,9 +143,23 @@ gboolean moz_container_wayland_map_event(GtkWidget* widget,
138143
// below.
139144
MOZ_DIAGNOSTIC_ASSERT(NS_IsMainThread());
140145

146+
// Set waiting_to_show flag. It means the mozcontainer is cofigured/mapped
147+
// and it's supposed to be visible. *But* it's really visible when we get
148+
// moz_container_wayland_add_or_fire_initial_draw_callback() which means
149+
// wayland compositor makes it live.
150+
MOZ_WL_CONTAINER(widget)->waiting_to_show = true;
151+
MozContainer* container = MOZ_CONTAINER(widget);
152+
MOZ_WL_SURFACE(container)->AddOrFireReadyToDrawCallback(
153+
[container]() -> void {
154+
LOGCONTAINER(
155+
"[%p] moz_container_wayland_add_or_fire_initial_draw_callback set "
156+
"visible",
157+
moz_container_get_nsWindow(container));
158+
moz_container_wayland_clear_waiting_to_show_flag(container);
159+
});
160+
141161
// Don't create wl_subsurface in map_event when it's already created or
142162
// if we create it for the first time.
143-
MozContainer* container = MOZ_CONTAINER(widget);
144163
if (MOZ_WL_SURFACE(container)->IsMapped() ||
145164
MOZ_WL_CONTAINER(container)->before_first_size_alloc) {
146165
return false;
@@ -262,7 +281,40 @@ static bool moz_container_wayland_ensure_surface(MozContainer* container,
262281
return true;
263282
}
264283

284+
struct wl_egl_window* moz_container_wayland_get_egl_window(
285+
MozContainer* container) {
286+
LOGCONTAINER("%s [%p] mapped %d eglwindow %d", __FUNCTION__,
287+
(void*)moz_container_get_nsWindow(container),
288+
MOZ_WL_SURFACE(container)->IsMapped(),
289+
MOZ_WL_SURFACE(container)->HasEGLWindow());
290+
291+
if (!MOZ_WL_SURFACE(container)->IsMapped()) {
292+
return nullptr;
293+
}
294+
295+
// TODO: Get size from bounds instead of GdkWindow?
296+
// We may be in rendering/compositor thread here.
297+
GdkWindow* window = gtk_widget_get_window(GTK_WIDGET(container));
298+
DesktopIntSize size(gdk_window_get_width(window),
299+
gdk_window_get_height(window));
300+
return MOZ_WL_SURFACE(container)->GetEGLWindow(size);
301+
}
302+
303+
gboolean moz_container_wayland_can_draw(MozContainer* container) {
304+
return MOZ_WL_SURFACE(container)->IsReadyToDraw();
305+
}
306+
265307
double moz_container_wayland_get_scale(MozContainer* container) {
266308
nsWindow* window = moz_container_get_nsWindow(container);
267309
return window ? window->FractionalScaleFactor() : 1.0;
268310
}
311+
312+
bool moz_container_wayland_is_waiting_to_show(MozContainer* container) {
313+
MOZ_DIAGNOSTIC_ASSERT(NS_IsMainThread());
314+
return MOZ_WL_CONTAINER(container)->waiting_to_show;
315+
}
316+
317+
void moz_container_wayland_clear_waiting_to_show_flag(MozContainer* container) {
318+
MOZ_DIAGNOSTIC_ASSERT(NS_IsMainThread());
319+
MOZ_WL_CONTAINER(container)->waiting_to_show = false;
320+
}

widget/gtk/MozContainerWayland.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,13 +43,24 @@ struct MozContainerWayland {
4343

4444
RefPtr<mozilla::widget::WaylandSurface> mSurface;
4545
gboolean before_first_size_alloc = false;
46+
gboolean waiting_to_show = false;
4647
};
4748

4849
void moz_container_wayland_map(GtkWidget*);
4950
gboolean moz_container_wayland_map_event(GtkWidget*, GdkEventAny*);
5051
void moz_container_wayland_size_allocate(GtkWidget*, GtkAllocation*);
5152
void moz_container_wayland_unmap(GtkWidget*);
53+
54+
struct wl_egl_window* moz_container_wayland_get_egl_window(
55+
MozContainer* container);
56+
57+
void moz_container_wayland_add_or_fire_initial_draw_callback(
58+
MozContainer* container, const std::function<void(void)>& initial_draw_cb);
59+
5260
wl_surface* moz_gtk_widget_get_wl_surface(GtkWidget* aWidget);
61+
gboolean moz_container_wayland_can_draw(MozContainer* container);
5362
double moz_container_wayland_get_scale(MozContainer* container);
63+
bool moz_container_wayland_is_waiting_to_show(MozContainer* container);
64+
void moz_container_wayland_clear_waiting_to_show_flag(MozContainer* container);
5465

5566
#endif /* __MOZ_CONTAINER_WAYLAND_H__ */

widget/gtk/WaylandSurface.cpp

Lines changed: 115 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,104 @@ WaylandSurface::~WaylandSurface() {
117117
"We can't release WaylandSurface with numap callback!");
118118
}
119119

120+
void WaylandSurface::ReadyToDrawFrameCallbackHandler(
121+
struct wl_callback* callback) {
122+
LOGWAYLAND(
123+
"WaylandSurface::ReadyToDrawFrameCallbackHandler() "
124+
"mReadyToDrawFrameCallback %p mIsReadyToDraw %d initial_draw callback "
125+
"%zd\n",
126+
(void*)mReadyToDrawFrameCallback, (bool)mIsReadyToDraw,
127+
mReadyToDrawCallbacks.size());
128+
129+
// We're supposed to run on main thread only.
130+
AssertIsOnMainThread();
131+
132+
// mReadyToDrawFrameCallback/callback can be nullptr when redering directly
133+
// to GtkWidget and ReadyToDrawFrameCallbackHandler is called by us from main
134+
// thread by WaylandSurface::Map().
135+
MOZ_RELEASE_ASSERT(mReadyToDrawFrameCallback == callback);
136+
137+
std::vector<std::function<void(void)>> cbs;
138+
{
139+
WaylandSurfaceLock lock(this);
140+
MozClearPointer(mReadyToDrawFrameCallback, wl_callback_destroy);
141+
// It's possible that we're already unmapped so quit in such case.
142+
if (!mIsMapped) {
143+
LOGWAYLAND(" WaylandSurface is unmapped, quit.");
144+
if (!mReadyToDrawCallbacks.empty()) {
145+
NS_WARNING("Unmapping WaylandSurface with active draw callback!");
146+
mReadyToDrawCallbacks.clear();
147+
}
148+
return;
149+
}
150+
if (mIsReadyToDraw) {
151+
return;
152+
}
153+
mIsReadyToDraw = true;
154+
cbs = std::move(mReadyToDrawCallbacks);
155+
156+
RequestFrameCallbackLocked(lock);
157+
}
158+
159+
// We can't call the callbacks under lock
160+
#ifdef MOZ_LOGGING
161+
int callbackNum = 0;
162+
#endif
163+
for (auto const& cb : cbs) {
164+
LOGWAYLAND(" initial callback fire [%d]", callbackNum++);
165+
cb();
166+
}
167+
}
168+
169+
static void ReadyToDrawFrameCallbackHandler(void* aWaylandSurface,
170+
struct wl_callback* callback,
171+
uint32_t time) {
172+
auto* waylandSurface = static_cast<WaylandSurface*>(aWaylandSurface);
173+
waylandSurface->ReadyToDrawFrameCallbackHandler(callback);
174+
}
175+
176+
static const struct wl_callback_listener
177+
sWaylandSurfaceReadyToDrawFrameListener = {
178+
::ReadyToDrawFrameCallbackHandler};
179+
180+
void WaylandSurface::AddReadyToDrawCallbackLocked(
181+
const WaylandSurfaceLock& aProofOfLock,
182+
const std::function<void(void)>& aDrawCB) {
183+
LOGVERBOSE("WaylandSurface::AddReadyToDrawCallbackLocked()");
184+
MOZ_DIAGNOSTIC_ASSERT(&aProofOfLock == mSurfaceLock);
185+
mReadyToDrawCallbacks.push_back(aDrawCB);
186+
}
187+
188+
void WaylandSurface::AddOrFireReadyToDrawCallback(
189+
const std::function<void(void)>& aDrawCB) {
190+
{
191+
WaylandSurfaceLock lock(this);
192+
if (!mIsReadyToDraw) {
193+
LOGVERBOSE(
194+
"WaylandSurface::AddOrFireReadyToDrawCallback() callback stored");
195+
mReadyToDrawCallbacks.push_back(aDrawCB);
196+
return;
197+
}
198+
}
199+
200+
LOGWAYLAND("WaylandSurface::AddOrFireReadyToDrawCallback() callback fire");
201+
202+
// We're ready to draw and we have a surface to draw into.
203+
aDrawCB();
204+
}
205+
206+
void WaylandSurface::ClearReadyToDrawCallbacksLocked(
207+
const WaylandSurfaceLock& aProofOfLock) {
208+
MOZ_DIAGNOSTIC_ASSERT(&aProofOfLock == mSurfaceLock);
209+
MozClearPointer(mReadyToDrawFrameCallback, wl_callback_destroy);
210+
mReadyToDrawCallbacks.clear();
211+
}
212+
213+
void WaylandSurface::ClearReadyToDrawCallbacks() {
214+
WaylandSurfaceLock lock(this);
215+
ClearReadyToDrawCallbacksLocked(lock);
216+
}
217+
120218
bool WaylandSurface::HasEmulatedFrameCallbackLocked(
121219
const WaylandSurfaceLock& aProofOfLock) const {
122220
return mFrameCallbackHandler.IsSet() && mFrameCallbackHandler.mEmulated;
@@ -135,7 +233,7 @@ void WaylandSurface::FrameCallbackHandler(struct wl_callback* aCallback,
135233
WaylandSurfaceLock lock(this);
136234

137235
// Don't run emulated callbacks on hidden surfaces
138-
if ((emulatedCallback || aRoutedFromChildSurface) && !mIsVisible) {
236+
if ((emulatedCallback || aRoutedFromChildSurface) && !mIsReadyToDraw) {
139237
return;
140238
}
141239

@@ -161,7 +259,6 @@ void WaylandSurface::FrameCallbackHandler(struct wl_callback* aCallback,
161259
// We're getting regular frame callback from this surface so we must
162260
// have buffer attached.
163261
if (!emulatedCallback && !aRoutedFromChildSurface) {
164-
mIsVisible = true;
165262
mBufferAttached = true;
166263
}
167264

@@ -381,7 +478,8 @@ bool WaylandSurface::MapLocked(const WaylandSurfaceLock& aProofOfLock,
381478
wl_surface* aParentWLSurface,
382479
WaylandSurfaceLock* aParentWaylandSurfaceLock,
383480
gfx::IntPoint aSubsurfacePosition,
384-
bool aSubsurfaceDesync) {
481+
bool aSubsurfaceDesync,
482+
bool aUseReadyToDrawCallback) {
385483
LOGWAYLAND("WaylandSurface::MapLocked()");
386484
MOZ_DIAGNOSTIC_ASSERT(&aProofOfLock == mSurfaceLock);
387485
MOZ_DIAGNOSTIC_ASSERT(!mIsMapped, "Already mapped?");
@@ -421,6 +519,14 @@ bool WaylandSurface::MapLocked(const WaylandSurfaceLock& aProofOfLock,
421519
LOGWAYLAND(" subsurface position [%d,%d]", (int)mSubsurfacePosition.x,
422520
(int)mSubsurfacePosition.y);
423521

522+
if (aUseReadyToDrawCallback) {
523+
mReadyToDrawFrameCallback = wl_surface_frame(mParentSurface);
524+
wl_callback_add_listener(mReadyToDrawFrameCallback,
525+
&sWaylandSurfaceReadyToDrawFrameListener, this);
526+
LOGWAYLAND(" created ready to draw frame callback ID %d\n",
527+
wl_proxy_get_id((struct wl_proxy*)mReadyToDrawFrameCallback));
528+
}
529+
424530
LOGWAYLAND(" register frame callback");
425531
RequestFrameCallbackLocked(aProofOfLock);
426532

@@ -450,7 +556,8 @@ bool WaylandSurface::MapLocked(const WaylandSurfaceLock& aProofOfLock,
450556
gfx::IntPoint aSubsurfacePosition) {
451557
return MapLocked(aProofOfLock, nullptr, aParentWaylandSurfaceLock,
452558
aSubsurfacePosition,
453-
/* aSubsurfaceDesync */ false);
559+
/* aSubsurfaceDesync */ false,
560+
/* aUseReadyToDrawCallback */ false);
454561
}
455562

456563
void WaylandSurface::SetUnmapCallbackLocked(
@@ -502,11 +609,11 @@ void WaylandSurface::UnmapLocked(WaylandSurfaceLock& aSurfaceLock) {
502609
return;
503610
}
504611
mIsMapped = false;
505-
mIsVisible = false;
506612

507613
LOGWAYLAND("WaylandSurface::UnmapLocked()");
508614

509615
RemoveAttachedBufferLocked(aSurfaceLock);
616+
ClearReadyToDrawCallbacksLocked(aSurfaceLock);
510617
ClearFrameCallbackLocked(aSurfaceLock);
511618
ClearScaleLocked(aSurfaceLock);
512619

@@ -526,6 +633,9 @@ void WaylandSurface::UnmapLocked(WaylandSurfaceLock& aSurfaceLock) {
526633
MozClearPointer(mPendingOpaqueRegion, wl_region_destroy);
527634
MozClearPointer(mOpaqueRegionFrameCallback, wl_callback_destroy);
528635

636+
mIsReadyToDraw = false;
637+
mBufferAttached = false;
638+
529639
// Remove references to WaylandBuffers attached to mSurface,
530640
// we don't want to get any buffer release callback when we're unmapped.
531641
ReleaseAllWaylandTransactionsLocked(aSurfaceLock);

widget/gtk/WaylandSurface.h

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,10 @@ class WaylandSurface final {
4343
void SetLoggingWidget(void* aWidget) { mLoggingWidget = aWidget; }
4444
#endif
4545

46+
void ReadyToDrawFrameCallbackHandler(struct wl_callback* aCallback);
47+
void AddOrFireReadyToDrawCallback(const std::function<void(void)>& aDrawCB);
48+
void ClearReadyToDrawCallbacks();
49+
4650
void FrameCallbackHandler(struct wl_callback* aCallback, uint32_t aTime,
4751
bool aRoutedFromChildSurface);
4852

@@ -74,12 +78,11 @@ class WaylandSurface final {
7478
bool SetEGLWindowSize(LayoutDeviceIntSize aSize);
7579
bool HasEGLWindow() const { return !!mEGLWindow; }
7680

81+
// Read to draw means we got frame callback from parent surface
82+
// where we attached to.
83+
bool IsReadyToDraw() const { return mIsReadyToDraw; }
7784
// Mapped means we have all internals created.
7885
bool IsMapped() const { return mIsMapped; }
79-
80-
// We've got first frame callback so we're really visible now.
81-
bool IsVisible() const { return mIsVisible; }
82-
8386
// Indicate that Wayland surface uses Gdk resources which
8487
// need to be released on main thread by GdkCleanUpLocked().
8588
// It may be called after Unmap() to make sure
@@ -119,6 +122,10 @@ class WaylandSurface final {
119122
bool CreateViewportLocked(const WaylandSurfaceLock& aProofOfLock,
120123
bool aFollowsSizeChanges);
121124

125+
void AddReadyToDrawCallbackLocked(
126+
const WaylandSurfaceLock& aProofOfLock,
127+
const std::function<void(void)>& aInitialDrawCB);
128+
122129
// Attach WaylandBuffer which shows WaylandBuffer content
123130
// on screen.
124131
bool AttachLocked(const WaylandSurfaceLock& aSurfaceLock,
@@ -274,7 +281,8 @@ class WaylandSurface final {
274281
bool MapLocked(const WaylandSurfaceLock& aProofOfLock,
275282
wl_surface* aParentWLSurface,
276283
WaylandSurfaceLock* aParentWaylandSurfaceLock,
277-
gfx::IntPoint aSubsurfacePosition, bool aSubsurfaceDesync);
284+
gfx::IntPoint aSubsurfacePosition, bool aSubsurfaceDesync,
285+
bool aUseReadyToDrawCallback = true);
278286

279287
void SetSizeLocked(const WaylandSurfaceLock& aProofOfLock,
280288
gfx::IntSize aSizeScaled, gfx::IntSize aUnscaledSize);
@@ -296,18 +304,21 @@ class WaylandSurface final {
296304
bool HasEmulatedFrameCallbackLocked(
297305
const WaylandSurfaceLock& aProofOfLock) const;
298306

307+
void ClearReadyToDrawCallbacksLocked(const WaylandSurfaceLock& aProofOfLock);
308+
299309
void ClearScaleLocked(const WaylandSurfaceLock& aProofOfLock);
300310

301311
// Weak ref to owning widget (nsWindow or NativeLayerWayland),
302312
// used for diagnostics/logging only.
303313
void* mLoggingWidget = nullptr;
304314

305-
// mIsMapped means we're supposed to be visible
306-
// (or not if Wayland compositor decides so).
315+
// WaylandSurface mapped - we have valid wl_surface where we can paint to.
307316
mozilla::Atomic<bool, mozilla::Relaxed> mIsMapped{false};
308317

309-
// mIsVisible means we're really visible as we've got frame callback.
310-
mozilla::Atomic<bool, mozilla::Relaxed> mIsVisible{false};
318+
// Wayland shows only subsurfaces of visible parent surfaces.
319+
// mIsReadyToDraw means our parent wl_surface has content so
320+
// this WaylandSurface can be visible on screen and get get frame callback.
321+
mozilla::Atomic<bool, mozilla::Relaxed> mIsReadyToDraw{false};
311322

312323
// We used Gdk functions which needs clean up in main thread.
313324
mozilla::Atomic<bool, mozilla::Relaxed> mIsPendingGdkCleanup{false};
@@ -374,6 +385,11 @@ class WaylandSurface final {
374385
bool mBufferTransformFlippedX = false;
375386
bool mBufferTransformFlippedY = false;
376387

388+
// Frame callback registered to parent surface. When we get it we know
389+
// parent surface is ready and we can paint.
390+
wl_callback* mReadyToDrawFrameCallback = nullptr;
391+
std::vector<std::function<void(void)>> mReadyToDrawCallbacks;
392+
377393
// Frame callbacks of this surface
378394
wl_callback* mFrameCallback = nullptr;
379395

0 commit comments

Comments
 (0)