Closed Bug 1206763 Opened 10 years ago Closed 10 years ago

enable SkiaGL canvas on gonk

Categories

(Core :: Graphics: Canvas2D, defect)

Product:
Core
Component:
Graphics: Canvas2D
Platform:
All
Unspecified
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla44
Tracking Flags:
Tracking Status
firefox44 --- fixed

People

(Reporter: sotaro, Assigned: sotaro)

References

Details

(Keywords: regression)

Attachments

(1 file, 5 obsolete files)

patch - Enable SkiaGL canvas on gonk
4.47 KB, patch
sotaro
: review+
Details | Diff | Splinter Review
We need to enable SkiaGL on gonk as soon as possible. See bug 1206076 Comment 9.
bug 1172719 might be related. It is a fix to SurfaceStream's gralloc buffer recycling.
Assignee: nobody → sotaro.ikeda.g
(In reply to Sotaro Ikeda [:sotaro] from comment #3) > check if bug 1172719 is related. > https://treeherder.mozilla.org/#/jobs?repo=try&revision=45a20200920f bug 1172719 seems not related.
Tryserver (Remove test_imagebitmap_cropping.html) https://treeherder.mozilla.org/#/jobs?repo=try&revision=d4cab131eb82 Only the following test was failed. > REFTEST TEST-UNEXPECTED-FAIL | http://10.0.2.2:8888/tests/dom/canvas/test/reftest/1177726-text-stroke-bounds.html | image comparison (==), max difference: 1, number of differing pixels: 1
> 303 INFO TEST-UNEXPECTED-FAIL | dom/canvas/test/test_imagebitmap_cropping.html | pixel 0,0 of is 0,60,136,255; expected 255,255,255,255 +/- 5 > 304 INFO TEST-UNEXPECTED-FAIL | dom/canvas/test/test_imagebitmap_cropping.html | pixel 50,0 of is 0,64,144,255; expected 255,255,0,255 +/- 5 > 309 INFO TEST-UNEXPECTED-FAIL | dom/canvas/test/test_imagebitmap_cropping.html | pixel 100,100 of is 0,60,136,255; expected 255,255,255,255 +/- 5 > 310 INFO TEST-UNEXPECTED-FAIL | dom/canvas/test/test_imagebitmap_cropping.html | pixel 150,120 of is 0,65,142,255; expected 255,255,0,255 +/- 5 I suspected that a lot of test failure like the above is caused by video rendering optimization code in CanvasRenderingContext2D::DrawImage() https://dxr.mozilla.org/mozilla-central/source/dom/canvas/CanvasRenderingContext2D.cpp#4417
Tryserver (Remove video optimization in CanvasRenderingContext2D::DrawImage()) https://treeherder.mozilla.org/#/jobs?repo=try&revision=c8b64976c72a Test failures of test_imagebitmap_cropping.html seems to be fixed.
For the time being, disable video rendering optimization.
Update nits.
Attachment #8664832 - Attachment is obsolete: true
Bug 1150944 fixed SkiaGL's video rendering problem. It seems to be landed soon.
Depends on: 1150944
Bug 1207153 seems to be related to GLBlitHelper::BlitTextureToTexture crash.
(In reply to Sotaro Ikeda [:sotaro] from comment #12) > Bug 1207153 seems to be related to GLBlitHelper::BlitTextureToTexture crash. Bug 1207153 is not related to tge GLBlitHelper::BlitTextureToTexture crash. tryserver(With Bug 1207153 fix) did not fixe the crash. https://treeherder.mozilla.org/#/jobs?repo=try&revision=ce6dac24adab
Remove disabling video optimization.
Attachment #8664856 - Attachment is obsolete: true
Remove fuzz change.
Attachment #8665420 - Attachment is obsolete: true
Tryserver(with additional log) says that DrawTargetSkia's GL texture was deleted before GLBlitHelper::BlitTextureToTexture() was called. https://treeherder.mozilla.org/logviewer.html#?job_id=11907089&repo=try
Tryserver(with additional log around DrawTargetSkia) https://treeherder.mozilla.org/logviewer.html#?job_id=11916498&repo=try From the following log, the crash happened because of DrawTargetSkia recreation. When DrawTargetSkia was recreated, CopyableCanvasLayer::mGLFrontbuffer was not updated. When GLBlitHelper::BlitTextureToTexture() was called, the gl texture of DrawTargetSkia was already deleted. ---------------------------------------------------------------------------------- 62): SharedSurface_Gralloc::Create ------- 21:07:55 INFO - 09-25 04:05:43.726 I/Gecko ( 862): SharedSurface_Gralloc::Create: success -- surface 0x45512430, GraphicBuffer 0x46e53b80. 21:07:55 INFO - 09-25 04:05:43.746 I/Gecko ( 862): CanvasRenderingContext2D::SwitchRenderingMode_E aRenderingMode 0 ------- 21:07:55 INFO - 09-25 04:05:43.746 I/Gecko ( 862): DrawTargetSkia::DrawTargetSkia() ------- 21:07:55 INFO - 09-25 04:05:43.966 I/Gecko ( 862): glDeleteTextures_mozilla_E ------- 21:07:55 INFO - 09-25 04:05:43.976 I/Gecko ( 862): glDeleteTextures_mozilla textures[0] texture 230 ------- 21:07:55 INFO - 09-25 04:05:43.986 I/Gecko ( 862): DrawTargetSkia::~DrawTargetSkia() mTexture 225 ------- 21:07:55 INFO - 09-25 04:05:43.986 I/Gecko ( 862): glDeleteTextures_mozilla_E ------- 21:07:55 INFO - 09-25 04:05:43.996 I/Gecko ( 862): glDeleteTextures_mozilla textures[0] texture 225 ------- 21:07:55 INFO - 09-25 04:05:44.016 I/Gecko ( 862): SharedSurface_Gralloc::Create ------- 21:07:55 INFO - 09-25 04:05:44.026 I/Gecko ( 862): SharedSurface_Gralloc::Create: success -- surface 0x45512628, GraphicBuffer 0x46efc880. 21:07:55 INFO - 09-25 04:05:44.046 I/Gecko ( 862): CanvasRenderingContext2D::SwitchRenderingMode_E aRenderingMode 0 ------- 21:07:55 INFO - 09-25 04:05:44.046 I/Gecko ( 862): DrawTargetSkia::DrawTargetSkia() ------- 21:07:55 INFO - 09-25 04:05:44.156 I/Gecko ( 862): glDeleteTextures_mozilla_E ------- 21:07:55 INFO - 09-25 04:05:44.156 I/Gecko ( 862): glDeleteTextures_mozilla textures[0] texture 232 ------- 21:07:55 INFO - 09-25 04:05:44.156 I/Gecko ( 862): DrawTargetSkia::~DrawTargetSkia() mTexture 227 ------- 21:07:55 INFO - 09-25 04:05:44.356 I/Gecko ( 862): SharedSurface_Gralloc::Create ------- 21:07:55 INFO - 09-25 04:05:44.377 I/Gecko ( 862): SharedSurface_Gralloc::Create: success -- surface 0x455b74d0, GraphicBuffer 0x46f14280. 21:07:55 INFO - 09-25 04:05:44.377 I/Gecko ( 862): GLBlitHelper::BlitTextureToTexture mGL 0x46f02000 srcTex 225 destTex 233 srcSize.width 256 srcSize.height 256 destSize.width 256 destSize.height 256 srcTarget de1 destTarget de1 -------
I succeeded to reproduce the problem on master flame-kk with the following url. - https://developer.mozilla.org/en-US/docs/Web/API/Canvas_API/Tutorial/Pixel_manipulation_with_canvas The problem happened when the following conditions were met. - CanvasDrawObserver::FrameEnd() decide to fallback from OpenGLBackendMode to SoftwareBackendMode. Then call CanvasRenderingContext2D::SwitchRenderingMode() - Next ClientCanvasLayer::RenderLayer() was called without CanvasRenderingContext2D::GetCanvasLayer() call CanvasRenderingContext2D expects to update ClientCanvasLayer by the GetCanvasLayer(). But there was a case that ClientCanvasLayer::RenderLayer() was called without calling GetCanvasLayer(). Therefore ClientCanvasLayer continues to hold old GL Texture.
GetCanvasLayer() is called from nsHTMLCanvasFrame::BuildLayer().
I tried several ways, but I could not find a way to ensure that GetCanvasLayer() is called in next layer transaction.
Just ensure lifetime of DrawTargetSkia when ClientCanvasLayer use its GL texture.
Attachment #8665426 - Attachment is obsolete: true
Comment on attachment 8666413 [details] [diff] [review] patch - Enable SkiaGL canvas on gonk :mattwoodrow, can you review the patch?
Attachment #8666413 - Flags: review?(matt.woodrow)
Comment on attachment 8666413 [details] [diff] [review] patch - Enable SkiaGL canvas on gonk Review of attachment 8666413 [details] [diff] [review]: ----------------------------------------------------------------- Why do we want to make the buffer provider available to the layer when we have skia-gl?
(In reply to Matt Woodrow (:mattwoodrow) from comment #26) > > Why do we want to make the buffer provider available to the layer when we > have skia-gl? To keep OpenGL texture alive that is created by DrawTargetSkia. When the problem in comment 17 happened, CopyableCanvasLayer held a stale OpenGL texture as SharedSurface_Basic. The OpenGL texture was created by DrawTargetSkia and the texture's lifetime was bound to DrawTargetSkia's lifetime. The texture was already deleted because DrawTargetSkia was already deleted. The DrawTargetSkia was deleted because CanvasRenderingContext2D decided to fallback from OpenGLBackendMode to SoftwareBackendMode in CanvasDrawObserver::FrameEnd(). https://dxr.mozilla.org/mozilla-central/source/dom/canvas/CanvasRenderingContext2D.cpp#774 CanvasRenderingContext2D seems to expect that CanvasRenderingContext2D::GetCanvasLayer() is called before ClientCanvasLayer::RenderLayer() calling. But there is a case that the GetCanvasLayer() is not called before the RenderLayer(). PersistentBufferProvider holds DrawTarget. Therefore holding PersistentBufferProvider has an effect of extending DrawTarget's lifetime. Even when CanvasRenderingContext2D clears the DrawTarget, if CopyableCanvasLayer holds PersistentBufferProvider, we could keep the DrawTarget alive and it makes OpenGL texture alive.
Flags: needinfo?(matt.woodrow)
If we want to hold OpenGL texture in CopyableCanvasLayer, we need to ensure its lifetime. attachment 8666413 [details] [diff] [review] does it by holding PersistentBufferProvider. There might be better way to do it. https://github.com/sotaroikeda/firefox-diagrams/blob/master/dom/dom_canvas_CanvasRenderingContext2D_FirefoxOS_2_5.pdf
Attachment #8666413 - Flags: review?(matt.woodrow) → review+
Flags: needinfo?(matt.woodrow)
Rebased.
Attachment #8666413 - Attachment is obsolete: true
Attachment #8676047 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/402cca1e7b46
Status: NEW → RESOLVED
Closed: 10 years ago
status-firefox44: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: