[webkit-reviews] review denied: [Bug 122016] [CoordinatedGraphics] ASSERTION FAILED: !m_flushingLayers (after r156291) : [Attachment 213691] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Oct 9 02:02:46 PDT 2013
Noam Rosenthal <noam at webkit.org> has denied Sergio Correia
<sergio.correia at openbossa.org>'s request for review:
Bug 122016: [CoordinatedGraphics] ASSERTION FAILED: !m_flushingLayers (after
r156291)
https://bugs.webkit.org/show_bug.cgi?id=122016
Attachment 213691: Patch
https://bugs.webkit.org/attachment.cgi?id=213691&action=review
------- Additional Comments from Noam Rosenthal <noam at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=213691&action=review
> Source/WebCore/ChangeLog:11
> + To prevent calling scheduleLayerFlush() inside
flushCompositingState(),
> + which is incorrect, we now only call m_client->notifyFlushRequired()
--
> + which will trigger scheduleLayerFlush() -- if we are not already
flushing
> + layer changes.
Please use - instead of --, and remove ", which is incorrect", which is
unnecessary :)
>
Source/WebCore/platform/graphics/texmap/coordinated/CompositingCoordinator.cpp:
93
> + notifyFlushRequired(nullptr);
I think we should pass the layer here, even if it's currently not in use.
Having nullptr here is hard to read. Alternatively, have a nullptr default for
CompositingCoordinator::notifyFlushRequired
>
Source/WebCore/platform/graphics/texmap/coordinated/CompositingCoordinator.cpp:
275
> + notifyFlushRequired(nullptr);
Ditto
>
Source/WebCore/platform/graphics/texmap/coordinated/CompositingCoordinator.cpp:
356
> + notifyFlushRequired(nullptr);
Ditto
>
Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cp
p:54
> + if (client() && m_coordinator &&
!m_coordinator->isFlushingLayerChanges()) {
Why do we need && m_coordinator? you already assert for this earlier.
More information about the webkit-reviews
mailing list