Closed Bug 1082850 Opened 5 years ago Closed 5 years ago

SkiaGL glue being attempted on every frame on Windows

Categories

(Core :: Graphics, defect, major)

x86_64
Windows 8
defect
Not set
major

Tracking

()

RESOLVED FIXED
mozilla36

People

(Reporter: vlad, Assigned: jgilbert)

References

Details

Attachments

(1 file, 1 obsolete file)

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: nobody → jgilbert
Attachment #8505196 - Flags: review?(snorp)
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?
(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)
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+
Done.
Attachment #8505196 - Attachment is obsolete: true
Attachment #8505675 - Flags: review+
Depends on: 1066280
https://hg.mozilla.org/mozilla-central/rev/b39cff5628df
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.