897635 - Demote GL-accelerated Skia canvas to software if we use non-standard compositing modes

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[George Wright (:gw280) (needinfo me!)]

Canvas specifies 11 standard compositing modes, and there are a bunch more experimental ones. The core set is supported by SkiaGL, but the advanced ops will require us to pull in a newer version of Skia. In the interim, let's fallback to software when we use these non-standard modes so that we don't break them.

Comment 1

[Jeff Muizelaar [:jrmuizel]]

(In reply to George Wright (:gw280) from comment #0)
> Canvas specifies 11 standard compositing modes, and there are a bunch more
> experimental ones. The core set is supported by SkiaGL, but the advanced ops
> will require us to pull in a newer version of Skia. In the interim, let's
> fallback to software when we use these non-standard modes so that we don't
> break them.

I don't believe the experimental ones are supported in any of the other backends right now, so this shouldn't be much of a problem.

Comment 2

[Jeff Muizelaar [:jrmuizel]]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #1)
> (In reply to George Wright (:gw280) from comment #0)
> > Canvas specifies 11 standard compositing modes, and there are a bunch more
> > experimental ones. The core set is supported by SkiaGL, but the advanced ops
> > will require us to pull in a newer version of Skia. In the interim, let's
> > fallback to software when we use these non-standard modes so that we don't
> > break them.
> 
> I don't believe the experimental ones are supported in any of the other
> backends right now, so this shouldn't be much of a problem.

I misinterpreted the set of blend modes with a different set. Ignore my previous comment.

Comment 3

[Jeff Muizelaar [:jrmuizel]]

This is what I was thinking about:
http://lists.w3.org/Archives/Public/public-fx/2012JulSep/0153.html

Comment 4

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: demote-non-standard.diff

George has a patch for this already, would be good to get some review

Comment 5

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 783119 [details] [diff] [review]
demote-non-standard.diff

Review of attachment 783119 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/canvas/src/CanvasRenderingContext2D.cpp
@@ +3193,5 @@
>    // XXX ERRMSG we need to report an error to developers here! (bug 329026)
>    else return;
>  
> +#ifdef USE_SKIA_GPU
> +  if (!IsStandardCompositeOp(comp_op)) {

Comment somewhere that this is temporary until we get a newer version of skia (at least for desktop).

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/cf17bd948e44