Skip to content

Commit

Permalink
REGRESSION(263118@main): [CoordinatedGraphics] Incomplete rendering a…
Browse files Browse the repository at this point in the history
…fter 10s inactivity without hardware acceleration

https://bugs.webkit.org/show_bug.cgi?id=260505

Reviewed by Michael Catanzaro.

When the backing store is discarded in the UI process we need to notify
the web process to reset the dirty region to the entire web view and
ensure next update is not partial. This avoids flickering and ensures a
full update of the view when a new backing store is created. We should
also avoid flickering when forcing an update, by waiting for the Update
message synchronously.

* Source/WebKit/UIProcess/CoordinatedGraphics/DrawingAreaProxyCoordinatedGraphics.cpp:
(WebKit::DrawingAreaProxyCoordinatedGraphics::paint):
(WebKit::DrawingAreaProxyCoordinatedGraphics::forceUpdateIfNeeded):
(WebKit::DrawingAreaProxyCoordinatedGraphics::incorporateUpdate):
(WebKit::DrawingAreaProxyCoordinatedGraphics::enterAcceleratedCompositingMode):
(WebKit::DrawingAreaProxyCoordinatedGraphics::discardBackingStore):
* Source/WebKit/UIProcess/CoordinatedGraphics/DrawingAreaProxyCoordinatedGraphics.h:
* Source/WebKit/UIProcess/DrawingAreaProxy.messages.in:
* Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/DrawingAreaCoordinatedGraphics.cpp:
(WebKit::DrawingAreaCoordinatedGraphics::updateGeometry):
(WebKit::DrawingAreaCoordinatedGraphics::didDiscardBackingStore):
* Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/DrawingAreaCoordinatedGraphics.h:
* Source/WebKit/WebProcess/WebPage/DrawingArea.h:
(WebKit::DrawingArea::didDiscardBackingStore):
* Source/WebKit/WebProcess/WebPage/DrawingArea.messages.in:

Canonical link: https://commits.webkit.org/267399@main
  • Loading branch information
carlosgcampos committed Aug 29, 2023
1 parent bd1a9e7 commit 3e47e6d
Show file tree
Hide file tree
Showing 7 changed files with 72 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -75,17 +75,62 @@ void DrawingAreaProxyCoordinatedGraphics::paint(BackingStore::PlatformGraphicsCo
if (isInAcceleratedCompositingMode())
return;

if (!m_backingStore) {
if (!m_isWaitingForDidUpdateGeometry)
m_webPageProxy.send(Messages::DrawingArea::ForceUpdate(), m_identifier);
if (!m_backingStore && !forceUpdateIfNeeded())
return;
}

m_backingStore->paint(context, rect);
unpaintedRegion.subtract(IntRect(IntPoint(), m_backingStore->size()));

discardBackingStoreSoon();
}

bool DrawingAreaProxyCoordinatedGraphics::forceUpdateIfNeeded()
{
ASSERT(!isInAcceleratedCompositingMode());

if (!m_webPageProxy.hasRunningProcess())
return false;

if (m_webPageProxy.process().state() == WebProcessProxy::State::Launching)
return false;

if (m_isWaitingForDidUpdateGeometry)
return false;

if (!m_webPageProxy.isViewVisible())
return false;

SetForScope inForceUpdate(m_inForceUpdate, true);
m_webPageProxy.send(Messages::DrawingArea::ForceUpdate(), m_identifier);
m_webPageProxy.process().connection()->waitForAndDispatchImmediately<Messages::DrawingAreaProxy::Update>(m_identifier, Seconds::fromMilliseconds(500));
return !!m_backingStore;
}

