Chromium Code Reviews Chromium Code Reviews
Issue
2455983005:
Refactor AcceleratedStaticBitmapImage (Closed)
DescriptionRefactor AcceleratedStaticBitmapImage
This CL solves this problems:
Calling OffscreenCanvas's transferToImageBitmap changes WebGL's
DepthTest states.
The cause is that in the method DrawingBuffer::transferToStaticBitmapImage(),
it calls SkImage::MakeFromAdoptedTexture(). The first parameter that
we passed to MakeFromAdoptedTexture method is WebGL's GrContext,
and can change all kinds of WebGL states.
To solve this, we should not using a SkImage to wrap up WebGL's mailbox
and syncToken. But the problem is: how are we going to keep them alive.
So this CL refactors the AcceleratedStaticBitmapImage class. In particular,
we designed an abstract class TextureHolder, which has two sub-classes:
SkiaTextureHolder, MailboxTextureHolder. Then AcceleratedStaticBitmapImage
holds an instance of TextureHolder. Being a SkiaTextureHolder means that
the AcceleratedStaticBitmapImage is created from a texture-backed SkImage.
Being a MailboxTextureHolder means that the AcceleratedStaticBitmapImage
is created by passing in WebGL's mailbox, syncToken and associated texture
Id to keep the texture alive.
This CL also fixes the problem where WebGL's commit have flickering. The cause
of the flickering is that in dispatchFrame() function call which takes a
StaticBitmapImage |image|, we accidentally called verifyImageSize(image->imageForCurrentFrame())),
the image->imageForCurrentFrame() consumes the mailbox and convert the mailbox into a
texture-backed SkImage, then when we prepare transferable resources from the image,
we call image->ensureMailbox() which converts the SkImage back to GPU texture again.
This is totally un-necessary. The fix is just let verifyImageSize take image->size().
This CL also fixes a crash on releaseImageThreadSafe method.
Currently in the AcceleratedStaticBitmapImage class, it keeps a pointer
m_imageThread which stores the thread where m_image is created. Then in
the method releaseImageThreadSafe(), it calls m_imageThread->getWebTaskRunner()
to release the m_image on the original thread where it was created. It
could crash in this case: when m_image was created on a worker thread,
and then transferred to the main thread, when releaseImageThreadSafe()
is invoked on the main thread, the worker thread is already out of scope.
So at this moment, calling m_imageThread->getWebTaskRunner() results in
a crash.
The fix of it is to store a clone of the WebTaskRunner for m_imageThread.
BUG=657975, 661614, 658251
Committed: https://crrev.com/5717f6e18242d06a01cab340f086624b8f55cad6
Cr-Commit-Position: refs/heads/master@{#430865}
Patch Set 1 #
Total comments: 4
Patch Set 2 : deleteTexture in desctructor #
Total comments: 13
Patch Set 3 : Add layout test, comment for PS2 not address, DO NOT COMMIT this PS #Patch Set 4 : build is fine, layout tests still crashes #Patch Set 5 : WeakPtr, still figuring out why one layout test timed out #
Total comments: 1
Patch Set 6 : keep originClean and isPremultiplied flag at StaticBitmap base class #Patch Set 7 : minor clean up #
Total comments: 6
Patch Set 8 : SkiaTextureHolder keeps a clone of WebTaskRunner #Patch Set 9 : minor clean up #Patch Set 10 : fix compile error #Patch Set 11 : fix crash #
Total comments: 3
Patch Set 12 : fix flickering #
Total comments: 1
Patch Set 13 : keep an additional m_imageThreadDoNotCall #Patch Set 14 : rebase #
Total comments: 3
Patch Set 15 : release texture thread safe #Patch Set 16 : No long keep WeakPtr<DrawingBuffer> #
Total comments: 14
Patch Set 17 : no crash! && address comments #
Total comments: 6
Patch Set 18 : Minor clean up, almost identical to PS17 #Patch Set 19 : address comments #Patch Set 20 : Keep a unique_ptr<WebGraphicsContext3DProvider>, otherwise crashes #Messages
Total messages: 59 (21 generated)
|
