In the process of refactoring, the root damage rect got "disconnected" from where it is used to perform the partial swap. The fix is very straightforward, and works fine. However, I would like to add a unit test to make sure this does not happen again. An effective unit test needs to mock the "getExtensions()" accessor, so we can pretend that partial swap is supported. Currently getExtensions() is implemented as non-virtual in un-mockable GraphicsContext3DPrivate. Is it OK to move the implementation of getExtensions() from GraphicsContext3DPrivate into WebGraphicsContext3D, where we can make it virtual and mock-able? or is there some reason not to do this?
Extensions3DChromium simply thunks to WebGraphicsContext3D for all of its operations and WebGraphicsContext3D is fully virtual and easy to mock. I don't understand why you would need more. Is there some scenario that you can't mock out today?
Thanks, it works exactly as you suggested. Will post a patch soon.
OK cool, glad that worked. If it helps shuffling around all of the Platform headers should make it pretty easy for us to cut out at least one layer of indirection in our GraphicsContext3D->WebGraphicsContext3D mappings.
Created attachment 119972 [details] Patch This patch has (1) the fix itself (2) appropriate unit test (3) drive-by style fix on ordering of include files.
Comment on attachment 119972 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=119972&action=review haven't looked through full patch, just some drive by nits for now. will try to look at this later > Source/WebCore/platform/graphics/chromium/LayerRendererChromium.h:108 > + void swapBuffers(IntRect subBuffer); we typically pass IntRects by const ref, can we do that here? > Source/WebKit/chromium/tests/CCLayerTreeHostImplTest.cpp:311 > + FakeDrawableCCLayerImpl(int id) : CCLayerImpl(id) { } nit: explicit > Source/WebKit/chromium/tests/CCLayerTreeHostImplTest.cpp:337 > +class PartialSwapTrackerContext: public MockWebGraphicsContext3D { nit: space between PartialSwapTrackerContext and the :
Created attachment 119981 [details] addressed nits and added one more case to unit test
Created attachment 120960 [details] updated to close to tip of tree
Comment on attachment 120960 [details] updated to close to tip of tree R=me
Comment on attachment 120960 [details] updated to close to tip of tree Clearing flags on attachment: 120960 Committed r103988: <http://trac.webkit.org/changeset/103988>
All reviewed patches have been landed. Closing bug.