void DrawingAreaProxyCoordinatedGraphics::incorporateUpdate(UpdateInfo&& updateInfo)
{
ASSERT(!isInAcceleratedCompositingMode());

if (updateInfo.updateRectBounds.isEmpty())
return;

if (!m_backingStore || m_backingStore->size() != updateInfo.viewSize || m_backingStore->deviceScaleFactor() != updateInfo.deviceScaleFactor)
m_backingStore = makeUnique<BackingStore>(updateInfo.viewSize, updateInfo.deviceScaleFactor, m_webPageProxy);

if (m_inForceUpdate) {
m_backingStore->incorporateUpdate(WTFMove(updateInfo));
return;
}

Region damageRegion;
if (updateInfo.scrollRect.isEmpty()) {
for (const auto& rect : updateInfo.updateRects)
damageRegion.unite(rect);
} else
damageRegion = IntRect(IntPoint(), m_webPageProxy.viewSize());

m_backingStore->incorporateUpdate(WTFMove(updateInfo));
m_webPageProxy.setViewNeedsDisplay(damageRegion);
}
#endif

void DrawingAreaProxyCoordinatedGraphics::sizeDidChange()
Expand Down Expand Up @@ -170,30 +215,6 @@ void DrawingAreaProxyCoordinatedGraphics::targetRefreshRateDidChange(unsigned ra
m_webPageProxy.send(Messages::DrawingArea::TargetRefreshRateDidChange(rate), m_identifier);
}

#if !PLATFORM(WPE)
void DrawingAreaProxyCoordinatedGraphics::incorporateUpdate(UpdateInfo&& updateInfo)
{
ASSERT(!isInAcceleratedCompositingMode());

if (updateInfo.updateRectBounds.isEmpty())
return;

if (!m_backingStore || m_backingStore->size() != updateInfo.viewSize || m_backingStore->deviceScaleFactor() != updateInfo.deviceScaleFactor)
m_backingStore = makeUnique<BackingStore>(updateInfo.viewSize, updateInfo.deviceScaleFactor, m_webPageProxy);

Region damageRegion;
if (updateInfo.scrollRect.isEmpty()) {
for (const auto& rect : updateInfo.updateRects)
damageRegion.unite(rect);
} else
damageRegion = IntRect(IntPoint(), m_webPageProxy.viewSize());

m_backingStore->incorporateUpdate(WTFMove(updateInfo));

m_webPageProxy.setViewNeedsDisplay(damageRegion);
}
#endif

