Bug 212477 - Extended Color: ColorMatrix should support smaller matrices and be constexpr
Summary: Extended Color: ColorMatrix should support smaller matrices and be constexpr
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sam Weinig
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-05-28 10:35 PDT by Sam Weinig
Modified: 2020-05-29 11:15 PDT (History)
5 users (show)

See Also:


Attachments
proof of concept (15.16 KB, text/plain)
2020-05-28 10:37 PDT, Sam Weinig
no flags Details
matrix.cpp (4.16 KB, text/x-csrc)
2020-05-28 10:38 PDT, Sam Weinig
no flags Details
proof of concept (4.25 KB, text/x-csrc)
2020-05-28 11:05 PDT, Sam Weinig
no flags Details
Patch (31.70 KB, patch)
2020-05-28 14:11 PDT, Sam Weinig
no flags Details | Formatted Diff | Diff
Patch (31.98 KB, patch)
2020-05-28 16:25 PDT, Sam Weinig
no flags Details | Formatted Diff | Diff
Patch (31.66 KB, patch)
2020-05-28 17:55 PDT, Sam Weinig
no flags Details | Formatted Diff | Diff
Patch (31.71 KB, patch)
2020-05-29 08:28 PDT, Sam Weinig
no flags Details | Formatted Diff | Diff
Patch (33.29 KB, patch)
2020-05-29 10:10 PDT, Sam Weinig
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sam Weinig 2020-05-28 10:35:41 PDT
The current ColorMatrix code always uses a 4x5 matrix. This is unnecessary for most (if not all) of the uses. We can do better by templatizing it and while we are at it, making it support constexpr.
Comment 1 Sam Weinig 2020-05-28 10:37:32 PDT
Created attachment 400476 [details]
proof of concept

Here is a proof-of-concept. Not sure if the actual matrix match is 100% right yet, but the code nicely compiles down to:

:-) samweinig:~> otool -Vx a.out 
a.out:
(__TEXT,__text) section
_main:
0000000100000f4a	pushq	%rbp
0000000100000f4b	movq	%rsp, %rbp
0000000100000f4e	leaq	0x53(%rip), %rdi ## literal pool for: "%f, %f, %f, %f\n"
0000000100000f55	movsd	0x3b(%rip), %xmm0
0000000100000f5d	movsd	0x3b(%rip), %xmm3
0000000100000f65	movaps	%xmm0, %xmm1
0000000100000f68	movaps	%xmm0, %xmm2
0000000100000f6b	movb	$0x4, %al
0000000100000f6d	callq	0x100000f76 ## symbol stub for: _printf
0000000100000f72	xorl	%eax, %eax
0000000100000f74	popq	%rbp
0000000100000f75	retq

when compiled with `clang++ -std=c++17 -Os matrix.cpp`
Comment 2 Sam Weinig 2020-05-28 10:38:36 PDT
Created attachment 400477 [details]
matrix.cpp
Comment 3 Sam Weinig 2020-05-28 11:05:09 PDT
Created attachment 400482 [details]
proof of concept
Comment 4 Dean Jackson 2020-05-28 12:45:47 PDT
This is awesome.
Comment 5 Dean Jackson 2020-05-28 12:47:32 PDT
Our Color Team is on fire these days.
Comment 6 Sam Weinig 2020-05-28 14:11:33 PDT Comment hidden (obsolete)
Comment 7 Sam Weinig 2020-05-28 16:25:39 PDT Comment hidden (obsolete)
Comment 8 Sam Weinig 2020-05-28 17:55:13 PDT Comment hidden (obsolete)
Comment 9 Sam Weinig 2020-05-29 08:28:54 PDT Comment hidden (obsolete)
Comment 10 Sam Weinig 2020-05-29 10:10:10 PDT
Created attachment 400596 [details]
Patch
Comment 11 EWS 2020-05-29 11:14:20 PDT
Committed r262304: <https://trac.webkit.org/changeset/262304>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 400596 [details].
Comment 12 Radar WebKit Bug Importer 2020-05-29 11:15:18 PDT
<rdar://problem/63766184>