1799258 - Do color-management on Windows+DComp via IDCompositionFilterEffects

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, enhancement)

Description

[Kelsey Gilbert [:jgilbert]]

Background investigation:

DirectComposition custom filter chains via IDCompositionDevice3 and IDCompositionFilterEffect

Requires IDCompositionDevice3:
https://learn.microsoft.com/en-us/windows/win32/api/dcomp/nn-dcomp-idcompositiondevice3

Minimum supported client Windows 8.1 [desktop apps only]

IDCompositionFilterEffect

https://learn.microsoft.com/en-us/windows/win32/api/dcomp/nn-dcomp-idcompositionfiltereffect

IDCompositionFilterEffect exposes a subset of Direct2D's image effects through DirectComposition for use in CSS filters in the browser platform.

Chaining effects: second->SetInput(0, first, 0)

We can chain IDCompositionFilterEffects via second->SetInput(0, first, 0):

void IDCompositionFilterEffect::SetInput(
  [in]           UINT     index,
  [in, optional] IUnknown *input,
  [in]           UINT     flags)

https://learn.microsoft.com/en-us/windows/win32/api/dcomp/nf-dcomp-idcompositionfiltereffect-setinput

input can be one of:

• IDCompositionAffineTransform2DEffect
• IDCompositionArithmeticCompositeEffect
• IDCompositionBlendEffect
• IDCompositionBrightnessEffect
• IDCompositionColorMatrixEffect (!!!)
• IDCompositionCompositeEffect
• IDCompositionFloodEffect
• IDCompositionGaussianBlurEffect
• IDCompositionHueRotationEffect
• IDCompositionLinearTransferEffect
• IDCompositionSaturationEffect
• IDCompositionShadowEffect
• IDCompositionTableTransferEffect (!!!)
• IDCompositionTurbulenceEffect

These are created from e.g. IDCompositionDevice3::CreateColorMatrixEffect, so we do need an IDCompositionDevice3.

IDCompositionTableTransferEffect

https://learn.microsoft.com/en-us/windows/win32/api/dcomp/nn-dcomp-idcompositiontabletransfereffect

The table transfer effect is used to map the color intensities of an image using a transfer function created from interpolating a list of values you provide.

rp<IDCompositionFilterEffect>
make_filter_invert_red(IDCompositionDevice3& dcomp) {

   rp<IDCompositionTableTransferEffect> eff;
   dcomp.CreateTableTransferEffect(&eff.setter_addrefs());
   REL_ASSERT(eff, "eff");
   const auto arr = std::vector<float>({1, 0.5, 0}); // Or just ({1, 0})
   eff->SetRedTable(arr.data(), arr.size());

   return eff;
}

Indeed, this does lerp smoothly between e.g. 1.0 and 0.5.

IDCompositionColorMatrixEffect and chaining

rp<IDCompositionFilterEffect>
make_filter_channel_swap(IDCompositionDevice3& dcomp) {

   rp<IDCompositionColorMatrixEffect> first;
   dcomp.CreateColorMatrixEffect(&first.setter_addrefs());
   REL_ASSERT(first, "first");
   first->SetMatrix(D2D1_MATRIX_5X4_F{
      // Swap green and blue
      1, 0, 0, 0,
      0, 0, 1, 0,
      0, 1, 0, 0,
      0, 0, 0, 1,
      0, 0, 0, 0,
   });

   rp<IDCompositionColorMatrixEffect> second;
   dcomp.CreateColorMatrixEffect(&second.setter_addrefs());
   REL_ASSERT(second, "second");
   second->SetMatrix(D2D1_MATRIX_5X4_F{
      // Swap red and green
      0, 1, 0, 0,
      1, 0, 0, 0,
      0, 0, 1, 0,
      0, 0, 0, 1,
      0, 0, 0, 0,
   });

   second->SetInput(0, first, 0);
   return second;
}

This does what we'd expect from first swapping G/B, and then R/G.
For an input pattern of RGBW, we first get RBGW, then GBRW, and that's indeed what we see.

Use for color management

The general form for most color space conversions are:

vec3 cspace_convert(vec3 val, src_space, dst_space) {
   val = src_space.mat_rgb_from_yuv * val;
   val = src_space.fn_linear_rgb_from_rgb(val); // eotf
   val = src_space.mat_xyz_from_linear_rgb * val;
   val = src_space.mat_approx_pcs_from_xyz * val;
   
   val = inverse(dst_space.mat_approx_pcs_from_xyz) * val;
   val = inverse(dst_space.mat_xyz_from_linear_rgb) * val;
   val = dst_space.fn_rgb_from_linear_rgb(val); // oetf
   val = inverse(dst_space.mat_rgb_from_yuv) * val;
   
   return val;
}

(We're using col-vectors and vec4 = mat4 * vec4 mult ordering)

PCS is Profile Connection Space, from ICC profiles, and is generally (CIE)XYZ with whitepoint=D50.
Annoyingly, most of our colorspaces are whitepoint=D65 instead, but if both src and dst are D65, they cancel out.
There's not a closed solution for whitepoint conversion ("Chromatic Adaptation"), and but ICC thankfully specifies a linear (Bradford matrix) approximation, and we can smash matrices together no problem.

So, generally, all we need is a chain of:

• (if src yuv) IDCompositionColorMatrixEffect src.mat_rgb_from_yuv
• then IDCompositionTableTransferEffect src.table_linear_rgb_from_rgb
• then IDCompositionColorMatrixEffect
  • inverse(dst.mat_xyz_from_linear_rgb)
  • times inverse(dst.mat_approx_pcs_from_xyz)
  • times src.mat_approx_pcs_from_xyz
  • times src.mat_xyz_from_linear_rgb
• then IDCompositionTableTransferEffect dst.table_from_rgb_from_linear_rgb

Limitations

Non-rgb non-yuv spaces like LAB need a different approach for conversion to rgb

This is fine for now.

Must sample analytical gamma functions into interpolation tables

Pretty easy to generate, but we do need to choose how many samples we want to use.

IDCompositionTableTransferEffect maps ranges [0.0-1.0]->[0.0-1.0], so HDR needs to be handled carefully

However, in practice PQ and HLG both have ceilings that we can scale with.
For arbitrary scRGB (maybe from webgl?) we can probably just pick a ceiling.
In particular, if we're outputting to HDR10(PQ) via dcomp, we can simply truncate to PQ's ceiling early.

Comment 1

[Kelsey Gilbert [:jgilbert]]

Attachment: Bug 1799258 - Do color-management on Windows+DComp via IDCompositionFilterEffects.

Comment 2

[Zaggy1024]

It seems that (at least on my Win11 installation), when the video processor's color space is YCBCR_(FULL/STUDIO)_G22_LEFT_P2020, it converts the Rec.2020 primaries to Rec.709, so the combination of color management code and the DXGI color space selection causes videos with Rec.2020 primaries to become overly saturated. Perhaps GetSourceDXGIColorSpace should now only return G22_LEFT_P709 with this color management taking care of the conversion?

Also, I think that color primaries conversion wasn't taking place in the past versions of Firefox for some reason, but that's probably not relevant for this decision, since I think the issue was fixed within the Firefox codebase at some point.

Comment 3

[Kelsey Gilbert [:jgilbert]]

Attachment: Bug 1799258 - Do color-management on Windows+DComp via IDCompositionFilterEffects.

• Add gfx.color_management.rec709_gamma_as_srgb:true. :'(

In particular, rec709(16/255) -> srgb(31/255). Even though it's
technically correct, it's practically-speaking incorrect, since that's
not what Chrome does, nor what the web expected for years and years.

In practice, basically everyone expects gamma to just be completely
ignored.

What people expect:

• Pretend gamut is srgb(==rec709), but stretch this naively for the
  display. If you have a display-p3-gamut display, srgb:0.5 expects to
  be displayed as display:0.5, which will be display-p3:0.5 to the eyes.
• Pretend all content gammas (TFs) are srgb(!=rec790), and then bitcast this
  naively for the display. E.g. rec709(16/255) should
  display the same as srgb(16/255), not srgb(31/255). (Note: display-p3
  uses srgb gamma) But if your display has e.g. gamma=3.0, don't
  convert or compensate.

This is a formalization of what you get when you spend decades ignoring
color management, and people build things based on behavior-in-practice,
not behavior-in-theory.

Also:

• gfx.color_management.native_srgb:true for Windows, so we don't use the
  display color profile, which no one else does.
• Add more std::span compat bits to mozilla::Span.
• Add rec2020_gamma_as_rec709, so we have a path towards maybe having
  rec2020 use its correct transfer function, rather than srgb (like
  rec709).

Comment 4

[Jeff Muizelaar [:jrmuizel]]

What impact does this have on power usage?

Comment 5

[Kelsey Gilbert [:jgilbert]]

With this patch, no power usage should change, since:

• Color gamut: rec709->srgb (the pref'd default) is a no-op
• Transfer function: rec709 is treated as srgb (via defaulted pref), and therefore rec709->srgb is a no-op
  So we should generate no filter chain for any content right now, under default preferences.

Comment 6

[Kelsey Gilbert [:jgilbert]]

Attachment: Bug 1799258 - [qcms] Add query for profile data and lut tables.

Comment 7

[Kelsey Gilbert [:jgilbert]]

Attachment: Bug 1799258 - Add prereq Colorspaces stuff, including generic gamma->linear LUT inversion approximation.

Comment 8

[Sotaro Ikeda [:sotaro]]

I tested patches of Bug 1799258. It adds IDCompositionFilterEffects to apply color management. I measured GPU usage by using Windows Task Manager on my one Windows11 laptop. It increased a fair amount of GPU 3D usage. I feel that the increase was too much for color management.

• 1440p60, default view, vp9 hw decode

  • latest m-c: GPU 3D 8%
  • latest m-c with patches: GPU 3D 18%

• 2160p60, fullscreen, vp9 hw decode

  • latest m-c: GPU 3D 45%
  • latest m-c with patches: GPU 3D 70%

• tested video

  • https://www.youtube.com/watch?v=LXb3EKWsInQ

Comment 9

[Kelsey Gilbert [:jgilbert]]

I made sure to fix the power usage regression for the most common case, by detecting when the input and output Transfer Functions were equal, and no-op'ing them.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/12e98a3054d0
[qcms] Add query for profile data and lut tables. r=jrmuizel
https://hg.mozilla.org/integration/autoland/rev/4f9ee3537468
Add prereq Colorspaces stuff, including generic gamma->linear LUT inversion approximation. r=gfx-reviewers,bradwerth
https://hg.mozilla.org/integration/autoland/rev/0facab7b9a1d
Do color-management on Windows+DComp via IDCompositionFilterEffects. r=sotaro

Comment 11

[Marian-Vasile Laza]

Backed out 3 changesets (Bug 1799258) for causing GTest-1proc failures on Span.
Backout link
Push with failures <--> GTest-1proc
Failure Log
Also Bd VMC32 Failure Log and Bd VMC64 Failure Log
Also bc3 Failure Log

Comment 12

[BugBot [:suhaib / :marco/ :calixte]]

There are some r+ patches which didn't land and no activity in this bug for 2 weeks.
:jgilbert, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit auto_nag documentation.

Comment 13

[Kelsey Gilbert [:jgilbert]]

https://treeherder.mozilla.org/jobs?repo=try&revision=f9a1ce1ae6a27d1c53ecf898f48df9071094a672

Comment 14

[Kelsey Gilbert [:jgilbert]]

Attachment: Bug 1799258 - No-op equal tfs rather than inverting.

Comment 15

[Kelsey Gilbert [:jgilbert]]

Attachment: Bug 1799258 - Support outByIn.size()<2 in SampleOutByIn.

This makes it easier to apply transform functions, even when they are
not defined/present.

Comment 16

[Kelsey Gilbert [:jgilbert]]

Attachment: Bug 1799258 - span_iterator::difference_type s/index_type/ptrdiff/.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b5157d950aa7
[qcms] Add query for profile data and lut tables. r=jrmuizel
https://hg.mozilla.org/integration/autoland/rev/353ef4721bba
Add prereq Colorspaces stuff, including generic gamma->linear LUT inversion approximation. r=gfx-reviewers,bradwerth
https://hg.mozilla.org/integration/autoland/rev/791eeb52f034
Do color-management on Windows+DComp via IDCompositionFilterEffects. r=sotaro
https://hg.mozilla.org/integration/autoland/rev/e05c809f58d0
No-op equal tfs rather than inverting. r=bradwerth
https://hg.mozilla.org/integration/autoland/rev/9bbbe3ed2794
Support outByIn.size()<2 in SampleOutByIn. r=bradwerth
https://hg.mozilla.org/integration/autoland/rev/22351f36b74b
span_iterator::difference_type s/index_type/ptrdiff/. r=bradwerth

Comment 18

[Natalia Csoregi [:nataliaCs]]

Backed out for failures on gfx.color_management.display_profile

• backout: https://hg.mozilla.org/integration/autoland/rev/92d1bc3781b966c1de758b6e20f68a0f484d50e0
• push: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&selectedTaskRun=BfFhpCfRTEy9ls9bvFY7tg.0&revision=22351f36b74b76f57887044e54f2c914c3fd49d5
• failure logs:
  • TEST-UNEXPECTED-FAIL | browser/base/content/test/performance/browser_preferences_usage.js | Should have accessed all known problematic prefs. Remaining: gfx.color_management.display_profile - Got 1, expected +0
  • /builds/worker/checkouts/gecko/gfx/webrender_bindings/DCLayerTree.cpp:717:30: error: cannot initialize a parameter of type 'IDCompositionEffect *' with an rvalue of type 'IDCompositionFilterEffect *'

Comment 19

[Kelsey Gilbert [:jgilbert]]

Attachment: Bug 1799258 - Ask dcomp.h to define IDCompositionFilterEffect.

Comment 20

[Kelsey Gilbert [:jgilbert]]

Attachment: Bug 1799258 - Share all-of-dcomp.h preamble, and deal with outdated mingw dcomp.h.

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/83744e1e372b
[qcms] Add query for profile data and lut tables. r=jrmuizel
https://hg.mozilla.org/integration/autoland/rev/2c23929f52f2
Add prereq Colorspaces stuff, including generic gamma->linear LUT inversion approximation. r=gfx-reviewers,bradwerth
https://hg.mozilla.org/integration/autoland/rev/b4b4977857c7
Do color-management on Windows+DComp via IDCompositionFilterEffects. r=sotaro
https://hg.mozilla.org/integration/autoland/rev/52afd24d2338
No-op equal tfs rather than inverting. r=bradwerth
https://hg.mozilla.org/integration/autoland/rev/4e68a54c6410
Support outByIn.size()<2 in SampleOutByIn. r=bradwerth
https://hg.mozilla.org/integration/autoland/rev/cd9af26bb314
span_iterator::difference_type s/index_type/ptrdiff/. r=bradwerth
https://hg.mozilla.org/integration/autoland/rev/0141b29bf5df
Ask dcomp.h to define IDCompositionFilterEffect. r=gfx-reviewers,bradwerth
https://hg.mozilla.org/integration/autoland/rev/7707637480e7
Share all-of-dcomp.h preamble, and deal with outdated mingw dcomp.h. r=gfx-reviewers,sotaro
https://hg.mozilla.org/integration/autoland/rev/a48db1421c6d
apply code formatting via Lando

Comment 22

[Joel Maher ( :jmaher ) (UTC -8)]

I am curious what the fix was? I can guess it might have been: https://phabricator.services.mozilla.com/D168325 ?

Comment 23

[Norisz Fay [:noriszfay]]

Backed out for causing build bustages on NSSCipherStrategy.cpp and Bug 1815321

Backout link

Push with failures

Failure log

Comment 24

[Tom Ritter [:tjr]]

Attachment: Bug 1799258: Add some dcomp structures needed for graphics work r?#build

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3c3c7366cc40
[qcms] Add query for profile data and lut tables. r=jrmuizel
https://hg.mozilla.org/integration/autoland/rev/b25110652394
Add prereq Colorspaces stuff, including generic gamma->linear LUT inversion approximation. r=gfx-reviewers,bradwerth
https://hg.mozilla.org/integration/autoland/rev/228200dcaf93
Do color-management on Windows+DComp via IDCompositionFilterEffects. r=sotaro
https://hg.mozilla.org/integration/autoland/rev/294a00d1c7b7
No-op equal tfs rather than inverting. r=bradwerth
https://hg.mozilla.org/integration/autoland/rev/5f911de66ec0
Support outByIn.size()<2 in SampleOutByIn. r=bradwerth
https://hg.mozilla.org/integration/autoland/rev/60a0351d9092
span_iterator::difference_type s/index_type/ptrdiff/. r=bradwerth
https://hg.mozilla.org/integration/autoland/rev/9c1d9405e8bf
Ask dcomp.h to define IDCompositionFilterEffect. r=gfx-reviewers,bradwerth
https://hg.mozilla.org/integration/autoland/rev/87f3532bfbcd
Share all-of-dcomp.h preamble, and deal with outdated mingw dcomp.h. r=gfx-reviewers,sotaro
https://hg.mozilla.org/integration/autoland/rev/40351b5987a5
apply code formatting via Lando

Comment 26

[Serban Stanca [:SerbanS]]

Backed out for causing build bustages in NSSCipherStrategy.cpp and mochitests failures in browser_preferences_usage.js.

• Backout link
• Push with failures - Build bustage --> Failure log
• Push with failures - Mochitests failures --> Failure log
• Failure line - Bustage: /builds/worker/checkouts/gecko/dom/quota/NSSCipherStrategy.cpp:112:24: error: no matching function for call to 'PK11_AEADOp'
• Failure line - Mochitests: TEST-UNEXPECTED-FAIL | browser/base/content/test/performance/browser_preferences_usage.js | Should have accessed all known problematic prefs. Remaining: gfx.color_management.display_profile - "Got 1, expected +0"

Comment 27

[Kelsey Gilbert [:jgilbert]]

Attachment: Bug 1799258 - Expect no record of accesses for gfx.color_management.display_profile for now.

StaticPrefs don't increment ACCESS_COUNTS, so switching from GetPref to
StaticPrefs, while a perf and best-practice win, causes this test to
fail.
See bug 1818130 for details.

Comment 28

[Kelsey Gilbert [:jgilbert]]

Attachment: Bug 1799258 - Fix constexpr issue on base toolchain builds.

Comment 29

[Kelsey Gilbert [:jgilbert]]

Attachment: Bug 1799258 - Fix Span const template param propagation issue on base toolchain builds.

Comment 30

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/832e547eb700
[qcms] Add query for profile data and lut tables. r=jrmuizel
https://hg.mozilla.org/integration/autoland/rev/aa4e010199c6
Add prereq Colorspaces stuff, including generic gamma->linear LUT inversion approximation. r=gfx-reviewers,bradwerth
https://hg.mozilla.org/integration/autoland/rev/ca2e6339f35f
Do color-management on Windows+DComp via IDCompositionFilterEffects. r=sotaro
https://hg.mozilla.org/integration/autoland/rev/3a22dfc1d4f0
No-op equal tfs rather than inverting. r=bradwerth
https://hg.mozilla.org/integration/autoland/rev/64f58c1c2c5c
Support outByIn.size()<2 in SampleOutByIn. r=bradwerth
https://hg.mozilla.org/integration/autoland/rev/e189f99eedd4
span_iterator::difference_type s/index_type/ptrdiff/. r=bradwerth
https://hg.mozilla.org/integration/autoland/rev/ab2b044c704c
Ask dcomp.h to define IDCompositionFilterEffect. r=gfx-reviewers,bradwerth
https://hg.mozilla.org/integration/autoland/rev/31587f0a98cd
Share all-of-dcomp.h preamble, and deal with outdated mingw dcomp.h. r=gfx-reviewers,sotaro
https://hg.mozilla.org/integration/autoland/rev/b00768cab25e
Expect no record of accesses for gfx.color_management.display_profile for now. r=florian,jrmuizel
https://hg.mozilla.org/integration/autoland/rev/3ba9692d7f5f
Fix constexpr issue on base toolchain builds. r=gfx-reviewers,lsalzman
https://hg.mozilla.org/integration/autoland/rev/e7327a3ce408
Fix Span const template param propagation issue on base toolchain builds. r=gfx-reviewers,lsalzman
https://hg.mozilla.org/integration/autoland/rev/59c2b380d474
apply code formatting via Lando

Comment 31

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/832e547eb700
https://hg.mozilla.org/mozilla-central/rev/aa4e010199c6
https://hg.mozilla.org/mozilla-central/rev/ca2e6339f35f
https://hg.mozilla.org/mozilla-central/rev/3a22dfc1d4f0
https://hg.mozilla.org/mozilla-central/rev/64f58c1c2c5c
https://hg.mozilla.org/mozilla-central/rev/e189f99eedd4
https://hg.mozilla.org/mozilla-central/rev/ab2b044c704c
https://hg.mozilla.org/mozilla-central/rev/31587f0a98cd
https://hg.mozilla.org/mozilla-central/rev/b00768cab25e
https://hg.mozilla.org/mozilla-central/rev/3ba9692d7f5f
https://hg.mozilla.org/mozilla-central/rev/e7327a3ce408
https://hg.mozilla.org/mozilla-central/rev/59c2b380d474