Set gfx.canvas.azure.backends to skia. Crash trying to run ./mach mochitest-plain layout/generic/test/test_bug449653.html

BasicLayerManager::PaintSelfOrChildren calls aGroupTarget->GetDrawTarget() which gives back DrawTargetCG and we eventually get trouble as we have SourceSurfaceSkia.

Comment on attachment 8447437 [details] [diff] [review] Create canvas specific draw target Review of attachment 8447437 [details] [diff] [review]: ----------------------------------------------------------------- Looks fine to me. We may want to add support for SourceSurfaceSkia objects in DrawTargetCG's DrawSurface as well at some point?

(In reply to George Wright (:gw280) from comment #3) > Comment on attachment 8447437 [details] [diff] [review] > Create canvas specific draw target > > Review of attachment 8447437 [details] [diff] [review]: > ----------------------------------------------------------------- > > Looks fine to me. We may want to add support for SourceSurfaceSkia objects > in DrawTargetCG's DrawSurface as well at some point? Any performance implications with DrawTargetCG+SourceSurfaceSkia vs. DrawTargetSkia+SourceSurfaceSkia?

(In reply to George Wright (:gw280) from comment #3) > Comment on attachment 8447437 [details] [diff] [review] > Create canvas specific draw target > > Review of attachment 8447437 [details] [diff] [review]: > ----------------------------------------------------------------- > > Looks fine to me. We may want to add support for SourceSurfaceSkia objects > in DrawTargetCG's DrawSurface as well at some point? You're right, bug 1031866 hits the scenario where that is the only thing that will work.

Comment on attachment 8447437 [details] [diff] [review] Create canvas specific draw target Review of attachment 8447437 [details] [diff] [review]: ----------------------------------------------------------------- Actually, I don't think we should do this. We've hit this branch because we decided that we weren't happy with the mTarget's backend (skia) being used for content drawn, going ahead and creating a new skia DT to replace it isn't useful. We either need to fix the interop between SourceSurfaceCG and DrawTargetSkia, or make SupportsAzureContentForDrawTarget return true for skia on OSX.

Thanks Matt, had a feeling it was something like that.

Is this the direction of what you guys had in mind?

Comment on attachment 8454697 [details] [diff] [review] DrawTargetCG + SourceSurfaceSkia support Review of attachment 8454697 [details] [diff] [review]: ----------------------------------------------------------------- Yeah that looks reasonable to me.

Comment on attachment 8454697 [details] [diff] [review] DrawTargetCG + SourceSurfaceSkia support Review of attachment 8454697 [details] [diff] [review]: ----------------------------------------------------------------- Looks fine, but I wonder if we need to differentiate between SurfaceType::DATA, SurfaceType::SKIA, and SurfaceType::ALLTHEOTHERS. Both DrawTargetD2D and DrawTargetCairo just call GetDataSurface (which should work for all types) if it's not a native D2D/CAIRO typed SourceSurface.

Yes, that would be a clean change. What I'd like to do on top is "tag" that "else" as a "slower path" somehow, so that we can notice when things like this happen. Separate story.