Closed
Bug 1082850
Opened 10 years ago
Closed 10 years ago
SkiaGL glue being attempted on every frame on Windows
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
mozilla36
People
(Reporter: vlad, Assigned: jgilbert)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
jgilbert
:
review+
|
Details | Diff | Splinter Review |
CanvasRenderingContext2D::DidRefresh always calls
SkiaGLGlue* glue = gfxPlatform::GetPlatform()->GetSkiaGLGlue();
at http://dxr.mozilla.org/mozilla-central/source/dom/canvas/CanvasRenderingContext2D.cpp#1023
This always tries to create SkiaGLGlue at http://dxr.mozilla.org/mozilla-central/source/gfx/thebes/gfxPlatform.cpp#898 -- and if it fails, it will keep trying to do so. On windows with D2D disabled, I had a (failing) GLContextProviderWGL Skia GL glue creation attempt happening on every frame.
Assignee | ||
Comment 1•10 years ago
|
||
Assignee: nobody → jgilbert
Attachment #8505196 -
Flags: review?(snorp)
Assignee | ||
Comment 2•10 years ago
|
||
Reporter | ||
Comment 3•10 years ago
|
||
Cool, though can't we cache the Skia GL texture uint32_t whenever mTarget changes? I know that it's relatively cheap to query it, but seems more straightforward to just store the value. Or can it change even with the same mTarget?
Assignee | ||
Comment 4•10 years ago
|
||
(In reply to Vladimir Vukicevic [:vlad] [:vladv] from comment #3)
> Cool, though can't we cache the Skia GL texture uint32_t whenever mTarget
> changes? I know that it's relatively cheap to query it, but seems more
> straightforward to just store the value. Or can it change even with the
> same mTarget?
There's less interdependent state this way, so it's better unless we see the query showing up in profiles. (60Hz per canvas, so unlikely)
Assignee | ||
Comment 5•10 years ago
|
||
Comment 6•10 years ago
|
||
Comment on attachment 8505196 [details] [diff] [review]
0001-Query-for-SkiaGL-by-asking-mTarget.patch
Review of attachment 8505196 [details] [diff] [review]:
-----------------------------------------------------------------
Looks good if you fix the one problem
::: dom/canvas/CanvasRenderingContext2D.cpp
@@ +4828,5 @@
> +
> +uint32_t
> +CanvasRenderingContext2D::SkiaGLTex() const
> +{
> + return (uint32_t)(uintptr_t)mTarget->GetNativeSurface(NativeSurfaceType::OPENGL_TEXTURE);
mTarget can be null or bogus. Guard with IsTargetValid().
Attachment #8505196 -
Flags: review?(snorp) → review+
Assignee | ||
Comment 7•10 years ago
|
||
Done.
Attachment #8505196 -
Attachment is obsolete: true
Attachment #8505675 -
Flags: review+
Assignee | ||
Comment 8•10 years ago
|
||
Comment 9•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in
before you can comment on or make changes to this bug.
Description
•