We should stop creating a Thebes backed gfxContext in CanvasRenderingContext2D.cpp, and kill off nsICanvasRenderingContextInternal::GetThebesSurface.

Backed out for Crashtest orange (assertion failure): https://hg.mozilla.org/integration/mozilla-inbound/rev/bd167d38965c

The issue is that the gfxPattern |pat| in CanvasRenderingContext2D::Render can't be used with the gfxContext that is passed in because the gfxContext is gfxASurface backed. In debug builds we hit the MOZ_ASSERT(!pattern->IsAzure()) in gfxContext::SetPattern; in opt builds we fail painting. The gfxContext is gfxASurface backed because nsLayoutUtils::SurfaceFromElement(HTMLCanvasElement* aElement, ...) creates such a context if SFE_NO_PREMULTIPLY_ALPHA is passed in. This all seems a bit weird to me since as I understand it both Thebes and Moz2D surfaces are pre-multiplied. Even weirder is that the gfxImageSurface that is used to create the gfxContext is wrapping a DataSourceSurface's data, and it's the DataSourceSurface that is used after the Render(). So are we abusing SourceSurface and making it non-pre-multiplied here?

It seems like a cleaner way to handle SFE_NO_PREMULTIPLY_ALPHA would be to work with pre-multiplied SourceSurfaces and only un-pre-multiply further up the call stack when the SourceSurface data is read and used to create whatever it is that is passed out to WebGL code.

Try push showing failures: https://tbpl.mozilla.org/?tree=Try&rev=53cdc6acdec5 Multiple failures on Linux, OS X and Windows in: content/canvas/test/webgl-conformance/test_webgl_conformance_test_suite.html browser/devtools/shared/test/browser_telemetry_button_tilt.js content/html/content/crashtests/798802-1.html content/html/content/crashtests/798802-1.html The mochitest-plain-1 failures are debug and opt failures where pixel data is wrong. The "bc" and "C" failures being are debug only due to failing the assertion mentioned in comment 4.

(In reply to Jonathan Watt [:jwatt] from comment #4) > So are we abusing > SourceSurface and making it non-pre-multiplied here? Yes we are. (In reply to Jonathan Watt [:jwatt] from comment #5) > It seems like a cleaner way to handle SFE_NO_PREMULTIPLY_ALPHA would be to > work with pre-multiplied SourceSurfaces and only un-pre-multiply further up > the call stack when the SourceSurface data is read and used to create > whatever it is that is passed out to WebGL code. The problem case here is when drawing WebGL -> WebGL with SFE_NO_PREMULTIPLY_ALPHA. The source data is probably already non-premultiplied, so no conversions are necessary. With your propsal we'd end up converting twice.

Okay, well I'd have to spend some time on this to get enough of a handle on things to come up with a solution to getting this patch landed. Due to upcoming PTO I'm unlikely to work on this again until May though (hence the dump of info above). If anyone else wants to pursue this, feel free.

From IRC: mattwoodrow jwatt: Looking at the non-premultiplied stuff mattwoodrow looks like there’s only two callers, so we can just change the API a bit mattwoodrow how about making it return a surface instead of drawing to a context mattwoodrow with an input flag ‘i’m ok with getting back a non-premultiplied result’ mattwoodrow and an output saying whether that actually happened or not mattwoodrow then the only caller that would pass that flag (webgl) can post-process if it needs to jgilbert mattwoodrow, if it's only a hint, pleaseplease make this obvious from the API mattwoodrow Yes of course

I don't think that works - see my comment in bug 995409 comment 4.

More recent Try push, post-bug 996393: https://tbpl.mozilla.org/?tree=Try&rev=d14e0b0075aa

(In reply to Jonathan Watt [:jwatt] [offline 17-28 April] from comment #9) > From IRC: > > mattwoodrow jwatt: Looking at the non-premultiplied stuff > mattwoodrow looks like there’s only two callers, so we can just change the > API a bit > mattwoodrow how about making it return a surface instead of drawing to a > context > mattwoodrow with an input flag ‘i’m ok with getting back a non-premultiplied > result’ > mattwoodrow and an output saying whether that actually happened or not > mattwoodrow then the only caller that would pass that flag (webgl) can > post-process if it needs to > jgilbert mattwoodrow, if it's only a hint, pleaseplease make this obvious > from the API > mattwoodrow Yes of course I filed bug 997014 for this idea.

Rebased patch on top of 997014. Carrying r=mattwoodrow