Bug 105074 - Add Canvas blend modes to Cairo
Summary: Add Canvas blend modes to Cairo
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Canvas (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: arno.
URL:
Keywords:
Depends on:
Blocks: 100069
  Show dependency treegraph
 
Reported: 2012-12-14 17:03 PST by arno.
Modified: 2013-01-15 16:00 PST (History)
4 users (show)

See Also:


Attachments
wip patch (5.38 KB, patch)
2012-12-19 16:51 PST, arno.
no flags Details | Formatted Diff | Diff
patch proposal (6.73 KB, patch)
2012-12-26 10:45 PST, arno.
no flags Details | Formatted Diff | Diff
test diff (24.45 KB, text/html)
2012-12-27 10:13 PST, arno.
no flags Details
patch with platform specific expected files (107.35 KB, patch)
2013-01-14 10:54 PST, arno.
no flags Details | Formatted Diff | Diff
same patch with Reviewed by NOBODY (OOPS!) added to LayoutTests/ChangeLog (107.42 KB, patch)
2013-01-15 14:46 PST, arno.
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description arno. 2012-12-14 17:03:41 PST
This bug will add the canvas blend modes to Cairo
Comment 1 arno. 2012-12-19 14:33:51 PST
I tried to add support for blending modes in cairo.
While most modes work correctly, color-burn, color-dogde, hue, luminosity and saturation mode do not work.

That's Pixman implementation is based on svg specification; and I think there is a difference between svg specification and the tests added in bug #104176 (and possibly also with pdf specification).

Pdf specification[1] states that the resulting color for color-burn compositing will be

Cr = (1 – as / ar) * Cb + as / ar × [( 1 – ab) × Cs + ab × B(Cb, Cs)]
with B(Cb, Cs) = 1 - min(1, (1 - Cb) / Cs) if Cs > 0, or 0 otherwise

So if we have a opaque source and backdrops (a = 1) and a color component of 0 for source, and 1 for backdrop, the result should be:
(1 - 1) * 1 + 1 / 1 * ((1 - 1) * 0 + 1 * 0) => 0
That is what is assumed in the tests for #104176.

But svg specification[2] states that result for color-burn compositing:

if Sca == 0 and Dca == Da
  Dca' = Sa × Da + Sca × (1 - Da) + Dca × (1 - Sa)
       = Sa × Da + Dca × (1 - Sa)
otherwise if Sca == 0
  Dca' = Sca × (1 - Da) + Dca × (1 - Sa)
       = Dca × (1 - Sa)
otherwise if Sca > 0
  Dca' = Sa × Da - Sa × Da × min(1, (1 - Dca/Da) × Sa/Sca) + Sca × (1 - Da) + Dca × (1 - Sa)
       = Sa × Da × (1 - min(1, (1 - Dca/Da) × Sa/Sca)) + Sca × (1 - Da) + Dca × (1 - Sa)

For the same backdrop and sources, we have
Sca = 0; Sa = 1
Dca = 1; Da = 1
So, we are in the first case, and the result should be:
1 * 1 + 0 * (1 - 1) + 1 * (1 - 1) => 1
This is what pixman computes.
(By the way, I don't really understand why there is this Dca == Da check).

I have not checked other modes yet, but I suppose there may be a the same problem there.

So am I correct, is the a difference between pdf specification and svg specification. If so, which one are we supposed to follow now ?

[1]: http://wwwimages.adobe.com/www.adobe.com/content/dam/Adobe/en/devnet/pdf/pdfs/PDF32000_2008.pdf 11.3.5
[2]: http://www.w3.org/TR/SVGCompositing/#comp-op-property
Comment 2 Rik Cabanier 2012-12-19 14:59:27 PST
(In reply to comment #1)
> I tried to add support for blending modes in cairo.
> While most modes work correctly, color-burn, color-dogde, hue, luminosity and saturation mode do not work.
> 
> That's Pixman implementation is based on svg specification; and I think there is a difference between svg specification and the tests added in bug #104176 (and possibly also with pdf specification).
> 
> Pdf specification[1] states that the resulting color for color-burn compositing will be
> 
> Cr = (1 – as / ar) * Cb + as / ar × [( 1 – ab) × Cs + ab × B(Cb, Cs)]
> with B(Cb, Cs) = 1 - min(1, (1 - Cb) / Cs) if Cs > 0, or 0 otherwise
> 
> So if we have a opaque source and backdrops (a = 1) and a color component of 0 for source, and 1 for backdrop, the result should be:
> (1 - 1) * 1 + 1 / 1 * ((1 - 1) * 0 + 1 * 0) => 0
> That is what is assumed in the tests for #104176.
> 
> But svg specification[2] states that result for color-burn compositing:
> 
> if Sca == 0 and Dca == Da
>   Dca' = Sa × Da + Sca × (1 - Da) + Dca × (1 - Sa)
>        = Sa × Da + Dca × (1 - Sa)
> otherwise if Sca == 0
>   Dca' = Sca × (1 - Da) + Dca × (1 - Sa)
>        = Dca × (1 - Sa)
> otherwise if Sca > 0
>   Dca' = Sa × Da - Sa × Da × min(1, (1 - Dca/Da) × Sa/Sca) + Sca × (1 - Da) + Dca × (1 - Sa)
>        = Sa × Da × (1 - min(1, (1 - Dca/Da) × Sa/Sca)) + Sca × (1 - Da) + Dca × (1 - Sa)
> 
> For the same backdrop and sources, we have
> Sca = 0; Sa = 1
> Dca = 1; Da = 1
> So, we are in the first case, and the result should be:
> 1 * 1 + 0 * (1 - 1) + 1 * (1 - 1) => 1
> This is what pixman computes.
> (By the way, I don't really understand why there is this Dca == Da check).
> 
> I have not checked other modes yet, but I suppose there may be a the same problem there.
> 
> So am I correct, is the a difference between pdf specification and svg specification. If so, which one are we supposed to follow now ?

In this case, the formula in the SVG specification is correct for color-burn and color-dodge.
I didn't update the formula in the CSS compositing spec yet, but will do so shortly.

I'm surprised to see that hue, luminosity and saturation mode do not work. Are they very different?
Comment 3 Rik Cabanier 2012-12-19 15:09:29 PST
> I didn't update the formula in the CSS compositing spec yet, but will do so shortly.
Updated the spec. See:
https://dvcs.w3.org/hg/FXTF/rawfile/tip/compositing/index.html#blendingcolordodge
Comment 4 arno. 2012-12-19 16:51:50 PST
Created attachment 180248 [details]
wip patch

wip patch.
Some changes are needed in CanvasRenderingContext2D are needed to have it working. They are currently implemented in #100070
I'll finish the patch when that bug is fixed.
Comment 5 arno. 2012-12-26 10:45:00 PST
Created attachment 180756 [details]
patch proposal
Comment 6 Rik Cabanier 2012-12-26 20:23:11 PST
(In reply to comment #5)
> Created an attachment (id=180756) [details]
> patch proposal

This looks reasonable.
Can you remove the exception for the testexpectations file so this can be tested?
Comment 7 arno. 2012-12-27 10:13:14 PST
Created attachment 180808 [details]
test diff

Actually, I still get test errors.
This time with

color-burn alpha on solid and alpha on alpha.
hue  alpha on solid and alpha on alpha.
saturation  alpha on solid and alpha on alpha.
color  alpha on solid and alpha on alpha.

Is the test or pixman more likely incorrect ?
Comment 8 Rik Cabanier 2012-12-28 13:17:09 PST
(In reply to comment #7)
> Created an attachment (id=180808) [details]
> test diff
> 
> Actually, I still get test errors.
> This time with
> 
> color-burn alpha on solid and alpha on alpha.
> hue  alpha on solid and alpha on alpha.
> saturation  alpha on solid and alpha on alpha.
> color  alpha on solid and alpha on alpha.
> 
> Is the test or pixman more likely incorrect ?

the results are pretty close so cairo probably needs a bit of tweaking.
Can you point me to wiki that tell me how I can build your code?
Comment 9 arno. 2012-12-28 13:44:53 PST
(In reply to comment #8)
> Can you point me to wiki that tell me how I can build your code?

Here is how to build WebKitGTK
http://trac.webkit.org/wiki/BuildingGtk
Comment 10 Rik Cabanier 2012-12-28 20:10:43 PST
(In reply to comment #1)
> I tried to add support for blending modes in cairo.
> While most modes work correctly, color-burn, color-dogde, hue, luminosity and saturation mode do not work.
> 
> That's Pixman implementation is based on svg specification; and I think there is a difference between svg specification and the tests added in bug #104176 (and possibly also with pdf specification).
> 
> Pdf specification[1] states that the resulting color for color-burn compositing will be
> 
> Cr = (1 – as / ar) * Cb + as / ar × [( 1 – ab) × Cs + ab × B(Cb, Cs)]
> with B(Cb, Cs) = 1 - min(1, (1 - Cb) / Cs) if Cs > 0, or 0 otherwise
> 
> So if we have a opaque source and backdrops (a = 1) and a color component of 0 for source, and 1 for backdrop, the result should be:
> (1 - 1) * 1 + 1 / 1 * ((1 - 1) * 0 + 1 * 0) => 0
> That is what is assumed in the tests for #104176.
> 
> But svg specification[2] states that result for color-burn compositing:
> 
> if Sca == 0 and Dca == Da
>   Dca' = Sa × Da + Sca × (1 - Da) + Dca × (1 - Sa)
>        = Sa × Da + Dca × (1 - Sa)
> otherwise if Sca == 0
>   Dca' = Sca × (1 - Da) + Dca × (1 - Sa)
>        = Dca × (1 - Sa)
> otherwise if Sca > 0
>   Dca' = Sa × Da - Sa × Da × min(1, (1 - Dca/Da) × Sa/Sca) + Sca × (1 - Da) + Dca × (1 - Sa)
>        = Sa × Da × (1 - min(1, (1 - Dca/Da) × Sa/Sca)) + Sca × (1 - Da) + Dca × (1 - Sa)
> 
> For the same backdrop and sources, we have
> Sca = 0; Sa = 1
> Dca = 1; Da = 1
> So, we are in the first case, and the result should be:
> 1 * 1 + 0 * (1 - 1) + 1 * (1 - 1) => 1
> This is what pixman computes.
> (By the way, I don't really understand why there is this Dca == Da check).
> 
> I have not checked other modes yet, but I suppose there may be a the same problem there.
> 

Looking at libpixman, their implementation has a number of bugs.
For instance, if you look at color-burn, you can see that the comments describe the formula correctly, but the implementation is different.

It seems that this should be fixed first.
Comment 11 Rik Cabanier 2013-01-10 13:06:35 PST
(In reply to comment #10)
> (In reply to comment #1)
> > I tried to add support for blending modes in cairo.
> > While most modes work correctly, color-burn, color-dogde, hue, luminosity and saturation mode do not work.
> > 
> > That's Pixman implementation is based on svg specification; and I think there is a difference between svg specification and the tests added in bug #104176 (and possibly also with pdf specification).
> > 
> > Pdf specification[1] states that the resulting color for color-burn compositing will be
> > 
> > Cr = (1 – as / ar) * Cb + as / ar × [( 1 – ab) × Cs + ab × B(Cb, Cs)]
> > with B(Cb, Cs) = 1 - min(1, (1 - Cb) / Cs) if Cs > 0, or 0 otherwise
> > 
> > So if we have a opaque source and backdrops (a = 1) and a color component of 0 for source, and 1 for backdrop, the result should be:
> > (1 - 1) * 1 + 1 / 1 * ((1 - 1) * 0 + 1 * 0) => 0
> > That is what is assumed in the tests for #104176.
> > 
> > But svg specification[2] states that result for color-burn compositing:
> > 
> > if Sca == 0 and Dca == Da
> >   Dca' = Sa × Da + Sca × (1 - Da) + Dca × (1 - Sa)
> >        = Sa × Da + Dca × (1 - Sa)
> > otherwise if Sca == 0
> >   Dca' = Sca × (1 - Da) + Dca × (1 - Sa)
> >        = Dca × (1 - Sa)
> > otherwise if Sca > 0
> >   Dca' = Sa × Da - Sa × Da × min(1, (1 - Dca/Da) × Sa/Sca) + Sca × (1 - Da) + Dca × (1 - Sa)
> >        = Sa × Da × (1 - min(1, (1 - Dca/Da) × Sa/Sca)) + Sca × (1 - Da) + Dca × (1 - Sa)
> > 
> > For the same backdrop and sources, we have
> > Sca = 0; Sa = 1
> > Dca = 1; Da = 1
> > So, we are in the first case, and the result should be:
> > 1 * 1 + 0 * (1 - 1) + 1 * (1 - 1) => 1
> > This is what pixman computes.
> > (By the way, I don't really understand why there is this Dca == Da check).
> > 
> > I have not checked other modes yet, but I suppose there may be a the same problem there.
> > 
> 
> Looking at libpixman, their implementation has a number of bugs.
> For instance, if you look at color-burn, you can see that the comments describe the formula correctly, but the implementation is different.
> 
> It seems that this should be fixed first.

Looking at this some more, it looks like cairo is correct, but not CG.

Maybe you should submit this path with platform specific results and I will try to fix it later.
Comment 12 arno. 2013-01-14 10:54:35 PST
Created attachment 182601 [details]
patch with platform specific expected files
Comment 13 WebKit Review Bot 2013-01-14 16:21:26 PST
Comment on attachment 182601 [details]
patch with platform specific expected files

Rejecting attachment 182601 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1 cwd: /mnt/git/webkit-commit-queue

/mnt/git/webkit-commit-queue/LayoutTests/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://queues.webkit.org/results/15868673
Comment 14 arno. 2013-01-15 14:46:54 PST
Created attachment 182849 [details]
same patch with Reviewed by NOBODY (OOPS!) added to LayoutTests/ChangeLog

OOPS. Sorry for the mistake
Comment 15 WebKit Review Bot 2013-01-15 16:00:07 PST
Comment on attachment 182849 [details]
same patch with Reviewed by NOBODY (OOPS!) added to LayoutTests/ChangeLog

Clearing flags on attachment: 182849

Committed r139804: <http://trac.webkit.org/changeset/139804>
Comment 16 WebKit Review Bot 2013-01-15 16:00:12 PST
All reviewed patches have been landed.  Closing bug.