bool DrawingAreaProxyCoordinatedGraphics::alwaysUseCompositing() const
{
return m_webPageProxy.preferences().acceleratedCompositingEnabled() && m_webPageProxy.preferences().forceCompositingMode();
Expand All @@ -203,7 +224,7 @@ void DrawingAreaProxyCoordinatedGraphics::enterAcceleratedCompositingMode(const
{
ASSERT(!isInAcceleratedCompositingMode());
#if !PLATFORM(WPE)
discardBackingStore();
m_backingStore = nullptr;
#endif
m_layerTreeContext = layerTreeContext;
m_webPageProxy.enterAcceleratedCompositingMode(layerTreeContext);
Expand Down Expand Up @@ -265,7 +286,11 @@ void DrawingAreaProxyCoordinatedGraphics::discardBackingStoreSoon()

void DrawingAreaProxyCoordinatedGraphics::discardBackingStore()
{
if (!m_backingStore)
return;

m_backingStore = nullptr;
m_webPageProxy.send(Messages::DrawingArea::DidDiscardBackingStore(), m_identifier);
}
#endif

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,6 @@ class DrawingAreaProxyCoordinatedGraphics final : public DrawingAreaProxy {

bool shouldSendWheelEventsToEventDispatcher() const override { return true; }

#if !PLATFORM(WPE)
void incorporateUpdate(UpdateInfo&&);
#endif

bool alwaysUseCompositing() const;
void enterAcceleratedCompositingMode(const LayerTreeContext&);
void exitAcceleratedCompositingMode();
Expand All @@ -85,6 +81,8 @@ class DrawingAreaProxyCoordinatedGraphics final : public DrawingAreaProxy {
void didUpdateGeometry();

#if !PLATFORM(WPE)
bool forceUpdateIfNeeded();
void incorporateUpdate(UpdateInfo&&);
void discardBackingStoreSoon();
void discardBackingStore();
#endif
Expand Down Expand Up @@ -123,6 +121,7 @@ class DrawingAreaProxyCoordinatedGraphics final : public DrawingAreaProxy {

#if !PLATFORM(WPE)
bool m_isBackingStoreDiscardable { true };
bool m_inForceUpdate { false };
std::unique_ptr<BackingStore> m_backingStore;
RunLoop::Timer m_discardBackingStoreTimer;
#endif
Expand Down
2 changes: 1 addition & 1 deletion Source/WebKit/UIProcess/DrawingAreaProxy.messages.in
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ messages -> DrawingAreaProxy NotRefCounted {
DispatchPresentationCallbacksAfterFlushingLayers(Vector<IPC::AsyncReplyID> callbackIDs);

#if USE(COORDINATED_GRAPHICS) || USE(TEXTURE_MAPPER)
Update(uint64_t stateID, struct WebKit::UpdateInfo updateInfo)
Update(uint64_t stateID, struct WebKit::UpdateInfo updateInfo) CanDispatchOutOfOrder
ExitAcceleratedCompositingMode(uint64_t backingStoreStateID, struct WebKit::UpdateInfo updateInfo)
#endif
}
Original file line number Diff line number Diff line change
Expand Up @@ -428,9 +428,11 @@ void DrawingAreaCoordinatedGraphics::updateGeometry(const IntSize& size, Complet
send(Messages::DrawingAreaProxy::UpdateAcceleratedCompositingMode(0, layerTreeContext));
} else {
UpdateInfo updateInfo;
updateInfo.viewSize = webPage->size();
updateInfo.deviceScaleFactor = webPage->corePage()->deviceScaleFactor();
display(updateInfo);
if (m_isPaintingSuspended) {
updateInfo.viewSize = webPage->size();
updateInfo.deviceScaleFactor = webPage->corePage()->deviceScaleFactor();
} else
display(updateInfo);
if (!m_layerTreeHost)
send(Messages::DrawingAreaProxy::Update(0, WTFMove(updateInfo)));
}
Expand Down Expand Up @@ -833,4 +835,10 @@ void DrawingAreaCoordinatedGraphics::forceUpdate()
display();
}

void DrawingAreaCoordinatedGraphics::didDiscardBackingStore()
{
// Ensure the next update will cover the entire view, since the UI process discarded its backing store.
m_dirtyRegion = m_webPage->bounds();
}

} // namespace WebKit
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ class DrawingAreaCoordinatedGraphics final : public DrawingArea {
void displayDidRefresh() override;
void setDeviceScaleFactor(float) override;
void forceUpdate() override;
void didDiscardBackingStore() override;

#if PLATFORM(GTK)
void adjustTransientZoom(double scale, WebCore::FloatPoint origin) override;
Expand Down
1 change: 1 addition & 0 deletions Source/WebKit/WebProcess/WebPage/DrawingArea.h
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,7 @@ class DrawingArea : public IPC::MessageReceiver, public WebCore::DisplayRefreshM
virtual void targetRefreshRateDidChange(unsigned /*rate*/) { }
virtual void setDeviceScaleFactor(float) { }
virtual void forceUpdate() { }
virtual void didDiscardBackingStore() { }
#endif
virtual void displayDidRefresh() { }

Expand Down
1 change: 1 addition & 0 deletions Source/WebKit/WebProcess/WebPage/DrawingArea.messages.in
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ messages -> DrawingArea NotRefCounted {
TargetRefreshRateDidChange(unsigned rate)
SetDeviceScaleFactor(float deviceScaleFactor)
ForceUpdate()
DidDiscardBackingStore()
#endif

DisplayDidRefresh()
Expand Down

0 comments on commit 3e47e6d

Please sign in to comment.